Skip to content

Rewrite ruby/setup-msys2-gcc #762

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

Merged
merged 3 commits into from
May 16, 2025
Merged

Rewrite ruby/setup-msys2-gcc #762

merged 3 commits into from
May 16, 2025

Conversation

ntkme
Copy link
Contributor

@ntkme ntkme commented May 4, 2025

This PR include a few changes for windows:

  • Fix the issue with gcc 15/14 and mingw-w64 r688/r720 by introducing flexible variants of toolchains.
  • Reduce friction with Rewrite setup-msys2-gcc#26.
  • Improves performance by install to fast disk and create junction link on slow disk.
    • Ruby is installed to $RUNNER_TEMP\$base and a junction link is created at original $rubyPrefix if $rubyPrefix is on the slow disk.
    • MSYS2 is installed to $RUNNER_TEMP\msys64 and a junction link is created at $rubyPrefix\msys64.
  • Improves the reliability of ridk enable step by using proper serialization.
  • Refactor and clean up tech debt.

Performance

Run to run improvement varies a lot due to different sizes of packages and the unstable performance of the slow disk.
However, in general this saves anywhere from 20s to 60s per run. Example of a performance improvement from 63.43s to 27.15s - or 36.38s in saving:

https://github.com/ruby/setup-ruby/actions/runs/14772696897/job/41475457782

Downloading Ruby
  https://github.com/oneclick/rubyinstaller2/releases/download/RubyInstaller-3.4.3-1/rubyinstaller-3.4.3-1-x64.7z
  Took   0.97 seconds
Extracting  Ruby
  Took  25.38 seconds
Modifying Path
Downloading msys2 build tools
  https://github.com/ruby/setup-msys2-gcc/releases/download/msys2-gcc-pkgs/msys2.7z
  Took   0.73 seconds
Extracting  msys2 build tools
  Took  12.52 seconds
Downloading ucrt64-3.0 build tools
  https://github.com/ruby/setup-msys2-gcc/releases/download/msys2-gcc-pkgs/ucrt64-3.0.7z
  Took   1.00 seconds
Extracting  ucrt64-3.0 build tools
  Took  22.83 seconds

https://github.com/ntkme/setup-ruby/actions/runs/14823150160/job/41612838924

Downloading Ruby
  https://github.com/oneclick/rubyinstaller2/releases/download/RubyInstaller-3.4.3-1/rubyinstaller-3.4.3-1-x64.7z
  Took   0.46 seconds
Extracting  Ruby
  Took   9.87 seconds
Modifying Path
Downloading msys2 build tools
  https://github.com/ntkme/setup-msys2-gcc/releases/latest/download/msys2-ucrt64-gcc@14.7z
  Took   1.18 seconds
Extracting  msys2 build tools
  Took  15.64 seconds

Note regarding why use $rubyPrefix\msys64:

>=2.4.1 means $rubyPrefix\msys64 is supported by all currently available ruby installer versions that's linked with msys2, and it has higher priority than C:\msys64 so that we don’t need to touch the preinstalled C:\msys64 at all.

@ntkme ntkme force-pushed the dev branch 10 times, most recently from 42e2c97 to ad6e0db Compare May 8, 2025 01:51
@ntkme ntkme marked this pull request as ready for review May 8, 2025 02:37
@ntkme ntkme force-pushed the dev branch 3 times, most recently from 363de67 to a8d7546 Compare May 8, 2025 05:50
@ntkme ntkme mentioned this pull request May 8, 2025
@ntkme ntkme force-pushed the dev branch 8 times, most recently from 15a109c to 7168d8b Compare May 8, 2025 21:06
@MSP-Greg
Copy link
Collaborator

Curious, why the need for 'Revert back to use GITHUB_WORKSPACE to detect fast drive'?

@ntkme
Copy link
Contributor Author

ntkme commented May 12, 2025

Curious, why the need for 'Revert back to use GITHUB_WORKSPACE to detect fast drive'?

Sorry, was just testing something and pushed to the wrong branch.

@MSP-Greg
Copy link
Collaborator

@eregon

I've quickly looked as this, and everything seems ok. We're getting close to finalizing the changes to setup-msys2-gcc, so checking is this is okay. Once the new packages are created, this can be merged.

@eregon
Copy link
Member

eregon commented May 14, 2025

From the PR title I thought this was a ruby/setup-msys2-gcc PR 😅
I'll try to take a look soon.

@MSP-Greg
Copy link
Collaborator

Re setup-msys2-gcc:

Much of the Windows' code was written when GHA was using 2016 & 2019 images. There have been a lot of changes in the images since then.

The only MSYS compatibility issue came up with the upgrade from OpenSSL 1.1 to 3.0. At the time, that only effected Ruby major.minor versions.

