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

grpc: Fix recipe to prepend to LD_LIBRARY_PATH rather than clobber #24651

Merged

Conversation

nextsilicon-itay-bookstein
Copy link
Contributor

@nextsilicon-itay-bookstein nextsilicon-itay-bookstein commented Jul 17, 2024

In the presence of an ambient LD_LIBRARY_PATH we want to prepend rather than clobber, same as in the case of Mac. Otherwise we may end up missing library paths required by the user.

Summary

Changes to recipe: grpc

Motivation

Recipe modifies cmake in a way that invokes protoc with a modified LD_LIBRARY_PATH under Linux. However, rather than prepending, it assigns, causing build errors when LD_LIBRARY_PATH is necessary.

Details

The PR fixes the recipe to prepend to the existing environment variable.


@conan-center-bot

This comment has been minimized.

@jcar87
Copy link
Contributor

jcar87 commented Jul 17, 2024

Hi @nextsilicon-itay-bookstein , thanks for opening this PR. We are currently in maintenance, but will get to this shortly.

I would imagine that this is something specific to your case, rather than something general? that is, you have en overridden LD_LIBRARY_PATH to support the system runtime (glibc, libstdc++, libm, pthread, etc), correct? Otherwise this should not cause errors

@nextsilicon-itay-bookstein
Copy link
Contributor Author

Yes, for example easybuild/lmod foreign package manager, where loading packages sets this environment variable. Note that the Mac flow just a few lines beforehand does prepend instead of assign.

@nextsilicon-itay-bookstein
Copy link
Contributor Author

Actually, upon further inspection, the Mac flow only propagates, doesn't preprend. Odd.

@conan-center-bot

This comment has been minimized.

In the presence of an ambient LD_LIBRARY_PATH we want to prepend rather than clobber, same as in the case of Mac. Otherwise we may end up missing library paths required by the user.
@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 5 (9665f1d07952442e7ef87fdac1bbf8dd86e5439b):

  • grpc/1.65.0:
    All packages built successfully! (All logs)

  • grpc/1.54.3:
    All packages built successfully! (All logs)

  • grpc/1.50.1:
    All packages built successfully! (All logs)

  • grpc/1.50.0:
    All packages built successfully! (All logs)


Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 5 (9665f1d07952442e7ef87fdac1bbf8dd86e5439b):

  • grpc/1.50.0:
    All packages built successfully! (All logs)

  • grpc/1.54.3:
    All packages built successfully! (All logs)

  • grpc/1.65.0:
    All packages built successfully! (All logs)

  • grpc/1.50.1:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit ed9caa3 into conan-io:master Jul 18, 2024
24 checks passed
@nextsilicon-itay-bookstein nextsilicon-itay-bookstein deleted the patch-1 branch July 19, 2024 09:23
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.

4 participants