shared library support (issue #6)

[stefantalpalaru]

and build options for shared/static libs (both enabled by default)

[jbkempf]

Looks go to me.

@chouquette an opinion on the matter?

[jbkempf]

@stefantalpalaru did you see @chouquette comments?

[stefantalpalaru]

@stefantalpalaru did you see @chouquette comments?

Yes, of course, but I'm not playing the bike-shedding game. Mine is a serious contribution. If you want to argue about stuff like number of commits or want somebody to teach you CMake/pkg-config/dynamic libraries/linking/etc. because you can't be bothered to do your own research, please waste somebody else's time.

If you have serious questions, I'll be happy to try and answer them.

[jbkempf]

2 remarks suggesting a split which are trivial to do (takes a couple of minutes), and one legitimate question (it's clearly not obvious that C_VISIBILITY_PRESET applies to C++ also, even after reading the documentation) is quite far from bike-shedding. Especially if it is only the first question on the discussion.

[stefantalpalaru]

Nobody benefits from rewriting Git history to split commits. It is an absurd request for something that helps no one and damages me as a contributor by asking me to waste my time and mess up the proper record of my contribution. It would be like me asking you to delete your previous comment in this thread and split it in two new ones.

it's clearly not obvious that C_VISIBILITY_PRESET applies to C++ also

I don't think it does. Try also setting the CXX_VISIBILITY_PRESET property and run make with VERBOSE=1 to see the resulting "-fvisibility=hidden" argument to the compiler.

[jbkempf]

Nobody benefits from rewriting Git history to split commits. It is an absurd request for something that helps no one

Absolutely not. One feature per commit helps reviewing, reading history, reverting commits and bisecting. A lot of projects require that, and a lot of PR are using multiple commits.

and damages me as a contributor by asking me to waste my time and mess up the proper record of my contribution.

Of course not! Instead of 1 commit in the tree, you got 2 consecutive ones. It even increases your commit count, if you care about that. I don't see how that "messes up the record". And I clearly don't see why this requests upsets you so much.

It would be like me asking you to delete your previous comment in this thread and split it in two new ones.

That is a ridiculous analogy. The idea for multiple commits if for other people, in the future, to read the code and understand it. The comments here are a process that is not necessary to understand the code.

Anyway, back to the point. This is clearly not bike-shedding, like asking you to remove trailing spaces or aligning equal signs vertically...

me to waste my time

Finally, in the time of your comments, you could have done it already.

But anyway...

I don't think it does. Try also setting the CXX_VISIBILITY_PRESET property and run make with VERBOSE=1 to see the resulting "-fvisibility=hidden" argument to the compiler.

Which is exactly why @chouquette asked you: "Why do you set a C property on a C++ project?".

It is a very good question, is not obvious at all, and is clearly not bike-shedding.

Yes, I think you are clearly over-reacting on this.

[stefantalpalaru]

I tested it myself and it works as I guessed it would. New property added to this pull request.

[stefantalpalaru]

It even increases your commit count, if you care about that.

I don't. I care about my time being wasted, since it is a finite resource.

I don't see how that "messes up the record".

You don't see how deleting a commit from 25 days ago and creating 3 new ones now changes the history of when my initial contribution was created?

The idea for multiple commits if for other people, in the future, to read the code and understand it.

I agree with the concept when a single commit is too big to be read in 2-3 minutes. My commit is very small and, surely, easy to grok.

"Why do you set a C property on a C++ project?"

Your C++ project contains 3 C files.

It is a very good question, is not obvious at all, and is clearly not bike-shedding.

OK, point taken, question addressed, pull request improved.

[stefantalpalaru]

CXX_VISIBILITY_PRESET change reverted, because it won't work without marking exported symbols with __attribute__ ((visibility ("default"))) and this is too big of a change for this pull request. Keep it in mind for when you have a significant amount of C++ symbols that can be safely hidden.

As an alternative, you can mark just the hidden ones and not change the compiler's default visibility policy.

Hiding all the C symbols does not create problems for VLC, so I'm assuming those are not part of the public API.

[chouquette]

Merged. Thanks for your patch