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

[question] Empty buildenv for test_package #14549

Closed
1 task done
PengZheng opened this issue Aug 23, 2023 · 7 comments
Closed
1 task done

[question] Empty buildenv for test_package #14549

PengZheng opened this issue Aug 23, 2023 · 7 comments

Comments

@PengZheng
Copy link

PengZheng commented Aug 23, 2023

What is your question?

When I add test_package for Conan 2 to Celix, I found that generated conanbuildenv.sh is empty so that linker failed to find Celix's dependencies:

conan create .  -pr:b default -pr:h default -b missing -o celix/*:build_all=True -tf examples/conan_test_package_v2/
/usr/bin/ld: warning: libzip.so.5, needed by /home/peng/.conan2/p/b/celix3a8b528c6212b/p/lib/libcelix_utils.so.2, not found (try using -rpath or -rpath-link)
/usr/bin/ld: /home/peng/.conan2/p/b/celix3a8b528c6212b/p/lib/libcelix_utils.so.2: undefined reference to `zip_close'
script_folder="/home/peng/Downloads/git/mycelix/examples/conan_test_package_v2/build/gcc-11-x86_64-gnu17-release/generators"
echo "echo Restoring environment" > "$script_folder/../../../build/gcc-11-x86_64-gnu17-release/generators/deactivate_conanbuildenv-release-x86_64.sh"
for v in 
do
    is_defined="true"
    value=$(printenv $v) || is_defined="" || true
    if [ -n "$value" ] || [ -n "$is_defined" ]
    then
        echo export "$v='$value'" >> "$script_folder/../../../build/gcc-11-x86_64-gnu17-release/generators/deactivate_conanbuildenv-release-x86_64.sh"
    else
        echo unset $v >> "$script_folder/../../../build/gcc-11-x86_64-gnu17-release/generators/deactivate_conanbuildenv-release-x86_64.sh"
    fi
done

# no LD_LIBRARY_PATH settings

On the other hand, conanrunenv.sh setups LD_LIBRARY_PATH correctly.
And for Conan 1, I have not encountered such issue.

Here is my test_package:
https://github.com/apache/celix/tree/feature/483-conan-2-support/examples/conan_test_package_v2

Here is Celix's package_info (we rely on Config.cmake to describe complex custom cmake targets):
https://github.com/apache/celix/blob/00336571046022d1d932db56befe0005cfeb45a4/conanfile.py#L448-L455

    def package_info(self):
        # enable imports() of conanfile.py to collect bundles from the local cache using @bindirs
        # check https://docs.conan.io/en/latest/reference/conanfile/methods.html#imports
        self.cpp_info.bindirs = ["bin", os.path.join("share", self.name, "bundles")]
        self.cpp_info.build_modules["cmake"].append(os.path.join("lib", "cmake", "Celix", "CelixConfig.cmake"))
        self.cpp_info.build_modules["cmake_find_package"].append(os.path.join("lib", "cmake",
                                                                              "Celix", "CelixConfig.cmake"))
        self.cpp_info.set_property("cmake_build_modules", [os.path.join("lib", "cmake", "Celix", "CelixConfig.cmake")])

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@PengZheng
Copy link
Author

PengZheng commented Aug 23, 2023

I just noticed that this issue has been reported several times:
#7192
#13302

We have the following work around in test_package in Conan1:

        # the following is workaround https://github.com/conan-io/conan/issues/7192
        if self.settings.os == "Linux":
            cmake.definitions["CMAKE_EXE_LINKER_FLAGS"] = "-Wl,--unresolved-symbols=ignore-in-shared-libs"
        elif self.settings.os == "Macos":
            cmake.definitions["CMAKE_EXE_LINKER_FLAGS"] = "-Wl,-undefined -Wl,dynamic_lookup"

@memsharded
Copy link
Member

This shouldn't be an issue resolved by conanbuild scripts, as that is intended for execution of tools, not for compilation. If using CMake, it should be able to find and link without any reliance on LD_LIBRARY_PATHS or the like.

Have you tried the recommendation in #13302 of doing target_link_libraries(libB PUBLIC libA::libA)? Didn't it work?

@PengZheng
Copy link
Author

Have you tried the recommendation in #13302 of doing target_link_libraries(libB PUBLIC libA::libA)? Didn't it work?

Yes, changing private linking to public works, at the price of breaking the semantics of private linking.

@memsharded
Copy link
Member

Yes, changing private linking to public works, at the price of breaking the semantics of private linking.

This is mostly seen in executables targets or shared targets. In the first case the semantics of private linking are irrelevant. In the second case, it shouldn't be a big issue, because when Conan consumes that library downstream it will generate new xxx-config.cmake files with the right visibility of transitive targets if necessary. I am not saying that this cannot be improved, we have a roadmap effort to try to improve all these things of the CMakeDeps integration. But in practice we haven't seen this being a huge problem, so the workaround could be reasonable for many users (No need to explicitly define in the recipe the CMAKE_EXE_LINKER_FLAGS manually)

@PengZheng
Copy link
Author

PengZheng commented Aug 23, 2023

because when Conan consumes that library downstream it will generate new xxx-config.cmake files with the right visibility of transitive targets if necessary.

Celix provides two kinds of artifacts: libraries and bundles. Conan can describe the former pretty well in a build-system neutral way. Celix provides its own CelixConfig.cmake mainly because bundles, which are complex custom targets, are difficult (if not impossible) to be described by Conan. I have to think about how to improve this in the future.

But in practice we haven't seen this being a huge problem, so the workaround could be reasonable for many users (No need to explicitly define in the recipe the CMAKE_EXE_LINKER_FLAGS manually).

What if the said private linking is introduced by an upstream project which I can not modify its source?
Should I send a patch to conan-center-index?

Conan deals with it. No problem.

@PengZheng
Copy link
Author

PengZheng commented Aug 24, 2023

My Conan 2 port is finished: apache/celix#620
Thanks for all helps in the past week. @memsharded

I encounter an issue when using CMakeUserPresets.json with CLion.
But I guess it is a Clion issue: https://youtrack.jetbrains.com/issue/CPP-34818

@memsharded
Copy link
Member

Thanks for all helps in the past week. @memsharded

Thanks to you for all the feedback and support!

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

No branches or pull requests

2 participants