OpenSSL: improve MinGW support on Windows #6079

Doekin · 2024-12-27T16:59:52Z

OpenSSL targeting MinGW can be built on Linux or MSYS2, but not directly on Windows. This PR introduces a workaround to enable building on Windows using Git Bash.

waruqi · 2024-12-28T17:17:30Z

packages/o/openssl/xmake.lua

-                if jom then
-                    package:add("deps", "jom", {private = true})
+            elseif is_subhost("windows") then
+                package:add("deps", "strawberry-perl", { private = true })


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

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.

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.

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.

waruqi · 2024-12-28T17:19:01Z

packages/o/openssl/xmake.lua

@@ -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


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.

OK, there is no is_precompiled now

OK, there is no is_precompiled now

but we need is_precompiled.

Sorry, I misunderstood.

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

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

Doekin · 2024-12-29T04:42:05Z

Supported Combinations of Targets and Windows Environments

Build for Windows in PowerShell with Strawberry Perl
Build for MinGW in Git Bash with bundled Perl

Unsupported Combinations of Targets and Environments

Build for MinGW in PowerShell
- This is expected due to the reliance on many Unix-like shell commands in the makefile.

waruqi · 2024-12-29T06:21:12Z

packages/o/openssl/xmake.lua

+            package:add("deps", "msys2", {configs = {msystem = "MINGW64", base_devel = true}, private = true})
+        elseif is_subhost("windows") then
+            import("lib.detect.find_tool")
+            local perl = find_tool("perl", {paths={"$(env PERL)"}})


we should not call find_tool in on_load

waruqi · 2024-12-29T06:22:01Z

packages/o/openssl/xmake.lua

+    if on_check then
+        on_check(function (package)
+            import("lib.detect.find_tool")
+            local perl = assert(find_tool("perl", {paths={"$(env PERL)"}}), "package(openssl): perl not found!")


use package:find_tool and add spaces =

waruqi · 2024-12-29T06:23:17Z

packages/o/openssl/xmake.lua

@@ -83,7 +102,13 @@ package("openssl")
            table.insert(configs, "no-makedepend")
            table.insert(configs, "/FS")
        end
-        os.vrunv("perl", configs)
+        import("lib.detect.find_tool")
+        local perl = assert(find_tool("perl", {paths={"$(env PERL)"}}), "package(openssl): perl not found!")


we should not call find_tool, we should use add_deps to bind perl path end and run perl

SirLynix · 2024-12-29T13:50:31Z

I'm sorry but I don't understand what benefit this PR does, it seems complicated to just use the system perl instead of installing strawberry perl or am I missing something?

Doekin · 2024-12-29T14:22:34Z

Thank you for your feedback. Let me clarify the reasoning behind this PR:

When building OpenSSL for Windows, we can make use of a previously installed Strawberry Perl, as OpenSSL‘s configure script won't find the Perl bundled with Git Bash if we are in PowerShell or Command Prompt.
For building OpenSSL for MinGW, Strawberry Perl cannot generate the Unix-style paths that OpenSSL requires, which leads to build failures. I added prompts to guide users to build in the appropriate environment. Suitable environments include Git Bash and MSYS2 shell.

Additionally, when building OpenSSL in Git Bash, we may encounter errors due to the missing POD::Usage Perl module, which is not included in the Perl bundled with Git Bash. However, this module is not essential for configuring OpenSSL's build; it is used to generate help information in the configure script. We can remove the use of POD::Usage to continue building.

I hope this clarifies the purpose of the changes.

openssl: enhance test

4a58502

Doekin marked this pull request as draft December 27, 2024 23:50

This comment was marked as outdated.

Sign in to view

Doekin force-pushed the openssl branch from 6c983cf to 13c765a Compare December 28, 2024 03:14

openssl: improve cross-build support on Windows

4556226

Doekin force-pushed the openssl branch from 13c765a to 4556226 Compare December 28, 2024 03:15

Doekin marked this pull request as ready for review December 28, 2024 03:17

star-hengxing requested a review from SirLynix December 28, 2024 05:49

waruqi reviewed Dec 28, 2024

View reviewed changes

Doekin added 4 commits December 29, 2024 08:27

openssl: Remove always-true if condition

17ea9ac

openssl: handle mismatched Perl

abe12e9

openssl: fix Lua script

79e98d4

openssl: fix removing DLL import libraries unexpectedly

79c6f04

Doekin added 2 commits December 29, 2024 13:05

openssl: fix BSD platform

0e4066b

openssl: fix wrong config

d5e12bc

Doekin force-pushed the openssl branch from d186b20 to d5e12bc Compare December 29, 2024 05:14

openssl: skip building tests

661f083

waruqi reviewed Dec 29, 2024

View reviewed changes

openssl: improve Perl dependency handling for Windows

2121619

Doekin marked this pull request as draft December 29, 2024 07:11

openssl: fix overlong make recipe

2cf58d8

Doekin marked this pull request as ready for review December 29, 2024 09:21

Doekin added 2 commits December 29, 2024 17:43

openssl: trim trailing whitespace

43c7fb9

openssl: improve Perl platform compatibility warning message

0de6b83

Doekin added 3 commits December 30, 2024 08:45

openssl: ensure ARFLAGS is not overridden with an empty string

aeecc86

openssl: restore the check for precompiled packages

a7144ca

openssl: add LLVM resource compiler support for MinGW

409ef37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OpenSSL: improve MinGW support on Windows #6079

OpenSSL: improve MinGW support on Windows #6079

Doekin commented Dec 27, 2024 •

edited

Loading

This comment was marked as outdated.

waruqi Dec 28, 2024

Doekin Dec 29, 2024

waruqi Dec 29, 2024

Doekin Dec 29, 2024

waruqi Dec 28, 2024

waruqi Dec 29, 2024

Doekin Dec 29, 2024 •

edited

Loading

waruqi Dec 30, 2024

Doekin Dec 30, 2024

waruqi Dec 30, 2024

Doekin Dec 30, 2024 •

edited

Loading

Doekin commented Dec 29, 2024 •

edited

Loading

waruqi Dec 29, 2024

waruqi Dec 29, 2024

waruqi Dec 29, 2024

SirLynix commented Dec 29, 2024

Doekin commented Dec 29, 2024 •

edited

Loading

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

OpenSSL: improve MinGW support on Windows #6079

Are you sure you want to change the base?

OpenSSL: improve MinGW support on Windows #6079

Conversation

Doekin commented Dec 27, 2024 • edited Loading

This comment was marked as outdated.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Doekin Dec 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Doekin Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

Doekin commented Dec 29, 2024 • edited Loading

Supported Combinations of Targets and Windows Environments

Unsupported Combinations of Targets and Environments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SirLynix commented Dec 29, 2024

Doekin commented Dec 29, 2024 • edited Loading

Doekin commented Dec 27, 2024 •

edited

Loading

Doekin Dec 29, 2024 •

edited

Loading

Doekin Dec 30, 2024 •

edited

Loading

Doekin commented Dec 29, 2024 •

edited

Loading

Doekin commented Dec 29, 2024 •

edited

Loading