[vcpkg baseline][vcpkg-build-make] Use the same MSYS_ROOT in configure and build #19325

JackBoosY · 2021-08-03T09:15:38Z

vcpkg_acquire_msys(MSYS_ROOT) in vcpkg_build_make will cause the extra msys components missing when building ports.
For example, building starlink-ast:x64-uwp:

libtool: link: warning: undefined symbols not allowed in x86_64-unknown-mingw32 shared libraries
libtool: link: warning: undefined symbols not allowed in x86_64-unknown-mingw32 shared libraries
libtool: link: warning: undefined symbols not allowed in x86_64-unknown-mingw32 shared libraries
libtool: link: warning: undefined symbols not allowed in x86_64-unknown-mingw32 shared libraries
libtool: link: warning: undefined symbols not allowed in x86_64-unknown-mingw32 shared libraries
libtool: link: warning: undefined symbols not allowed in x86_64-unknown-mingw32 shared libraries
libtool: link: warning: undefined symbols not allowed in x86_64-unknown-mingw32 shared libraries
libtool: link: warning: undefined symbols not allowed in x86_64-unknown-mingw32 shared libraries
libtool: link: warning: undefined symbols not allowed in x86_64-unknown-mingw32 shared libraries
libtool: link: warning: undefined symbols not allowed in x86_64-unknown-mingw32 shared libraries
/bin/sh: /usr/bin/perl: No such file or directory
make[1]: *** [Makefile:4510: ast.h] Error 127
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1674: all] Error 2

In starlink-ast's portfile.cmake:

vcpkg_acquire_msys(MSYS_ROOT PACKAGES make automake1.16 perl)    
vcpkg_configure_make(
...
)

vcpkg_install_make()

This will overwrite the value MSYS_ROOT and use the incorrect make.exe to build it.

This code was introduced in PR #10402.

JackBoosY · 2021-08-03T09:16:01Z

cc @Neumann-A

dg0yt · 2021-08-03T10:16:03Z

I guess this PR will break vcpkg_configure_make because the msys root lacks important tools.

Every call to vcpkg_acquire_msys() leads to a different tree for a differen set of packages. starlink-ast needs to request perl via the ADDITIONAL_MSYS_PACKAGES parameter to vcpkg_configure_make.

JackBoosY · 2021-08-04T02:05:47Z

@dg0yt That's not related to the configure step but related to the build step.

dg0yt · 2021-08-04T05:30:15Z

That's not related to the configure step but related to the build step.

Indeed. And it works, due to the default packages.

Still, this proliferates different interfaces for defining the msys environment.

ADDITIONAL_MSYS_PACKAGES parameter to vcpkg_configure_make
MSYS_ROOT variable for vcpkg_build_make

This creates two different msys roots for a single package like starlink-ast (and a third one if acquiring pkg-config).

I don't think this is the best way forward.

JackBoosY · 2021-08-04T06:33:29Z

@Neumann-A what do you think about?

JackBoosY · 2021-08-04T06:51:53Z

That's not related to the configure step but related to the build step.

Indeed. And it works, due to the default packages.

Still, this proliferates different interfaces for defining the msys environment.

ADDITIONAL_MSYS_PACKAGES parameter to vcpkg_configure_make

MSYS_ROOT variable for vcpkg_build_make

This creates two different msys roots for a single package like starlink-ast (and a third one if acquiring pkg-config).

I don't think this is the best way forward.

Yes, the vcpkg_acquire_msys before vcpkg_configure_make needs to be deleted and the parameters should be passed to vcpkg_configure_make, and the same MSYS_ROOT is used in vcpkg_build_make to solve the problem of inconsistent configuration and build using MSYS2 path.

…igure and build.

github-actions

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 a863c84812089836a3c0f2f215ab3e2579fc8acf -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 8cadbc5..eb0a367 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -6086,7 +6086,7 @@
     },
     "starlink-ast": {
       "baseline": "9.2.4",
-      "port-version": 0
+      "port-version": 1
     },
     "status-code": {
       "baseline": "1.0.0-ab3cd821",
diff --git a/versions/s-/starlink-ast.json b/versions/s-/starlink-ast.json
index f357f55..a724950 100644
--- a/versions/s-/starlink-ast.json
+++ b/versions/s-/starlink-ast.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "2fecd468269d73b6e8f29a297c4f7db771ea37c4",
+      "version-semver": "9.2.4",
+      "port-version": 1
+    },
     {
       "git-tree": "50f0b71ca66bd9e4ce6cb5a153e25878dd7a0600",
       "version-semver": "9.2.4",

github-actions

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 a863c84812089836a3c0f2f215ab3e2579fc8acf -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 8cadbc5..eb0a367 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -6086,7 +6086,7 @@
     },
     "starlink-ast": {
       "baseline": "9.2.4",
-      "port-version": 0
+      "port-version": 1
     },
     "status-code": {
       "baseline": "1.0.0-ab3cd821",
diff --git a/versions/s-/starlink-ast.json b/versions/s-/starlink-ast.json
index f357f55..a724950 100644
--- a/versions/s-/starlink-ast.json
+++ b/versions/s-/starlink-ast.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "2fecd468269d73b6e8f29a297c4f7db771ea37c4",
+      "version-semver": "9.2.4",
+      "port-version": 1
+    },
     {
       "git-tree": "50f0b71ca66bd9e4ce6cb5a153e25878dd7a0600",
       "version-semver": "9.2.4",

dg0yt · 2021-08-04T11:21:04Z

scripts/cmake/vcpkg_configure_make.cmake

@@ -305,7 +309,7 @@ function(vcpkg_configure_make)
    # Pre-processing windows configure requirements
    if (VCPKG_TARGET_IS_WINDOWS)
        if(CMAKE_HOST_WIN32)
-            list(APPEND MSYS_REQUIRE_PACKAGES binutils libtool autoconf automake-wrapper automake1.16 m4)
+            list(APPEND MSYS_REQUIRE_PACKAGES binutils libtool autoconf automake-wrapper automake1.16 m4 make)


make is in the default packages:

vcpkg/scripts/cmake/vcpkg_acquire_msys.cmake

Lines 152 to 154 in bd5ea16

if(NOT arg_NO_DEFAULT_PACKAGES)

list(APPEND Z_VCPKG_MSYS_PACKAGES bash coreutils sed grep gawk diffutils make pkg-config)

endif()

dg0yt · 2021-08-04T11:24:54Z

scripts/cmake/vcpkg_configure_make.cmake

@@ -866,4 +870,7 @@ function(vcpkg_configure_make)

    SET(_VCPKG_PROJECT_SOURCE_PATH ${_csc_SOURCE_PATH} PARENT_SCOPE)
    set(_VCPKG_PROJECT_SUBPATH ${_csc_PROJECT_SUBPATH} PARENT_SCOPE)
+    if (MSYS_ROOT)
+        set(MSYS_ROOT ${MSYS_ROOT} PARENT_SCOPE)


This adds the side effect of changing MSYS_ROOT in the parent scope without user control. I don't think this is desirable. Either the scripts interact via Z_-prefixed variables (it is an implementation detail), or the variable name must be taken from yet another parameter.

dg0yt · 2021-08-04T16:32:38Z

I implemented passing of the make tool (from MSYS_ROOT or system) from vcpkg_configure_make to vcpkg_build_make in #19361, and CI already succeeded for x64-uwp, building gettext[core]:x64-uwp, (host) gettext[core,tools]:x64-windows, and many other ports
That PR took inspiration from this PR here. The parallel work was only meant to speed up resolving the urgent make/gettext issue. Pick what seems more useful.

JackBoosY · 2021-08-05T07:50:01Z

Close this PR in a favor of #19361.

[vcpkg-build-make] Do not find msys when MSYS_ROOT has value

27019ab

JackBoosY added info:internal This PR or Issue was filed by the vcpkg team. category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly labels Aug 3, 2021

JackBoosY assigned PhoebeHui Aug 3, 2021

github-actions bot approved these changes Aug 3, 2021

View reviewed changes

JackBoosY mentioned this pull request Aug 4, 2021

[gettext] build failure #19337

Closed

JackBoosY linked an issue Aug 4, 2021 that may be closed by this pull request

[gettext] build failure #19337

Closed

JackBoosY added 2 commits August 4, 2021 00:39

Add ADDITIONAL_MSYS_PACKAGES document, use the same MSYS_ROOT in conf…

acf7e59

…igure and build.

[starlink-ast] Add ADDITIONAL_MSYS_PACKAGES to vcpkg_configure_make

b4d8729

JackBoosY changed the title ~~[vcpkg baseline][vcpkg-build-make] Do not find msys when MSYS_ROOT has value~~ [vcpkg baseline][vcpkg-build-make] Use the same MSYS_ROOT in configure and build Aug 4, 2021

github-actions bot requested changes Aug 4, 2021

View reviewed changes

update doc

053ca32

github-actions bot requested changes Aug 4, 2021

View reviewed changes

JackBoosY mentioned this pull request Aug 4, 2021

[gettext] build failure #18270

Closed

JackBoosY added the info:world-rebuild label Aug 4, 2021

dg0yt reviewed Aug 4, 2021

View reviewed changes

JackBoosY mentioned this pull request Aug 5, 2021

[mpfr] x64-windows build failure #19353

Closed

JackBoosY linked an issue Aug 5, 2021 that may be closed by this pull request

[mpfr] x64-windows build failure #19353

Closed

JackBoosY mentioned this pull request Aug 5, 2021

[vcpkg baseline][vcpkg_configure_make] Pass make tool to vcpkg_build_make #19361

Merged

JackBoosY closed this Aug 5, 2021

JackBoosY deleted the dev/jack/fix-vcpkg-build-make branch May 26, 2022 07:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vcpkg baseline][vcpkg-build-make] Use the same MSYS_ROOT in configure and build #19325

[vcpkg baseline][vcpkg-build-make] Use the same MSYS_ROOT in configure and build #19325

JackBoosY commented Aug 3, 2021 •

edited

Loading

JackBoosY commented Aug 3, 2021

dg0yt commented Aug 3, 2021

JackBoosY commented Aug 4, 2021

dg0yt commented Aug 4, 2021

JackBoosY commented Aug 4, 2021

JackBoosY commented Aug 4, 2021

github-actions bot left a comment

github-actions bot left a comment

dg0yt Aug 4, 2021

dg0yt Aug 4, 2021

dg0yt commented Aug 4, 2021

JackBoosY commented Aug 5, 2021

	if(NOT arg_NO_DEFAULT_PACKAGES)
	list(APPEND Z_VCPKG_MSYS_PACKAGES bash coreutils sed grep gawk diffutils make pkg-config)
	endif()

[vcpkg baseline][vcpkg-build-make] Use the same MSYS_ROOT in configure and build #19325

[vcpkg baseline][vcpkg-build-make] Use the same MSYS_ROOT in configure and build #19325

Conversation

JackBoosY commented Aug 3, 2021 • edited Loading

JackBoosY commented Aug 3, 2021

dg0yt commented Aug 3, 2021

JackBoosY commented Aug 4, 2021

dg0yt commented Aug 4, 2021

JackBoosY commented Aug 4, 2021

JackBoosY commented Aug 4, 2021

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

dg0yt Aug 4, 2021

Choose a reason for hiding this comment

dg0yt Aug 4, 2021

Choose a reason for hiding this comment

dg0yt commented Aug 4, 2021

JackBoosY commented Aug 5, 2021

JackBoosY commented Aug 3, 2021 •

edited

Loading