I was thinking about how to restructure it, and I wanted to move to a more data/configuration centric setup (using yaml, json, etc) vs the current 'method centric' code.

Also, recently two updates to MSYS2 have made things messy, and they require resolving to the Ruby patch level.

Anyway, @ntkme grabbed it and updated the code, and we've been testing it and making small changes. Alot more information has moved to the workflow file, along with the build tools package info and Ruby version constraints moving to windows-toolchain.json.

It will make it much easier to quickly update things when MSYS2 issue arise. The information re which tool-chain to install has always been located in this repo, and now it's retrieved via a json file.

Copy link
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

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

This looks great, thank you for your work on this!

I guess the order is merge ruby/setup-msys2-gcc#26, create the release & assets, then update this PR to use ruby/setup-msys2-gcc links, and merge this PR?

@ntkme
Copy link
Contributor Author

ntkme commented May 15, 2025

I guess the order is merge ruby/setup-msys2-gcc#26, create the release & assets, then update this PR to use ruby/setup-msys2-gcc links, and merge this PR?

Correct.

@MSP-Greg
Copy link
Collaborator

@eregon

I guess the order is merge ruby/setup-msys2-gcc#26, create the release & assets, then update this PR to use ruby/setup-msys2-gcc links, and merge this PR?

The PR has been merged in https://github.com/ruby/setup-msys2-gcc

@ntkme
Copy link
Contributor Author

ntkme commented May 15, 2025

@eregon Release artifacts have been uploaded and I've updated this PR to use ruby/setup-msys2-gcc. There is a truffleruby test that's failing but it's unrelated to my change.

@ronaldtse
Copy link

Hoping this gets merged soon as we have a number of release pipelines blocked due to the recent Ruby 3.1 on Windows issue. Thank you for the excellent work and maintenance! 🙇‍♂️

@eregon eregon merged commit 1a0ff44 into ruby:master May 16, 2025
207 of 208 checks passed
@ntkme ntkme deleted the dev branch May 16, 2025 15:00
@ntkme ntkme restored the dev branch May 16, 2025 15:15
@eregon
Copy link
Member

eregon commented May 28, 2025

@ntkme I think this is causing a spec to fail in ruby/spec, see
https://github.com/ruby/spec/actions/runs/15306688120/job/43061100999?pr=1268

Expected ["D:/a/spec/mspec/lib",
 "C:/hostedtoolcache/windows/Ruby/3.4.2/x64/lib/ruby/site_ruby/3.4.0",
 "C:/hostedtoolcache/windows/Ruby/3.4.2/x64/lib/ruby/site_ruby/3.4.0/x64-ucrt",
 "C:/hostedtoolcache/windows/Ruby/3.4.2/x64/lib/ruby/site_ruby",
 "C:/hostedtoolcache/windows/Ruby/3.4.2/x64/lib/ruby/vendor_ruby/3.4.0",
 "C:/hostedtoolcache/windows/Ruby/3.4.2/x64/lib/ruby/vendor_ruby/3.4.0/x64-ucrt",
 "C:/hostedtoolcache/windows/Ruby/3.4.2/x64/lib/ruby/vendor_ruby",
 "C:/hostedtoolcache/windows/Ruby/3.4.2/x64/lib/ruby/3.4.0",
 "C:/hostedtoolcache/windows/Ruby/3.4.2/x64/lib/ruby/3.4.0/x64-mingw-ucrt"].include? "D:/a/_temp/rubyinstaller-3.4.2-1-x64/lib/ruby/site_ruby/3.4.0"
to be truthy but was false

Which means the value of RbConfig::CONFIG['sitelibdir'] is D:/a/_temp/rubyinstaller-3.4.2-1-x64/lib/ruby/site_ruby/3.4.0.

And also I checked with https://github.com/eregon/actions-shell/actions/runs/15309231990/job/43069588075
There we see most paths in RbConfig start with D:/a/_temp (the value of TMPDIR).
But what is added to PATH is C:\hostedtoolcache\windows\Ruby\3.4.4\x64\bin.

So then we end up with this mismatch of $LOAD_PATH using C:/hostedtoolcache/windows/Ruby/3.4.2/x64/ and RbConfig having D:/a/_temp/ values, but they should really match.

I guess a workaround would be to File.realpath each $LOAD_PATH entry for that check, but that's inconvenient/shouldn't be necessary.
IIRC it's intentional that CRuby doesn't realpath $LOAD_PATH entries.

Is there a way to fix this? I suppose it's related to the junction link.

@eregon
Copy link
Member

eregon commented May 28, 2025

