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

Add windows-toolchain input #563

Merged
merged 1 commit into from
Jan 22, 2024
Merged

Add windows-toolchain input #563

merged 1 commit into from
Jan 22, 2024

Conversation

MSP-Greg
Copy link
Collaborator

@MSP-Greg MSP-Greg commented Jan 17, 2024

Adds a windows-toolchain input.

  1. Only affects Windows Rubies.
  2. Current PR allows only one optional setting, which is none.  It installs the selected Ruby and adds it to PATH. No build tools are installed/activated. Hence, no extension gems can be compiled/installed via bundler.

Closes #562

@k0kubun
Copy link
Member

k0kubun commented Jan 17, 2024

Thanks!

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.

The general functionality looks fine but I don't understand in which cases it has effect or not, and the input name seems too confusing.

action.yml Outdated
Comment on lines 39 to 43
win-platform:
description: |
Allows Windows platform setup to be independent of RUBY_PLATFORM. At present, the only allowed
setting is "none", which only adds Ruby to PATH. No build tools are installed, nor are any ENV setting
changed to activate them.
Copy link
Member

@eregon eregon Jan 17, 2024

Choose a reason for hiding this comment

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

The name of the option and this description is confusing me.
Is the effect whether to install MSYS2 or not? If so msys2: true/false seems clearer.
Or maybe a more general term like setup-toolchain: true/false (or maybe something with windows in the name since it only applies on Windows).

Also what does this do for RubyInstaller2 builds (both release and head), is the native toolchain there always provided and the option has no effect? So then the option only affects the mswin & ucrt builds?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe a better name would be win-platform-override?

As mentioned, I'd like to add additional values like mingw, ucrt, and mswin, but I don't have time to write those now.

The input allows one to override the installation of build tools and packages, which currently is based on the installed/activated Ruby. So, one could install/activate a Windows ucrt Ruby, but install mswin tools and packages.

For now, the only available value is none, which doesn't install any build tools or packages.

What @k0kubun has asked for is the ability to install/activate a Ruby specified by ruby-version, but nothing else.

If you look at the current ruby/ruby CI build scripts, they install all the build tools and packages. They'd prefer to do so, as it allows ruby/ruby committers to make changes without affecting anything here.

The main application of this input would be building Ruby. For instance, in ruby-loco, I use this to install build tools/packages. But, that means I have to activate a Ruby that matches the platform of the Ruby being built. That's a 'dependency' that I'd prefer not to have.

Almost all users of this action should not use the new input.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe a better name would be win-platform-override?

I feel weird about using the term platform for this.
My understanding of platform (and RUBY_PLATFORM) is it's basically $ARCH-$OS[$OSVERSION].
It's true that on Windows the values of RUBY_PLATFORM are more varied: x64-mingw-ucrt, x64-mingw32, x64-mswin64_140.

But RUBY_PLATFORM at least to me sounds like "the platform on which that ruby was built". And there is no way to override or change that for an existing binary.

I think toolchain seems a better term for this, at least that's the term I know on Linux & macOS for "which C compiler, libc, etc is available/in PATH" (e.g. a LLVM 15.0.7 toolchain (which e.g. might be configured to use libc++ instead libstdc++), a gcc 12.3.1 toolchain using glibc).
BTW there is also musl toolchains vs glibc toolchains on Linux (more precisely, toolchains can use one of these libc's), that seems very similar to the ucrt/mingw/mswin situation.

I would say for instance that there is toolchain to build Ruby and a toolchain to build Ruby native extensions and they don't have to be the same.

@MSP-Greg Do you think the name windows-toolchain: default/none (and later adding mingw/ucrt/mswin if usefl) would make sense? i.e. is "toolchain" a meaningful term on Windows too and appropriate for this?

Or maybe devkit as the input name? WDYT?


I have tried looking up some of the websites to see if they talk about toolchain or something that sounds like it.
It seems to match:

https://rubyinstaller.org/downloads/ names it Devkit (I also found the term "development environment" used).
Also from that page:

With Development Kit?
RubyInstaller uses the MSYS2 toolchain as its development kit.
...
[...] with the rich UNIX toolset of MSYS2 and the large repository of MINGW libraries.

devkit could also be a good name I think.
https://en.wikipedia.org/wiki/Software_development_kit fits rather well.

https://www.msys2.org/

MSYS2 is a collection of tools and libraries ...

Sounds like a toolchain to me.

https://learn.microsoft.com/en-us/cpp/windows/universal-crt-deployment?view=msvc-170

From Visual Studio .NET through Visual Studio 2013, each major release of the C++ compiler and tools has included a new, standalone version of the Microsoft C Runtime (CRT) library.

So one way to see that is MSVC is a toolchain and somewhat part of that toolchain (or a dependency of it) is the CRT (basically the libc).

https://gnutoolchains.com/ (a random website I found while googling) seems to use the term toolchain for Windows too.

@MSP-Greg MSP-Greg changed the title Add win-platform input Add win-ruby-platform-override input Jan 17, 2024
@MSP-Greg
Copy link
Collaborator Author

Changed input name to win-ruby-platform-override. Also updated the comments in action.yml.

@MSP-Greg
Copy link
Collaborator Author

I updated the test added to test.yml. I originally used a PowerShell script, but that's unclear for many people. Changed to a ruby command, and checked it in my fork (see https://github.com/MSP-Greg/setup-ruby/actions/runs/7560951646)

@MSP-Greg
Copy link
Collaborator Author

Although I don't recall people using Windows self-hosted runners, they may prefer to use win-ruby-platform-override: none if they have all 'build related files' pre-installed on their images...

action.yml Outdated
Comment on lines 41 to 43
This only affect Windows runners. Allows Windows platform setup to override the setup based on RUBY_PLATFORM.
At present, the only allowed setting is "none", which only adds Ruby to PATH.
No build tools are installed, nor are any ENV setting changed to activate them.
Copy link
Member

Choose a reason for hiding this comment

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

It would be great to describe what 'default' does here

win-ruby-platform-override: none
bundler: none
- name: C:/msys64/mingw64/bin/gcc.exe not installed
run: ruby -e "File.exist?('C:/msys64/mingw64/bin/gcc.exe') && exit!(1)"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
run: ruby -e "File.exist?('C:/msys64/mingw64/bin/gcc.exe') && exit!(1)"
run: ruby -e "abort if File.exist?('C:/msys64/mingw64/bin/gcc.exe')"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Never noticed abort, I'll change.

@eregon
Copy link
Member

eregon commented Jan 18, 2024

I understand the functionality well know, thanks for your comments.
The main thing left is finding a good name for this input.

@MSP-Greg
Copy link
Collaborator Author

MSP-Greg commented Jan 18, 2024

Do you think the name windows-toolchain: default/none (and later adding mingw/ucrt/mswin if usefl) would make sense? i.e. is "toolchain" a meaningful term on Windows too and appropriate for this?

That's fine by me, could also be windows-toolchain-override, which makes clear that it's changing something.

Which would you prefer, windows-toolchain or windows-toolchain-override?

EDIT:

Re using the word devkit, that's always been about MSYS2 toolchains, some people might not even be aware of mswin...

I understand the functionality well now, thanks for your comments.

Glad that helped. Apologies in advance for Windows explanations. I wrote plenty of code on MSFT DOS, sometimes my Windows explanations are too brief, sometimes too thorough...

@eregon
Copy link
Member

eregon commented Jan 18, 2024

Let's go with windows-toolchain. I think the "override" part is implied by not using the input's default value.

@MSP-Greg
Copy link
Collaborator Author

MSP-Greg commented Jan 18, 2024

Before I spin up a gazillion jobs, how's the below for action.yml?

  windows-toolchain:
    description: |
      This only affects Windows runners.
      The default setting ('default') installs a toolchain based on the installed/activated Ruby.
      This allows Windows platform setup to override the default toolchain setup.
      At present, the only allowed setting is 'none', which only adds Ruby to PATH.
      No build tools are installed, nor are any ENV setting changed to activate them.
    default: 'default'

@MSP-Greg MSP-Greg changed the title Add win-ruby-platform-override input Add windows-toolchain input Jan 18, 2024
@eregon
Copy link
Member

eregon commented Jan 19, 2024

Sounds good. I'm thinking to take this opportunity to document a bit better what the default does:

  windows-toolchain:
    description: |
      This input allows to override the default toolchain setup on Windows.
      The default setting ('default') installs a toolchain based on the selected Ruby.
      Specifically it installs MSYS2 if not already there and installs mingw/ucrt/mswin build tools.
      It also sets environment variables using ridk if that's shipped with that Ruby.
      At present, the only other setting than 'default' is 'none', which only adds Ruby to PATH.
      No build tools are installed, nor are any ENV setting changed to activate them.
    default: 'default'

@eregon
Copy link
Member

eregon commented Jan 19, 2024

It also sets environment variables using ridk if that's shipped with that Ruby.

Do you know when ridk is present, is it always there for RubyInstaller builds?
And so it's not there for mingw/ucrt/mswin?

@MSP-Greg
Copy link
Collaborator Author

How about the following, I changed two things -

  1. 'mingw/ucrt/mswin build tools' to 'mingw/ucrt/mswin build tools and packages'
  2. 'ridk if that's shipped with that Ruby.' to ''ridk' or 'vcvars64.bat' based on the selected Ruby.'
  windows-toolchain:
    description: |
      This input allows to override the default toolchain setup on Windows.
      The default setting ('default') installs a toolchain based on the selected Ruby.
      Specifically, it installs MSYS2 if not already there and installs mingw/ucrt/mswin build tools and packages.
      It also sets environment variables using 'ridk' or 'vcvars64.bat' based on the selected Ruby.
      At present, the only other setting than 'default' is 'none', which only adds Ruby to PATH.
      No build tools are installed, nor are any ENV setting changed to activate them.
    default: 'default'

Thoughts? Note that it also uses an openssl package compatible with the selected Ruby.

@MSP-Greg
Copy link
Collaborator Author

Pushed the changes as above...

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.

Thank you!

It also sets environment variables using 'ridk' or 'vcvars64.bat' based on the selected Ruby.
At present, the only other setting than 'default' is 'none', which only adds Ruby to PATH.
No build tools or packages are installed, nor are any ENV setting changed to activate them.
default: 'default'
Copy link
Member

Choose a reason for hiding this comment

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

Having the default here is nice documentation-wise, but OTOH it has the side effect to always show this input (e.g. https://github.com/ruby/setup-ruby/actions/runs/7586102328/job/20663507608?pr=563#step:3:5) even though this input is likely to be used by very few.
So I'll remove this line to be consistent with other "non-required inputs".

@eregon eregon merged commit 5580575 into ruby:master Jan 22, 2024
166 of 168 checks passed
@eregon
Copy link
Member

eregon commented Jan 22, 2024

yolanv pushed a commit to Ordina-Group/JWorks-blog that referenced this pull request Feb 12, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [ruby/setup-ruby](https://togithub.com/ruby/setup-ruby) | action |
minor | `v1.167.0` -> `v1.171.0` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>ruby/setup-ruby (ruby/setup-ruby)</summary>

###
[`v1.171.0`](https://togithub.com/ruby/setup-ruby/releases/tag/v1.171.0):
Add support for macos-14 GitHub runners

[Compare
Source](https://togithub.com/ruby/setup-ruby/compare/v1.170.0...v1.171.0)

#### What's Changed

- Update url to supported by
[@&#8203;LaStrada](https://togithub.com/LaStrada) in
[ruby/setup-ruby#569
- Add support for macos-14 GitHub runners by
[@&#8203;ntkme](https://togithub.com/ntkme) in
[ruby/setup-ruby#567

#### New Contributors

- [@&#8203;LaStrada](https://togithub.com/LaStrada) made their first
contribution in
[ruby/setup-ruby#569
- [@&#8203;ntkme](https://togithub.com/ntkme) made their first
contribution in
[ruby/setup-ruby#567

**Full Changelog**:
ruby/setup-ruby@v1.170.0...v1.171.0

###
[`v1.170.0`](https://togithub.com/ruby/setup-ruby/releases/tag/v1.170.0):
Add CRuby 3.2.3 on Windows

[Compare
Source](https://togithub.com/ruby/setup-ruby/compare/v1.169.0...v1.170.0)

#### What's Changed

- Update CRuby releases on Windows by
[@&#8203;ruby-builder-bot](https://togithub.com/ruby-builder-bot) in
[ruby/setup-ruby#566

**Full Changelog**:
ruby/setup-ruby@v1.169.0...v1.170.0

###
[`v1.169.0`](https://togithub.com/ruby/setup-ruby/releases/tag/v1.169.0):
Add windows-toolchain input

[Compare
Source](https://togithub.com/ruby/setup-ruby/compare/v1.168.0...v1.169.0)

#### What's Changed

- Add windows-toolchain input by
[@&#8203;MSP-Greg](https://togithub.com/MSP-Greg) in
[ruby/setup-ruby#563

**Full Changelog**:
ruby/setup-ruby@v1.168.0...v1.169.0

###
[`v1.168.0`](https://togithub.com/ruby/setup-ruby/releases/tag/v1.168.0):
Add ruby-3.2.3

[Compare
Source](https://togithub.com/ruby/setup-ruby/compare/v1.167.0...v1.168.0)

#### What's Changed

- Add ruby-3.2.3 by
[@&#8203;ruby-builder-bot](https://togithub.com/ruby-builder-bot) in
[ruby/setup-ruby#565

**Full Changelog**:
ruby/setup-ruby@v1.167.0...v1.168.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 10pm every weekday,before 5am
every weekday,every weekend" in timezone Europe/Brussels, Automerge - At
any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/Ordina-Group/ordina-jworks.github.io).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNzMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE3My4wIiwidGFyZ2V0QnJhbmNoIjoic291cmNlIn0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants