ruby / setup-ruby

An action to download a prebuilt Ruby and add it to the PATH in 5 seconds
https://github.com/marketplace/actions/setup-ruby-jruby-and-truffleruby
MIT License
788 stars 255 forks source link

Add ridk option to disable ridk env variables #562

Closed k0kubun closed 8 months ago

k0kubun commented 8 months ago

I tried to use ruby/setup-ruby to set up PATH in mswin build on ruby/ruby https://github.com/ruby/ruby/pull/9566, but ridk environment variables added by it seem to break the build:

Run ../src/win32/configure.bat --disable-install-doc --with-opt-dir=C:/vcpkg/installed/x64-windows
../src/win32/setup.mak:3: *** missing separator.  Stop.
NMAKE : fatal error U1077: 'C:\msys64\usr\bin\make.exe' : return code '0x2'
Stop.

Since only PATH is needed for the mswin build and adding ridk environment variables only breaks it, I'd like to add an option to disable those environment variables. This PR proposes to add it as ridk: none.

MSP-Greg commented 8 months ago

@k0kubun

Question - Something similar I've thought about would be an input like win_platform, it's main use would be win_platform: mswin. You could load any Windows Ruby (all but mswin are MSY2 gcc), but it would use the setup that is used for the mswin build?

It would add all the ENV settings needed for an mswin build, and also install the pre-compiled MSFT/vpkg archive.

Several ruby org 'extension' repos are using the same logic when they run CI with mswin.

EDIT: we could also add win_platform: ucrt, which would setup MSYS2 'UCRT64' regardless of the Ruby installed...

k0kubun commented 8 months ago

I'm not interested in setting up environment variables for mswin build using ruby/setup-ruby because Ruby committers don't have write access to ruby/setup-ruby and doing so in ruby/setup-ruby only makes it harder for us to fix broken builds like this time. As long as your proposed solution also allows win_platform: none, I'm fine with that too. I just need a way to modify only PATH.

MSP-Greg commented 8 months ago

because Ruby committers don't have write access to ruby/setup-ruby

Well, it would seem that a discussion with @eregon might be a good idea.

I just need a way to modify only PATH

Not sure what you mean. Do you only want ruby/setup-ruby to add the selected Ruby to PATH?

k0kubun commented 8 months ago

Do you only want ruby/setup-ruby to add the selected Ruby to PATH?

Exactly. I want to write:

      - uses: ruby/setup-ruby@v1
        with:
          ruby-version: '2.7'
          bundler: none
          ridk: none # or win_platform: none

and let the action execute only "Modifying Path" (and "Print Ruby version"). For our use case, we want no other environment variables.

We currently hard-code C:\hostedtoolcache\windows\Ruby\2.7.8\x64\bin to choose the Ruby version. It's fine for Ruby 2.7 since it's past EOL, but once we upgrade the version to, say, 3.2.2, it would be broken when 3.2.3 comes out. I want to resolve a version cached by GitHub Actions using this.

MSP-Greg commented 8 months ago

I believe PR #563 adds the functionality you've described.

See https://github.com/MSP-Greg/setup-ruby/actions/runs/7550784437/job/20556972770#step:3:17

k0kubun commented 8 months ago

That looks good. Thank you for working on that. Could you merge and/or release that one?

MSP-Greg commented 8 months ago

You're welcome.

As you know, ruby/setup-ruby installs both MSYS2 toolchains and packages and also MSFT/vcpkg packages. At some point, I'd like to add options mingw, ucrt, and mswin to the input.

As it is now, it works well for 'normal' CI, where the Ruby that's being installed/activated has the build tools needed to compile extension code for itself (eg Nokogiri, Puma, Ruby bundled extension gems, etc).

But, if one wants to build Ruby, one has to install a Ruby that matches the one being built, which is the problem you're having.

JFYI, ruby-loco uses ruby/setup-ruby for the build tools and package installation. I figured I should 'walk the talk', and it's a good test of ruby/setup-ruby. Sorry for mentioning 'third party software'...

Could you merge and/or release that one?

I don't have commit rights. I'm sure @eregon can review it soon.

eregon commented 8 months ago

As you know, ruby/setup-ruby installs both MSYS2 toolchains and packages and also MSFT/vcpkg packages. At some point, I'd like to add options mingw, ucrt, and mswin to the input.

Does it make sense to pick a subset of those in practice? I guess it's not a good idea to mix e.g. mswin and mingw stuff, right?

Maybe something like extras: all|none|some-specific-ones could be good, but we would need to understand which combinations are useful then.

eregon commented 8 months ago

because Ruby committers don't have write access to ruby/setup-ruby

We need a proper review for PRs to setup-ruby (e.g. no direct push), because e.g. any input added can basically never be removed without bumping the major version and that's a high cost.

MSP-Greg commented 8 months ago

Does it make sense to pick a subset of those in practice?

The code to allow one or the code to allow all is not much different. But, it will take time I don't have right now. So, just setup none.

I guess it's not a good idea to mix e.g. mswin and mingw stuff, right?

It's not quite that simple. The Windows files installed here can be categorized as 'bash tools', 'build tools/compilers' and 'packages'. 'bash tools' are needed/used by all Windows Ruby platforms (RUBY_PLATFORM), 'build tools/compilers' and 'packages' are platform dependent.

Maybe something like extras: all|none|some-specific-ones could be good, but we would need to understand which combinations are useful then.

extras: is about as vague as it gets. I think win-ruby-platform-override would be a better choice, as it's pretty clear as to what it does? Or, win means Windows, ruby-platform is RUBY_PLATFORM, and override is dangerous.