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

Install openssl 1.0.2 on windows to use with connext #490

Closed
wants to merge 2 commits into from

Conversation

mikaelarguedas
Copy link
Member

Redo of #421 as requested in #421 (comment)

This installs openssl 1.0.2 alongside openssl 1.1.1.

Relies on ros2/system_tests#439
First commit is unrelated to this change but was useful to reduce CI time, it can be removed from this PR if not sensible

Debug

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Release:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
…sl 1.0.2

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
@nuclearsandwich
Copy link
Member

This PR introduces further conflicts with #425 which I'm hesitant to merge.

Regarding the removal of opensplice as a dependency I would rather see that change made across all platforms rather than "snuck in" to an unrelated PR. I don't mind if it stays in the meantime while you iterate.

How confident are you that both OpenSSL installations are non-conflicting and being used? The 1.1.1g binaries default to installing in a separate path from the 1.0.2 binaries unless there was an existing installation of 1.0.2 previously in which case I've seen them use the same install paths.

If there are log excerpts from the test builds which show the correct OpenSSL version being used in both scenarios can you please add them to the PR body.

@nuclearsandwich nuclearsandwich self-requested a review July 6, 2020 15:51
@mikaelarguedas
Copy link
Member Author

Regarding the removal of opensplice as a dependency I would rather see that change made across all platforms rather than "snuck in" to an unrelated PR. I don't mind if it stays in the meantime while you iterate.

👍

How confident are you that both OpenSSL installations are non-conflicting and being used? The 1.1.1g binaries default to installing in a separate path from the 1.0.2 binaries unless there was an existing installation of 1.0.2 previously in which case I've seen them use the same install paths.

Not very, this is why I was originally attempting to use the ones provided by RTI where we had a good handle on the installation location. Unfortunately those never seem to have worked on the windows job configuration.
TBH the only things I'm confident with are the fact that when adding new install location to the paths the connext jobs start passing and that ros2/system_tests#439 fixes cases of mixing openssl versions on master (e.g. Fast-RTPS tests on MacOS currently use openssl 1.0.2 instead of openssl 1.1.1g and would be fixed with that PR).
There is a chance that these installs are conflicting somehow. And currently we don't know from the tests which one is being used.

@nuclearsandwich
Copy link
Member

@mikaelarguedas we've moved our Windows setup into a chef cookbook that's invoked from CI (#425). Would you be willing to create a PR for the chef cookbook, then we can use this PR to test the cookbook changes.

Based on my experience with local VMs the OpenSSL installers being used here share some Windows registry values which lead me to suspect that by default they'll install on top of each other. I'm not sure if there are command line parameters which can be passed to the installers to control install location explicitly but that needs to be investigated before we can confidently make use of both OpenSSL distributions.

@mikaelarguedas
Copy link
Member Author

I'm not sure how to go about this, I'll go ahead and close this PR.
I'm not familiar with chef, how would I go about adding that to the chef cookbook ? Just add another "openssl_version" entry in ci/install_ros2_foxy.json ?
The chef cookbook seems to reference openssl 1.0.2u but the PR you linked is using 1.0.2n, which one should I use?

@mikaelarguedas mikaelarguedas deleted the recent-openssl branch August 25, 2020 20:08
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.

2 participants