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

Revert "Fix check for openssl on Windows" #1714

Merged
merged 1 commit into from Apr 11, 2022
Merged

Revert "Fix check for openssl on Windows" #1714

merged 1 commit into from Apr 11, 2022

Conversation

Biswa96
Copy link
Contributor

@Biswa96 Biswa96 commented Apr 3, 2022

This reverts commit 045e5c5
In Windows platform, openssl libraries have same name
(libcrypto and libssl) like other unix-like platforms.

@Biswa96
Copy link
Contributor Author

Biswa96 commented Apr 8, 2022

Any comment on this pull request? Please review.

@evelikov
Copy link
Contributor

evelikov commented Apr 8, 2022

Can you provide some references to (semi) authoritative place which shows the correct names? Say official documentation (mingw, cygwin, openssl, ...), build script or other.

Off the top of my head, this PR looks good ... yet I haven't touched Windows in 5+ years.

@evelikov
Copy link
Contributor

evelikov commented Apr 8, 2022

Casual search flags https://wiki.openssl.org/index.php/Compilation_and_Installation ... you might want to mention in the commit message:

Older version of OpenSSL X used $name_X as need in $ref_X, while newer versions Y+ have switched to $name_Y as per $ref_Y.

@Biswa96
Copy link
Contributor Author

Biswa96 commented Apr 8, 2022

Here are some of the references I found. I can try to provide further information, if required.

Casual search flags https://wiki.openssl.org/index.php/Compilation_and_Installation ...

That library name was for OpenSSL 1.0.2 or earlier. That version is not supported https://www.openssl.org/policies/releasestrat.html

@evelikov
Copy link
Contributor

evelikov commented Apr 8, 2022

Casual search flags https://wiki.openssl.org/index.php/Compilation_and_Installation ...

That library name was for OpenSSL 1.0.2 or earlier. That version is not supported https://www.openssl.org/policies/releasestrat.html

What is supported today is kind of irrelevant. The link I pasted effectively says why the original commit, and this PR, are needed. I would encourage you to summarise that ... in the commit message.

By having it in the commit itself people can see the reasoning, as things break yet again in another 9 years or so :-P

This reverts commit 045e5c5
For Windows platform, openssl 1.0.2 and earlier versions have
eay64 and eay32 libraries[1]. But from openssl 1.1.0 and above
versions have same library name[2] (libcrypto and libssl) like
other unix-like platforms.

[1]: https://wiki.openssl.org/index.php/Compilation_and_Installation#OpenSSL_1.0.2
[2]: https://wiki.openssl.org/index.php/Compilation_and_Installation#OpenSSL_1.1.0
@Biswa96
Copy link
Contributor Author

Biswa96 commented Apr 8, 2022

Thanks for the suggestion. I have added your given links in commit message.

@evelikov
Copy link
Contributor

evelikov commented Apr 9, 2022

Looks great. I don't have commit access, but suspect that @mmatuska will get to it shortly.

@mmatuska mmatuska merged commit 042956b into libarchive:master Apr 11, 2022
@Biswa96 Biswa96 deleted the win32-openssl branch April 11, 2022 14:28
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.

None yet

3 participants