hhvm / packaging

The sources for how we have built the HHVM packages.
MIT License
64 stars 63 forks source link

bin/prune-source-tree #242

Closed jjergus closed 4 years ago

jjergus commented 4 years ago

There are multiple redundant submodules in third-party/fb-mysql (boost, lz4, maybe others), I wonder if we can clean that up. But fb-mysql seems to have strong opinions about the exact boost version it needs, so it would probably make more sense if we used fb-mysql's boost in HHVM than vice versa. I'm not sure how linking works if we end up trying to statically link multiple different versions of the same library...

fredemmott commented 4 years ago

Does fb-mysql actually need boost objects, or just headers?

I'd rather avoid using another project's boost version if at all possible - that sounds like it's going to cause dependency problems down the line.

jjergus commented 4 years ago

Hmm if I'm reading it correctly it might not need boost at all if we're building WITHOUT_SERVER=1 -- so that one should be fine. I can probably update this to delete the whole copy of boost from the fb-mysql submodule so we only have the original one in third-party/boost.

There are a few more potentially conflicting submodules though:

fredemmott commented 4 years ago

re2: if we don't directly need it, let's delete it lz4/zstd: if they can use ours, great. Otherwise, as long as everyone's statically linking or using the system version, good to use ours if practical, but if it's too painful, doesn't hurt to use both.

jjergus commented 4 years ago

lz4/zstd: if they can use ours, great.

I may need to modify our cmake setup so that these are treated as "external projects" that get "installed" somewhere in the build directory (so we can pass that path to fb-mysql, which looks for <path>/include and <path>/lib) but I think we wanted to do that anyway for all the submodules eventually.

It might also help with another problem I've been seeing lately, which is that about half the projects have a config.h file so if we put the source directory (as opposed to an <install>/include directory) in the include path for other projects, they sometimes include the wrong config.h :/

BTW should we keep adding a new "install" directory for each submodule (currently we have ${CMAKE_CURRENT_BINARY_DIR}/libzip-install etc.), or should we just have a single "install" directory for all the submodules? It could simplify things if we could just do -I${PROJECT_BINARY_DIR}/install/include for every submodule...

doesn't hurt to use both.

Hmm I don't know how this works, what will happen during linking if we have 2 identical static libraries (.a files) on the command line? What if they're not identical but have the same symbols? Is it different if the linking happens in multiple passes (e.g. libmysqlclient.a has its own liblz4.a inside it, then later we try to link libmysqlclient.a with a different liblz4.a)?

fredemmott commented 4 years ago

I may need to modify our cmake setup so that these are treated as "external projects" that get "installed" somewhere in the build directory (so we can pass that path to fb-mysql, which looks for /include and /lib) but I think we wanted to do that anyway for all the submodules eventually.

Yeah, this should generally happen for (almost?) all our dependencies

Also, we shouldn't need to choose a safe place - really we should do -DCMAKE_INSTALL_PREFIX=<INSTALL_DIR> - <INSTALL_DIR> is a placeholder in the context of ExternalProject_add; what cmake decides on can be retrieved with ExternalProject_Get_property