Skip to content

[openssl] fix cross compilation with mingw#17246

Merged
vicroms merged 8 commits intomicrosoft:masterfrom
d12fk:openssl-mingw-windres
Oct 16, 2021
Merged

[openssl] fix cross compilation with mingw#17246
vicroms merged 8 commits intomicrosoft:masterfrom
d12fk:openssl-mingw-windres

Conversation

@d12fk
Copy link
Contributor

@d12fk d12fk commented Apr 12, 2021

When cross compiling openssl on Linux, e.g.

./vcpkg install openssl:x64-mingw-dynamic

failed, because 'windres' was not found. The call to
openssl's Configure was missing the WINDRES environment
variable, which is needed in case it is prefixed. On
Ubuntu the binutils-mingw-w64-x86-64 package installed
it as /usr/bin/x86_64-w64-mingw32-windres.

Since mingw uses openssl-unix to build, it's worth mentioning
that the WINDRES env has no impact for non-mingw and is ignored
there.

@ghost
Copy link

ghost commented Apr 12, 2021

CLA assistant check
All CLA requirements met.

@NancyLi1013 NancyLi1013 self-assigned this Apr 13, 2021
@NancyLi1013 NancyLi1013 added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. labels Apr 13, 2021
@NancyLi1013
Copy link
Contributor

Hi @d12fk

Thanks for your PR.

Could you please run vcpkg x-add-version openssl to update the version files?

@d12fk
Copy link
Contributor Author

d12fk commented Apr 13, 2021

Hi @d12fk

Thanks for your PR.

Could you please run vcpkg x-add-version openssl to update the version files?

Indeed, forgot to add them to the commit. Will update right away.

@d12fk d12fk force-pushed the openssl-mingw-windres branch from dc6dadd to 5e9d796 Compare April 13, 2021 01:50
@d12fk d12fk requested a review from NancyLi1013 April 13, 2021 02:15
@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013
Copy link
Contributor

The failures caused by popsift will be fixed by #17277.

@NancyLi1013 NancyLi1013 added the depends:different-pr This PR or Issue depends on a PR which has been filed label Apr 14, 2021
@NancyLi1013
Copy link
Contributor

/azp run

@NancyLi1013 NancyLi1013 removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Apr 15, 2021
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013
Copy link
Contributor

LGTM now, thanks for your fixing @d12fk.

@NancyLi1013 NancyLi1013 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Apr 16, 2021
@strega-nil-ms strega-nil-ms force-pushed the openssl-mingw-windres branch from 5e9d796 to d920e67 Compare April 29, 2021 23:41
Copy link
Contributor

@longnguyen2004 longnguyen2004 left a comment

Choose a reason for hiding this comment

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

LGTM. Let's get this merged soon.
@d12fk Please resolve the merge conflicts.

When cross compiling openssl on Linux, e.g.

  ./vcpkg install openssl:x64-mingw-dynamic

failed, because 'windres' was not found. The call to
openssl's Configure was missing the WINDRES environment
variable, which is needed in case it is prefixed. On
Ubuntu the binutils-mingw-w64-x86-64 package installed
it as /usr/bin/x86_64-w64-mingw32-windres.

Since mingw uses openssl-unix to build, it's worth mentioning
that the WINDRES env has no impact for non-mingw and is ignored
there.
@strega-nil-ms strega-nil-ms force-pushed the openssl-mingw-windres branch from d920e67 to 0804a21 Compare May 1, 2021 17:00
@strega-nil-ms
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013
Copy link
Contributor

@d12fk

Could you please rebase to solve the conflicts?

@NancyLi1013 NancyLi1013 removed the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label May 7, 2021
NancyLi1013 added 3 commits July 20, 2021 01:27
…ssl-mingw-windres

# Conflicts:
#	ports/openssl/CONTROL
#	ports/openssl/unix/CMakeLists.txt
#	versions/baseline.json
#	versions/o-/openssl.json
@longnguyen2004
Copy link
Contributor

@d12fk Can you test #18554? It seems to be fixed there

@PhoebeHui
Copy link
Contributor

@d12fk, could you resolve the conflicts?

@NancyLi1013
Copy link
Contributor

@d12fk Is work still being done for this PR?

@dg0yt
Copy link
Contributor

dg0yt commented Sep 30, 2021

@NancyLi1013 The fix is still needed. A merge conflict exists only in the versions. Can't you just push the version updates?

NancyLi1013 added 2 commits October 8, 2021 01:34
…ssl-mingw-windres

# Conflicts:
#	ports/openssl/vcpkg.json
#	versions/baseline.json
#	versions/o-/openssl.json
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 973a7d517c09c8cfb7e6a548fcc260ca34ba7b60 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/o-/openssl.json b/versions/o-/openssl.json
index caf39581..69a54b37 100644
--- a/versions/o-/openssl.json
+++ b/versions/o-/openssl.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "d5310e60291032390ab8c893794f6044a7b9ae04",
+      "version-string": "1.1.1l",
+      "port-version": 2
+    },
     {
       "git-tree": "6d19a647704efae9398b178a0012140c1f1ee8b8",
       "version-string": "1.1.1l",

@NancyLi1013
Copy link
Contributor

@NancyLi1013 The fix is still needed. A merge conflict exists only in the versions. Can't you just push the version updates?

Thanks for your confirmation and reminder. @d12fk

I have merged master and updated versions.

@NancyLi1013 NancyLi1013 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Oct 15, 2021
@vicroms vicroms merged commit b28d77a into microsoft:master Oct 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants