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

[bug] bazel generator not usable for packages with multiple libs, like openssl #11559

Closed
a4z opened this issue Jun 30, 2022 · 5 comments · Fixed by #11560
Closed

[bug] bazel generator not usable for packages with multiple libs, like openssl #11559

a4z opened this issue Jun 30, 2022 · 5 comments · Fixed by #11560
Milestone

Comments

@a4z
Copy link
Contributor

a4z commented Jun 30, 2022

Environment Details

  • Operating System+version: All
  • Compiler+version: Not relevant
  • Conan version: All
  • Python version: All

Steps to reproduce

Use the bazel generator and use libcurl on Linux in your project, so it depends on openssl.
libcurl will not be usable in your bazel project, since libcrypto.a and libssl.a appear in the wrong order for the linker

Solution

To make the order work, the alwayslink = True attribute should be set on the cc_import libs

So this:

cc_import(
    name = "ssl_precompiled",
    static_library = "lib/libssl.a",
)

cc_import(
    name = "crypto_precompiled",
    static_library = "lib/libcrypto.a",
)

should look like that

cc_import(
    name = "ssl_precompiled",
    static_library = "lib/libssl.a",
    alwayslink = True,
)

cc_import(
    name = "crypto_precompiled",
    static_library = "lib/libcrypto.a",
    alwayslink = True,
)

in the generated BUILD file for openssl

we can also discuss if it would not make sense to set visibility = ["//visibility:public"], on the cc_import part, since there are use cases where users use only libcrypto , but not libssl from openssl, but that might be a different issue

@a4z
Copy link
Contributor Author

a4z commented Jun 30, 2022

The alwayslink option has of course the disadvantage that it will blow up the size from the consumer

But I have no idea how that could be solved otherwise, since we do not know the required order of the libs.

It could be an option to make the cc_import visibility public, then consumers can create their own version of a cc_library, and have the library just exporting the interface, but that is also complicated.

With alwayslink, the recipes can at least be used ...

@a4z
Copy link
Contributor Author

a4z commented Jul 1, 2022

additional info: conan writes the order of libraries correct,
I am not sure if that happens on purpose or by accident, but that does not matter in that context.

anyhow, the reason why I have the problem:
since I use the packages in a multi platform scenario, I need to re-write some names, like, openssl -> openssl_armv8

this happens with buildozer

unfortunately, buildozer uses buildifier to format the BUILD file, and that flips the order
I can not find out how to prevent that from happen, see bazelbuild/buildtools#1073

shall I change the PR so it adds a # do not sort comment on the right position, so buildifier does not touch the deps order ?
sorting can also happen when you have your editor configured to automatically run buildifier on files, like I do it in some projects , and we have even a CI rule, that BUILD files need to be buildifier conform.

so maybe adding the comment would be a good idea, what do you think, @czoido ?

@czoido
Copy link
Contributor

czoido commented Jul 1, 2022

Hi @a4z,
I was also double checking that the order was correct, yes, we sort the components before writing the deps in the generator files. I could not reproduce because I did not use buildozer for my tests, but now the problem seems clear.
Does only adding the # do not sort comment (but without alwayslink = True) fix the issue for you? If that's true I think we could just add that comment to prevent this to happen for people using this tool.

@a4z
Copy link
Contributor Author

a4z commented Jul 1, 2022

yes, the #do not sort on the right position does prevent buildifier from sorting array entries
I updated the PR

@czoido
Copy link
Contributor

czoido commented Jul 1, 2022

yes, the #do not sort on the right position does prevent buildifier from sorting array entries I updated the PR

Great, I'll mark this for the next release. Thanks for the contribution.

@czoido czoido added this to the 1.51 milestone Jul 1, 2022
@czoido czoido removed their assignment Jul 5, 2022
memsharded pushed a commit that referenced this issue Jul 26, 2022
* Add alwayslink attribute to bazel static library import

Fixes #11559

There is no test currently for the static library only version
Test exists only for the case when shared_with_interface_libs contains
elements.

* Remove always link in favor of do not sort

* fix tests

* Update conans/test/unittests/tools/google/test_bazeldeps.py

Co-authored-by: czoido <mrgalleta@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants