-
Notifications
You must be signed in to change notification settings - Fork 254
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
link openssl statically for Windows #3118
Conversation
(will re-enable those 2 pipelines when this one is ready to merge) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the lateness! The bin in the release works for me on windows, even in cmd.exe, and I was able to (eventually) build this on my local Windows box, so the change looks good! I got a bit confused about the Perl stuff, so just a nit to add a comment
run: | | ||
echo "PERL=$((where.exe perl)[0])" | Out-File -FilePath $env:GITHUB_ENV -Append -Encoding utf8 | ||
echo "OPENSSL_SRC_PERL=$((where.exe perl)[0])" | Out-File -FilePath $env:GITHUB_ENV -Append -Encoding utf8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment linking to this issue sfackler/rust-openssl#2149? I was very confused about this line, and I'm assuming this helped you too 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure! 84af4f9 🫶
- name: Set Perl environment variables | ||
run: | | ||
echo "PERL=$((where.exe perl)[0])" | Out-File -FilePath $env:GITHUB_ENV -Append -Encoding utf8 | ||
echo "OPENSSL_SRC_PERL=$((where.exe perl)[0])" | Out-File -FilePath $env:GITHUB_ENV -Append -Encoding utf8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, can you add a comment linking to this issue? sfackler/rust-openssl#2149
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put it in the same commit! 84af4f9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care of this!
Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis. |
* ci: fix windows pipeline * vendor openssl for windows * add comment for the workaround (cherry picked from commit 49d2298)
link openssl statically for Windows (#3118) * ci: fix windows pipeline * vendor openssl for windows * add comment for the workaround (cherry picked from commit 49d2298) Co-authored-by: Yihau Chen <[email protected]>
* ci: fix windows pipeline * vendor openssl for windows * add comment for the workaround
Problem
a refined version of the timeline for future reference:
vendored
feature on openssl for all platforms.Unvendor OpenSSL for Windows to avoid CI troubles with perl.exe
c9bfc99Summary of Changes
vendored
in openssl for all platforms (it should work. I verified by this pipeline https://github.com/yihau/solana/actions/runs/11241556386/job/31253379456#step:7:8654)