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

Pick up the provided OpenSSL #4136

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Pick up the provided OpenSSL #4136

wants to merge 6 commits into from

Conversation

ktf
Copy link
Member

@ktf ktf commented Apr 30, 2022

Without this it picks up the system version on linux.

Without this it picks up the system version on linux.
@ktf ktf requested a review from a team as a code owner April 30, 2022 00:21
@chiarazampolli
Copy link
Collaborator

Hello @ktf ,

Are these errors genuine, or could this be merged?

Thanks!

@ktf
Copy link
Member Author

ktf commented May 2, 2022

I think they are actually genuine.

@Barthelemy @knopers8 could you investigate a bit? It seems to me that the cmake from QC actually picks up OpenSSL from the system. This was meant to force it to pick it up from our own build, where specified, but still it does not work. I remember you had to do something weird to pick up openssl, can you remind me what that was?

@ktf
Copy link
Member Author

ktf commented May 2, 2022

Ok, I am now forcing the OpenSSL detection also in O2. In principle the actual issue could actually be fixed at QC level by dropping the dependency on CURL::libcurl since O2::CCDB should already provide a consistent set of dependencies. @Barthelemy is looking into it. However, since that would require a new QC release, let's see if the changes proposed in the PR can converge faster.

o2.sh Show resolved Hide resolved
@chiarazampolli
Copy link
Collaborator

Hello,

Are the errors still genuine?

Chiara

@ktf
Copy link
Member Author

ktf commented May 4, 2022

Yes. It looks like gRPC is built with system openssl on cc8.

@ktf ktf requested a review from a team as a code owner May 4, 2022 07:40
@ktf
Copy link
Member Author

ktf commented May 4, 2022

It should be fixed in the last commit.

@chiarazampolli
Copy link
Collaborator

Hello,
I am sorry to bother about this, but the tests are all red now...

ktf added 2 commits May 5, 2022 12:26
RPATH done in this way is not relocatable, which will end up not actually picking our SSL version.
@chiarazampolli
Copy link
Collaborator

Sigh, still not ok... @ktf

@TimoWilken TimoWilken requested a review from a team as a code owner March 14, 2023 13:07
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