Skip to content

Conversation

@thisisnic
Copy link
Member

@thisisnic thisisnic commented Aug 7, 2025

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions
Copy link

github-actions bot commented Aug 7, 2025

⚠️ GitHub issue #45449 has been automatically assigned in GitHub to PR creator.

Comment on lines 273 to 275
#if OPENSSL_VERSION_NUMBER >= 0x30000000L
#error Using OpenSSL version 3
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to remove this now, you either have openssl 3.0 or not

Suggested change
#if OPENSSL_VERSION_NUMBER >= 0x30000000L
#error Using OpenSSL version 3
#endif

# openssl is < 3.0
lg("Found libcurl and OpenSSL >= 1.1")
return("openssl-1.1")
# There was no error in compiling: so we found libcurl and OpenSSL >= 3.0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above, if you don't remove that second check, the script will error on openssl >= 3.0 so this won't be true.

}
lg("Found libcurl and OpenSSL < 1.1")
return("openssl-1.0")
} else if (any(grepl("Using OpenSSL version 3", errs))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I think you can remove this last else if

@thisisnic
Copy link
Member Author

thisisnic commented Aug 8, 2025

Thanks for the review @nealrichardson! Just FYI, I'm now no longer entirely convinced that the source of the problem with the binaries was definitely OpenSSL 1.X compatibility and I'm going to look into that a bit more.

@thisisnic
Copy link
Member Author

This fix isn't actually needed; superseded by #47299

@thisisnic thisisnic closed this Aug 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants