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.

Sign up for GitHub

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

OpenSSL: improve MinGW support on Windows #6079

Merged
merged 21 commits into from
Dec 31, 2024
Merged

OpenSSL: improve MinGW support on Windows #6079

Changes from 2 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
4a58502
openssl: enhance test
Doekin Dec 27, 2024
4556226
openssl: improve cross-build support on Windows
Doekin Dec 28, 2024
17ea9ac
openssl: Remove always-true `if` condition
Doekin Dec 29, 2024
abe12e9
openssl: handle mismatched Perl
Doekin Dec 29, 2024
79e98d4
openssl: fix Lua script
Doekin Dec 29, 2024
79c6f04
openssl: fix removing DLL import libraries unexpectedly
Doekin Dec 29, 2024
0e4066b
openssl: fix BSD platform
Doekin Dec 29, 2024
d5e12bc
openssl: fix wrong config
Doekin Dec 29, 2024
661f083
openssl: skip building tests
Doekin Dec 29, 2024
2121619
openssl: improve Perl dependency handling for Windows
Doekin Dec 29, 2024
2cf58d8
openssl: fix overlong make recipe
Doekin Dec 29, 2024
43c7fb9
openssl: trim trailing whitespace
Doekin Dec 29, 2024
0de6b83
openssl: improve Perl platform compatibility warning message
Doekin Dec 29, 2024
aeecc86
openssl: ensure ARFLAGS is not overridden with an empty string
Doekin Dec 30, 2024
a7144ca
openssl: restore the check for precompiled packages
Doekin Dec 30, 2024
409ef37
openssl: add LLVM resource compiler support for MinGW
Doekin Dec 30, 2024
6d5fbd8
openssl: remove temporary file after use
Doekin Dec 31, 2024
d6a3f76
openssl: add try-catch to configuration and Makefile patch functions
Doekin Dec 31, 2024
c86fa6e
openssl: improve compatibility with older versions
Doekin Dec 31, 2024
dbe0b01
openssl: add warning for building versions earlier than 1.1.1
Doekin Dec 31, 2024
977b69c
openssl: improve version checks
Doekin Dec 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 41 additions & 10 deletions packages/o/openssl/xmake.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,18 @@ package("openssl")
if package:is_plat("android") and is_subhost("windows") and os.arch() == "x64" then
-- when building for android on windows, use msys2 perl instead of strawberry-perl to avoid configure issue
package:add("deps", "msys2", {configs = {msystem = "MINGW64", base_devel = true}, private = true})
elseif is_subhost("windows") and not package:is_precompiled() then
Copy link
Member

Choose a reason for hiding this comment

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

and why not remove not package:is_precompiled()?
If you are using the build-artifacts binary, you do not need to compile openssl and install its build dependencies.

Copy link
Member

Choose a reason for hiding this comment

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

and here

Copy link
Contributor Author

@Doekin Doekin Dec 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

OK, there is no is_precompiled now

Copy link
Member

Choose a reason for hiding this comment

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

OK, there is no is_precompiled now

but we need is_precompiled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I misunderstood.

Copy link
Member

Choose a reason for hiding this comment

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

If you are downloading a precompiled binary, then all build dependencies do not need to be installed.

https://github.com/xmake-mirror/build-artifacts/releases?q=openssl&expanded=true

Copy link
Contributor Author

@Doekin Doekin Dec 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I guess the outer if not package:is_precompiled() then might work .

xmake-repo/packages/o/openssl/xmake.lua

Lines 30 to 31 in a7144ca

on_load(function (package)
if not package:is_precompiled() then

package:add("deps", "nasm", { private = true })
-- the perl executable found in GitForWindows will fail to build OpenSSL
-- see https://github.com/openssl/openssl/blob/master/NOTES-PERL.md#perl-on-windows
package:add("deps", "strawberry-perl", { system = false, private = true })
-- check xmake tool jom
import("package.tools.jom", {try = true})
if jom then
package:add("deps", "jom", {private = true})
elseif is_subhost("windows") then
package:add("deps", "strawberry-perl", { private = true })
Copy link
Member

Choose a reason for hiding this comment

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

why remove {system = false}, it will break some things. it will use incorrect perl in some systems and ci envs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please share more details about the scenarios where the system Perl might fail? There might be a workaround we can implement to address this issue. Using the system Perl helps save disk space.

For your reference, I tested with Perl installed via scoop, and it seemed to work as expected.

Copy link
Member

Choose a reason for hiding this comment

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

I can't reproduce it because I don't remember what happened before. But since there is a clear comment to fix this problem by passing system = false, we shouldn't just remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comment indicated that Perl bundled with GitForWindows fails to build OpenSSL. I encountered this issue too at the beginning. The error message is as follows:

Can't locate Pod/Usage.pm in @INC (you may need to install the Pod::Usage module) (@INC entries checked: . /usr/lib/perl5/site_perl /usr/share/perl5/site_perl /usr/lib/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib/perl5/core_perl /usr/share/perl5/core_perl) at configdata.pm line 11398.
BEGIN failed--compilation aborted at configdata.pm line 11398.
Compilation failed in require.
BEGIN failed--compilation aborted.

I looked into the config script and found that Pod::Usage is only used to generate help messages, which are shown with the ./Configure help command. I guess that the help message is not needed when installing the package. So, I removed the use of the Pod::Usage module, and it builds successfully using Perl from GitForWindows.

if package:is_plat("windows") then
package:add("deps", "nasm", { private = true })
-- check xmake tool jom
import("package.tools.jom", {try = true})
if jom then
package:add("deps", "jom", {private = true})
end
else
wprint("package(openssl): OpenSSL should be built in a Unix-like shell (e.g., MSYS2, Git Bash) if not targeting MSVC. " ..
"It seems you are not in such an environment. If you encounter build errors, please consider trying again in a Unix-like shell.")
end
end
end
Expand Down Expand Up @@ -118,6 +121,9 @@ package("openssl")
if package:config("shared") then
os.mkdir("fuzz")
end

io.replace("Configure", "use Pod::Usage;", "", {plain = true})
io.replace("Configure", "pod2usage.-;", "")
os.vrunv("perl", configs, {envs = buildenvs})
import("package.tools.make").build(package)
import("package.tools.make").make(package, {"install_sw"})
Expand Down Expand Up @@ -173,6 +179,9 @@ package("openssl")
if package:debug() then
table.insert(configs, "--debug")
end

io.replace("Configure", "use Pod::Usage;", "", {plain = true})
io.replace("Configure", "pod2usage.-;", "")
local buildenvs = import("package.tools.autoconf").buildenvs(package)
if package:is_cross() then
if is_host("windows") and package:is_plat("android") then
Expand All @@ -192,5 +201,27 @@ package("openssl")
end
end)
on_test(function (package)
assert(package:has_cfuncs("SSL_new", {includes = "openssl/ssl.h"}))
assert(package:check_csnippets({test = [[
#include <openssl/ssl.h>

int test(){
SSL_library_init();
SSL_load_error_strings();
SSL_CTX *ctx = SSL_CTX_new(SSLv23_client_method());
if(ctx == NULL){
return 1;
}

SSL *ssl = SSL_new(ctx);
if(ssl == NULL){
SSL_CTX_free(ctx);
return 1;
}

SSL_free(ssl);
SSL_CTX_free(ctx);

return 0;
}
]]}))
end)
Loading