Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add note about EXT_BUILD_DEPS to docs #970

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yeswalrus
Copy link

@yeswalrus yeswalrus commented Oct 3, 2022

When building libraries with dependencies, such as curl & OpenSSL, I've found this to be necessary. It would have been useful to me to have this documented.

@google-cla
Copy link

google-cla bot commented Oct 3, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@jiridanek
Copy link

jiridanek commented Oct 18, 2022

Please also document EXT_BUILD_ROOT. In my defs, I always add something like

cmake(
   name = "cli-cpp",
   cache_entries = {
       "EXT_BUILD_DEPS": "$EXT_BUILD_DEPS",
       "EXT_BUILD_ROOT": "$EXT_BUILD_ROOT",

       "Qpid_DIR": "$EXT_BUILD_DEPS/qpid-cpp/lib64/cmake/Qpid",
      # ...

That is, I make EXT_BUILD_DEPS and EXT_BUILD_ROOT visible as CMake variables to the build. This is to make the .cmake files in the install directory work when I have two layers of cmake builds (ext_dep_B depends on ext_dep_A, both are built by CMake).

For example, under "Qpid_DIR", in file QpidConfig.cmake, I end up with the following, so if the EXT_BUILD_ROOT variable isn't defined, I get compile error make[2]: *** No rule to make target '/bazel-out/k8-fastbuild/bin/qpid-cpp/lib64/libqpidmessaging.so', needed by 'target/bin/aac3_connector'. Stop..

set (Qpid_LIBRARIES     optimized ${EXT_BUILD_ROOT}/bazel-out/k8-fastbuild/bin/qpid-cpp/lib64/libqpidmessaging.so ${EXT_BUILD_ROOT}/bazel-out/k8-fastbuild/bin/qpid-cpp/lib64/libqpidtypes.so debug ${EXT_BUILD_ROOT}/bazel-out/k8-fastbuild/bin/qpid-cpp/lib64/libqpidmessaging.so ${EXT_BUILD_ROOT}/bazel-out/k8-fastbuild/bin/qpid-cpp/lib64/libqpidtypes.so)

@jsharpe
Copy link
Member

jsharpe commented Nov 4, 2022

@jiridanek that sounds like a bug, are you able to create a simple reproducer to show this behaviour? Those variables should be rewritten as part of the build script so if you're needing to do that then something isn't quite right in the rules.

@jiridanek
Copy link

jiridanek commented Nov 4, 2022

@jiridanek that sounds like a bug, are you able to create a simple reproducer to show this behaviour? Those variables should be rewritten as part of the build script so if you're needing to do that then something isn't quite right in the rules.

@jsharpe thanks for looking into this!

Part of the problem was a bug in the CMakeLists.txt of the project being built using the foreign_cc rules. The project was (still is) using absolute paths in its *Config.cmake file, so the make installed directory cannot be moved around. The necessary change to fix that is apache/qpid-cpp#33. Meaning, the manipulation with EXT_BUILD_ROOT I described (and disabling bazel sandbox) is now not necessary.

Other than that, I still think I need to do

cache_entries = {
       "EXT_BUILD_DEPS": "$EXT_BUILD_DEPS",

but this is something I wrote when I was first getting familiar with foreign_cc, and there were also subsequent fixes to my other 3rd party dependency CMakeLists.txt build files (such as apache/qpid-proton#330), so maybe it is no longer necessary...

These custom cmake install scripts in 3rd party dependencies that rely on absolute paths are a plague.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants