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

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

Merged
merged 4 commits into from
Aug 7, 2021

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Aug 4, 2021

  • What does your PR fix?

    This PR amends [vcpkg_build_make] Always use msys make on win32 host #19164 which could unexpectedly change the make tool location to a second MSYS root, acquired in vcpkg_build_make, in addition to the one created in vcpkg_configure_make. The usage of different MSYS roots in configure and build will lead to different mapping of POSIX-style paths (/usr/bin etc.), possibly causing strange msys build errors such as [gettext] build failure #19337 (gettext).

    With this PR, the make tool is discovered and set as part of vcpkg_configure_make. When vcpkg_build_make is used together with vcpkg_configure_make, it no longer needs to acquire an MSYS root. However, this capability is left for compatibility with portfile which do not use vcpkg_configure_make.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    All, no.
    Note that apart from changes for windows (MSYS2), there is also a change for other systems, merging the previous find_program calls for gmake and make.

  • Does your PR follow the maintainer guide?

    yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    No ports changed.

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!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for gettext but no changes to version or port version.
-- Version: 0.21#4
-- Old SHA: b6cde01ab4095a258993eaf85eb31c1e845c64a6
-- New SHA: b3d90d5c4b8c12645cc11ec9cb0c515e96061324
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

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!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for gettext but no changes to version or port version.
-- Version: 0.21#4
-- Old SHA: b6cde01ab4095a258993eaf85eb31c1e845c64a6
-- New SHA: cb7daf9630824a6fc374cccd33249df5be905381
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@Neumann-A
Copy link
Contributor

Neumann-A commented Aug 4, 2021

@dg0yt add [skip ci] to the commit messages to deactivate the github actions spam

@dg0yt
Copy link
Contributor Author

dg0yt commented Aug 4, 2021

From x64-windows-static, edited for readability:

PATH:
C:/a/2/s/scripts/buildsystems/make_wrapper
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30037\bin\HostX64\x64
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\VC\VCPackages
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\CommonExtensions\Microsoft\TestWindow
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\CommonExtensions\Microsoft\TeamFoundation\Team Explorer
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Current\bin\Roslyn
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Team Tools\Performance Tools\x64
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Team Tools\Performance Tools
C:\Program Files (x86)\Microsoft Visual Studio\Shared\Common\VSPerfCollectionTools\vs2019\\x64
C:\Program Files (x86)\Microsoft Visual Studio\Shared\Common\VSPerfCollectionTools\vs2019
C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8 Tools\x64
C:\Program Files (x86)\HTML Help Workshop
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\devinit
C:\Program Files (x86)\Windows Kits\10\bin\10.0.19041.0\x64
C:\Program Files (x86)\Windows Kits\10\bin\x64
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\\MSBuild\Current\Bin
C:\Windows\Microsoft.NET\Framework64\v4.0.30319
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools
C:\Program Files\PowerShell\7
D:/downloads/tools/msys2/703e5aa15d402568/usr/share/automake-1.16
C:/a/2/s/scripts/buildsystems/make_wrapper
D:/downloads/tools/msys2/703e5aa15d402568/usr/bin
C:\Windows\system32
C:\Windows
C:\Windows\system32\Wbem
C:\Windows\system32\WindowsPowerShell\v1.0
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\Llvm\x64\bin
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\VC\Linux\bin\ConnectionManagerExe
D:/downloads/tools/winflexbison/0a14154bff-a8cf65db07
D:/downloads/tools/ninja/1.10.2-windows
D:/downloads/tools/ninja/1.10.2-windows'

  MAKE before PR 19164:
  'D:/downloads/tools/msys2/703e5aa15d402568/usr/bin/make.exe'

  MAKE after PR 19164:
  'D:/downloads/tools/msys2/aaafe87dcead272e/usr/bin/make.exe'

Facts:

  • There is a wild mix of \ and / paths.
  • C:/a/2/s/scripts/buildsystems/make_wrapper is listed twice.
  • vcpkg_build_make probably acquires a different MSYS_ROOT than vcpkg_configure_make. (I didn't want to touch vcpkg_build_make at this point, in order to limit the number of ports being rebuild.)
    This can be a problem when MSYS tries to map between Windows paths and /usr/bin etc.
    This can be seen as an argument for vcpkg_build_make to really use the same MSYS_ROOT as vcpkg_configure_make.

CC @JackBoosY

@dg0yt
Copy link
Contributor Author

dg0yt commented Aug 4, 2021

  • This can be seen as an argument for vcpkg_build_make to really use the same MSYS_ROOT as vcpkg_configure_make.

The other option is to set MAKE already in vcpkg_configure_make. That's how it is called...

@dg0yt dg0yt changed the title [gettext] Examining build problem (WIP) [vcpkg_configure_make] Pass make tool to vcpkg_build_make Aug 4, 2021
@dg0yt
Copy link
Contributor Author

dg0yt commented Aug 4, 2021

For discussion: Use a public global VCPKG_MAKE instead of Z_VCPKG_MAKE, and document it?
This would allow to avoid MSYS root acquisition even in portfiles which do not use vcpkg_configure_make, or allow pre-definition in custom triplets.

@dg0yt dg0yt marked this pull request as ready for review August 4, 2021 20:46
@JackBoosY JackBoosY self-assigned this Aug 5, 2021
scripts/cmake/vcpkg_build_make.cmake Outdated Show resolved Hide resolved
@JackBoosY JackBoosY added the category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly label Aug 5, 2021
@dg0yt
Copy link
Contributor Author

dg0yt commented Aug 5, 2021

With regard to OpenBSD, this PR would accept make if there is no gmake installed. IIRC this is a different flavour of make which doesn't work for all Makefiles. Maybe the former if branching has to be added again.

@JackBoosY
Copy link
Contributor

JackBoosY commented Aug 5, 2021

Can you please merge the starlink-ast part of the code in #19325 to this branch after ci test?

@dg0yt
Copy link
Contributor Author

dg0yt commented Aug 5, 2021

Can you please merge the starlink-ast part of the code in #19325 to this branch after ci test?

Okay.

  • I will wait for CI to finish. (It may take some hours. Given that some represenative runs are okay, I might perhaps start earlier?)
  • I will restore safe BSD gmake lookup, to avoid regressions which are not covered by CI.
  • I will pick the starlink-ast changes.

This was linked to issues Aug 5, 2021
@JackBoosY JackBoosY changed the title [vcpkg_configure_make] Pass make tool to vcpkg_build_make [vcpkg baseline][vcpkg_configure_make] Pass make tool to vcpkg_build_make Aug 5, 2021
@JackBoosY
Copy link
Contributor

@dg0yt Please merge starlink-ast changes when x64-windows test finished.

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 bd5ea16b97e91cb620fed0e10b7d9b3a8a943a52 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index cf5bb06..81c3a2a 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -6090,7 +6090,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
Copy link
Contributor Author

dg0yt commented Aug 5, 2021

x64_windows_static: The failing port is tcl, which doesn't use the changed scripts for windows.

D:\buildtrees\tcl\x64-windows-static-rel\win\..\libtommath\bn_s_mp_toom_sqr.c(148): fatal error C1051: program database file, 'D:\buildtrees\tcl\x64-windows-static-rel\win\vc140.pdb', has an obsolete format, delete it and recompile

@JackBoosY
Copy link
Contributor

LGTM on my side, ping @dan-shaw for review and merge this PR first.

@strega-nil-ms
Copy link
Contributor

Since this fixes the same thing as #19418, and this is a better fix, merging this instead of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mpfr] x64-windows build failure [gettext] build failure
5 participants