openframeworks / ofxAddonTemplate

an empty ofxAddon folder structure for easy creating your own addon for openFrameworks
http://ofxaddons.com/howto
Other
77 stars 34 forks source link

Static and shared library names #10

Open bakercp opened 11 years ago

bakercp commented 11 years ago

Hi all,

It seems that the core oF does not follow any particular convention about the ways that its pre-compiled libraries are named. Sometimes they are libMYLIB.{a,so} and sometimes they are just MYLIB.{a,so}. For the purposes of consistency (and a slightly more streamlined makefile system), I'd like to hear what folks think about renaming our core libs to be prefixed with libMYLIB.{a,so} and require addons to also use that template model. The idea is that we compile using the standard -lMYLIB with search paths -L that can be reordered by the user. While obviously specifying the full path of a lib without an -l flag is possible, I think it will help keep things cleaner and more consistent across platforms.

Of course, the downside is that some project templates will need to be updated but we can do it all at once in my Makefile branch and roll it out across the board.

@arturoc @bilderbuchi what do you think?

arturoc commented 11 years ago

i changed yesteraday the way makefiles include .so in linux since it was problematic in some distributions, no it uses -L -l

for static libraries i would keep it as it is, include them with full path since that avoids ambiguities with libraries in the system vs libraries in the OF folders

for osx the standard (in OF :) is to have libraries without prefixes to avoid the compiler including libraries that might be installed in the system, macports... and include them using full path

bakercp commented 11 years ago

so in the new addons_config.mk file, users will specify shared libraries (e.g. libMySharedLib.so MyOtherSharedLib.so) as

ADDON_SHARED_LIBRARIES := MySharedLib
ADDON_SHARED_LIBRARIES += MyOtherSharedLib

and static libs like (e.g. libopencv_legacy.a and libopencv_haartraining_engine.a)

ADDON_STATIC_LIBRARIES := $(PATH_OF_ADDON)/lib/linux64/libopencv_legacy.a
ADDON_STATIC_LIBRARIES += $(PATH_OF_ADDON)/lib/linux64/libopencv_haartraining_engine.a

and for pkg-config (e.g. libMyPackageConfigLib and libMyOtherPackageConfigLib)

ADDON_SHARED_LIBRARIES := MyPackageConfigLib
ADDON_SHARED_LIBRARIES += MyOtherPackageConfigLib

It just seems rather inconsistent and potentially confusing. Since we are now (with the new makefile system) very carefully structuring our library search paths (and search paths seems to closely specify a specified order), you think we still need to be explicit about our static libs?

bakercp commented 11 years ago

Just thinking about this some more, if we use -l / -L for addon shared libs, then they will be required to be prefixed with lib, but if we go with the scenario above, we won't require static libs to be prefixed with lib. I feel like these inconsistencies will lead to more problems than mis-ordered library search paths ... particularly when this new Makefile system is so rock solid (wink wink :)

bakercp commented 11 years ago

Also, just FYI (and this is well-documented in the new makefile system), the reason we are separating out shared and static libs is so that we can now avoid the old export folder by copying any shared libs during the final build step. This is important for both the core and any addons that ship with shared libs. @bilderbuchi what are your thoughts?

bilderbuchi commented 11 years ago