A bit more debugging in https://github.com/eregon/actions-shell/actions/runs/15309439156/job/43070305903#step:4:550 shows D:/a/_temp/ paths in $" but C:/hostedtoolcache/windows/Ruby/3.4.4/x64/ in $:, which seems not unlikely to cause some incompatibility in gem test suites.
And indeed File.realpath("C:/hostedtoolcache/windows/Ruby/3.4.4/x64/lib/ruby/site_ruby/3.4.0") = "D:/a/_temp/rubyinstaller-3.4.4-2-x64/lib/ruby/site_ruby/3.4.0"

eregon added a commit to ruby/spec that referenced this pull request May 28, 2025
@ntkme
Copy link
Contributor Author

ntkme commented May 28, 2025

which seems not unlikely to cause some incompatibility in gem test suites.

Yes, I understand the junction can cause issues when some code use a mix of realpath vs absolute_path. The main reason to use junction is to let us extract files on the faster disk.

I wish the code run into the issue can be more consistent in that either to use realpath everywhere or use absolute_path everywhere but never a mix of the two, but not sure if that's a fair ask. If we want to pursue better compatibility than performance, we can easily revert the usage of junction and just extract to slow disk as before.

eregon added a commit to ruby/spec that referenced this pull request May 28, 2025
@eregon
Copy link
Member

eregon commented May 28, 2025

If we want to pursue better compatibility than performance, we can easily revert the usage of junction and just extract to slow disk as before.

Mmh the fast disk is really a huge performance gain, so I wish we didn't have to choose between both 😅
If we have to, I think we'll have to live with this caveat, vs making things very slow.

Some ideas:

  • We could just add D:/a/_temp/rubyinstaller-3.4.4-2-x64/bin to PATH (instead of C:/hostedtoolcache/windows/Ruby/3.4.2/x64/bin), and not bother about trying to put it "in the hostedtoolcache" on Windows (i.e., create C:/hostedtoolcache/windows/Ruby/3.4.2/x64), especially since anyway we don't really store it there, we just put the junction link, so that serves no real purpose. The original intent of storing in the toolcache is on self-hosted runners if the toolcache is persistent between workflow runs then it can avoid repeated downloads. It's also important to look in the toolcache if the Ruby is already installed and if so not download it, but that's fine and orthogonal.
  • Maybe there is something else than junction links on Windows which behave closer than symlink on Linux? For example on Linux if I run /path/to/symlink/bin/ruby ... it resolves the symlink fairly early and mostly avoid this issue, notably $" and $: don't have symlinks in there (at least in zsh).

Could you try the first idea?

@MSP-Greg
Copy link
Collaborator

@eregon

IIRC, Ruby head builds have always used D:\ for their PATH addition, so it should work fine. Rubies that are part of the runner image will need to stay in C:/hostedtoolcache/windows/Ruby.

I probably can't get to it tonite, tomorrow should be better...

Lastly, I also recall that there are two kinds of Windows links, may look at that also.

@ntkme
Copy link
Contributor Author

ntkme commented May 28, 2025

Lastly, I also recall that there are two kinds of Windows links, may look at that also.

Correct, there are two kinds of soft links: junction and symlink. There is also hard link but hard link can not have directory as target.

There is a good explanation about the difference here: https://superuser.com/questions/343074/directory-junction-vs-directory-symbolic-link.

The biggest concern I had at the time was that creating symlink require privileged access, while junction only need regular access.

@ntkme
Copy link
Contributor Author

ntkme commented May 28, 2025

Another idea here:

Detect if we're in self-hosted runner - If so extract into hosttoolcache (no junction) directly, otherwise extract into $RUNNER_TEMP (also no junction) directly.

@eregon
Copy link
Member

eregon commented May 28, 2025

Detect if we're in self-hosted runner - If so extract into hosttoolcache (no junction) directly, otherwise extract into $RUNNER_TEMP (also no junction) directly.

It should work, but since no one complained and probably no one uses self-hosted runners on Windows with persistent toolcache (as a note, persistent toolcaches typically have concurrency issues if multiple jobs execute concurrently for the same toolcache) let's keep it simple and do only that latter part always.
FYI detecting self-hosted runner is unfortunately not so reliable unfortunately.

@ntkme
Copy link
Contributor Author

ntkme commented May 28, 2025

FYI detecting self-hosted runner is unfortunately not so reliable unfortunately.

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/store-information-in-variables#default-environment-variables

RUNNER_ENVIRONMENT

The environment of the runner executing the job. Possible values are: github-hosted for GitHub-hosted runners provided by GitHub, and self-hosted for self-hosted runners configured by the repository owner.

@eregon
Copy link
Member

eregon commented May 28, 2025

Interesting, thanks for the link, but in reality it's more complicated than that as there are some self-hosted runners for which we can reuse prebuilt Rubies and some self-hosted runners where we can't. Anyway, simple is almost always better unless there is a need for something more complicated (I don't see one).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants