Skip to content

Use msys2 bash regardless of target and of Windows Apps - #30172

Merged
dan-shaw merged 3 commits into
microsoft:masterfrom
dg0yt:msys-host
Mar 20, 2023
Merged

Use msys2 bash regardless of target and of Windows Apps#30172
dan-shaw merged 3 commits into
microsoft:masterfrom
dg0yt:msys-host

Conversation

@dg0yt

@dg0yt dg0yt commented Mar 14, 2023

Copy link
Copy Markdown
Contributor

Moving Windows msys2 bash setup out of VCPKG_TARGET_IS_WINDOWS.
Fixes building for Android on Windows.
Together with #29966, fixes #30065.

Ensures that msys root is inserted into PATH before Windows Apps dir.
Fixes #29658, interference between msys and WSL executables. Cf. #30172 (comment)

  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.


message(DEBUG "path_list:${path_list}") # Just to have --trace-expand output

set(find_system_dirs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should probably include the Win11 apps dir. (you probably find the reference to it faster than me)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hm, not sure about "faster", but probably I could find a Win11 machine. Would this help resolve the bash -> env -> bash puzzle from #29658?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I mean you already found it ;)
Yeah that is basically the idea. Put msys before any windows related paths so that it and it tools get found.

@dg0yt dg0yt Mar 14, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This still leaves the door open for other providers: git bash, scoop, etc. And I don't use them.
There may be multiple things to watch out for along the PATH:

  • bash
  • env
  • msys2 DLL
  • sh
  • find

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't forget that PATH gets sanitized on windows. Unless vcvars pulls it in it won't be in vcpkg env.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

also cc @BillyONeal why does vcpkg even put PowerShell into the environment? What is it used for? Could vcpkg not do that? (at least not for building ports)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Powershell is used at least for app-local deployment of DLLs, i.e. for tools built by ports.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, it's due to applocal.ps1.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

that is only called in vcpkg_copy_tool_dependencies as far as i am concerned and that should have a vcpkg_find_acquire_program(POWERSHELL) instead or run directly via CMake GET_RUNTIME_DEPS.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

that is only called in vcpkg_copy_tool_dependencies as far as i am concerned and that should have a vcpkg_find_acquire_program(POWERSHELL) instead or run directly via CMake GET_RUNTIME_DEPS.

We already have plans to get rid of it, but those haven't been turned on yet: microsoft/vcpkg-tool#814

@jimwang118 jimwang118 added the category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly label Mar 14, 2023
@dg0yt
dg0yt marked this pull request as draft March 15, 2023 07:15
@dg0yt

dg0yt commented Mar 15, 2023

Copy link
Copy Markdown
Contributor Author

Currently not working. To be continued on a Windows machine...

@dg0yt
dg0yt marked this pull request as ready for review March 15, 2023 08:28
@dg0yt dg0yt changed the title Use msys2 bash regardless of target Use msys2 bash regardless of target and of Windows Apps Mar 15, 2023
@dg0yt
dg0yt marked this pull request as draft March 16, 2023 16:01
@dg0yt
dg0yt marked this pull request as ready for review March 16, 2023 16:04
@jimwang118 jimwang118 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Mar 17, 2023
@julianxhokaxhiu

Copy link
Copy Markdown
Contributor

Can confirm this fixed my issue as well on #29658

Thanks @dg0yt !

@dan-shaw
dan-shaw merged commit 66372e8 into microsoft:master Mar 20, 2023
@dg0yt
dg0yt deleted the msys-host branch March 20, 2023 07:50
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: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.

[openssl] Build error Include '[openssl] Build error [x264] build failure on x86-windows-static when WSL2 is installed

6 participants