I can't really judge the technical side I fear, but I would like consistency in the naming. Also, being able to drop the whole path when including would be nice - gcc console output is really painful to parse at times, since it's three lines of paths at first (but that's minor). I see arturo's point about disambiguation with system/outside libraries, though. While this may be avoid with the makefile system where we can tailor search paths nicely, I'm a bit concerned that that could lead to strange bugs when using all our supported IDEs (what with people drag&dropping things all over the place etc) - maybe a different prefix (oflibMyLib?) might be a potential solution, if that's technically possible? or libOFMyLib although that sounds a bit forced? Getting rid of the export folder: +1e6, that would be nice. all this binary duplication really seems unnecessary.

bakercp commented 11 years ago

Given these comments, perhaps we should stick with the full path idea for both shared and static libs (I don't mind that). But, @arturoc perhaps you can explain what issues you were having that caused you to make the modifications to the makefiles currently in the repo? The $(basename $(notdir ...) syntax that you have specifically outlined for linux is also what I am currently using for all platforms in my make system (which of course I can undo per this conversation), but anyway -- perhaps you could describe why you needed to make the changes and why that requirement for linux can't (or doesn't need to be) be applied to all platforms? Basically, I'm trying to not have a bunch of exceptions for specific platforms :) Thanks!

bilderbuchi commented 11 years ago

Basically, I'm trying to not have a bunch of exceptions for specific platforms

that sounds like sound logic, the makefile system is insanely complex as it is! :magic:

arturoc commented 11 years ago

there's no need to use only the name for dynamic libraries, we can use full path and parse them later inside the makefile for linux

also not 100% sure but i think dynamic libraries won't need to be specified in addons_config.mk since those won't have the order problems that static libraries have

the problem i had with specifying the full path for dynamic libraries is that in arch linux then the executable tries to find the library in that path and since the executable is in different path (bin) than the makefile the relative path to the library won't work and the -Wl,-rpath... flag won't have any effect. here's the related commits:

https://github.com/openframeworks/openFrameworks/commit/543ee081250a3d46315407875d0da1e70520c41f https://github.com/openframeworks/openFrameworks/commit/11784dffe2545af295310450a978257dd1cee0b0 https://github.com/openframeworks/openFrameworks/commit/f347c28fbe8e8ead1c2b4a4f30187017db3095f4 https://github.com/openframeworks/openFrameworks/commit/e434140db04b5af0dd460a232bdb4ae11c994dc8

this one is not that related but would be good to have it too:

https://github.com/openframeworks/openFrameworks/commit/324839e579e59a489ac500e1ff667d9f51852fe7

it removes unused symbols and libraries, so if you don't use fmod it won't get included in the executable dependencies and you can then run the binary from anywhere

bakercp commented 11 years ago

Cool. Well, since it's already in there, I'm going to leave the ability to reorder dynamic libs :)

So, please take a look at the latest config.addons.mk file. It is almost complete, pending changes in the way that libraries are specified (i.e. the discussion above). Currently I have it set so that the lib prefix is required, but obviously we can change that.

You'll notice that it's significantly easier to read, there are tons of comments (they still need a proof read). Basically, the system now recursively searches for and orders addon dependencies using tsort (that was a very tricky thing to write in gnu make :)). After the topologically ordered list of addons is created, then we iterate through the addons and create a bunch of variables with the prefix "ORDERED". These "ORDERED" variables will be accumulated in into a variable (that will be accessible to the main project compile make file) in the for loop here https://github.com/bakercp/openFrameworks/blob/49fb1b2264b3ee39161e9eceae8b28a82004f78a/libs/openFrameworksCompiled/project/makefileCommon/config.addons.mk#L1010

... and then it will finish compiling.

I haven't accumulated all of the variables yet, pending our final decision about the lib situation. I also have a few other TODO items. Other than that and the integration of a few more things (and getting caught up with your changes), it's just about ready to go. A TODO list is in the PR description above ...

I have tested everything pretty well to this point. Please let me know what you think. I need to take a little break on this for a day or two :)

bakercp commented 11 years ago

p.s. ping @arturoc

bakercp commented 11 years ago

Also, the PR above was referring to this one ... I thought I was posting in that thread :) https://github.com/openframeworks/openFrameworks/pull/2094

arturoc commented 11 years ago

wow! :) that looks really great!

so just to make it clear i think the best would be that every library has to be included with full path no matter if it's static or dynamic then in the project config phase there's a conditional for linux that parses the shared libraries variable and converts them to -lbasename -Ldir

arturoc commented 11 years ago

btw, this:

ADDON_STATIC_LIBRARIES := $(PATH_OF_ADDON)/lib/linux64/libopencv_legacy.a

it's probably better like

ADDON_STATIC_LIBRARIES := libs/opencv/liblinux64/libopencv_legacy.a

without any variable since that might be confusing for people that don't know about makefiles and more difficult to parse from anything else