Add ruby-loco mingw and mswin #24

MSP-Greg · 2020-02-18T21:22:24Z

Add ruby-loco mingw and mswin
Adjust workflow for mswin - 'ridk version' & 'c extension' steps
Allow head, mingw, & mswin to be used without ruby prefix
win_names.yml_ tests the above, fails on some non Windows jobs deliberately

eregon · 2020-02-18T21:40:24Z

Thank you, I'll review this tomorrow.

MSP-Greg · 2020-02-18T21:44:38Z

No hurry. I'm thinking about the package action...

ioquatix · 2020-02-19T00:05:08Z

Here is how I did packages in rack.

https://github.com/rack/rack/blob/2b22be07f189fd852fb66a601573c38439b1f7b4/.github/workflows/development.yml#L27-L33

It's not that great but it works.

I think I prefer something simpler where possible, e.g.

packages:
  - foo
  - bar
  - baz

MSP-Greg · 2020-02-19T01:21:18Z

I thought a generic 'packages' might be helpful, but I doubt many packages have a universal name.

Using your rack example, maybe something like:

- name: install packages
  uses: ruby/setup-ruby-packages@v1
  with:
    apt-get: libfcgi-dev libmemcached-dev
    brew: fcgi libmemcached

Normal Windows Ruby (mingw) has MSYS2 packages and MinGW packages, so for example, a generic package input wouldn't detail whether openssl was the openssl MSYS2 package, or the mingw-w64-x86_64-openssl MinGW package...

MSP-Greg · 2020-02-19T16:01:43Z

A little off topic, but...

One of the issues here and with custom actions is determining what the environment is, whether it be OS, image version, Ruby version/platform, etc.

See actions/runner-images#407 (comment)

An ENV variable ImageOS will be added to the next image releases that will contain the Ubuntu or Windows image type. There's a table in the comment. macOS looks like it will need to checked in a different manner. I mentioned this because I forget stuff...

EDIT: after looking over some comments, the issue for it is also tagged 'macOS', so that may be added as a value, don't know...

eregon · 2020-02-20T00:06:34Z

It would be best to keep the packages discussion in one place: #21
Maybe I can reopen that issue if it helps, although I don't plan to implement that in this action?

Sorry, I got too busy today, will review it when I get some time.

eregon · 2020-02-22T13:04:22Z

.github/workflows/test.yml

+      shell: cmd
+      if: endsWith(matrix.ruby, 'mswin')
+      run: |
+        call "C:\Program Files (x86)\Microsoft Visual Studio\${{ matrix.vs }}\Enterprise\VC\Auxiliary\Build\vcvars64.bat"


It seems quite inconvenient to have to use this long line before installing any C extension.
Is this something the action could setup once and for all? Or it has to be done in each shell session?

I think very few Rubyists expect having to anything extra before being able to install C extensions.

Again, this is mess. I've defined the string as an ENV variable in actions-ruby.

The vcvars*.bat files are needed when using msvc from the command line, as the scripts set up the environment. So, if one needs the tools in multiple steps (which isolate the ENV from user script changes), one needs to run the script in each one until there's a better solution. I am looking into one.

Some of this is made messier by Actions image options. I haven't checked recently, but they announced that they were removing the Window-2016 image. They are still updating the code for it.

So, although they've stated that they'll only have one Windows image, that will likely change when they release another version of Windows Server. That's the image side, then there is the Visual Studio ('VS') side. There's only one version of VS in each image, but that may also change. So, again, I may see if I can get answers about the singular/multiple issues, as that effects the code and needed inputs in custom actions.

If https://github.com/MSP-Greg/actions-ruby/blob/164cd7048c050264a0360510c572e1d4472b4bfb/index.js#L139 is enough to let gem install json work with mswin builds, I think we should do that in this action too.

Ok. I'll set it up assuming 'singular' (as does actions-ruby) and hope we can keep an eye on Actions images.

Done. The Actions job runs it both in a cmd shell and the default PowerShell shell. A little different. I've got a bunch of issues to open re the Actions runner, one will be about adding the setting in VCVARS to the env with an action...

eregon · 2020-02-22T13:07:53Z

index.js

@@ -7,6 +7,10 @@ async function run() {
    const platform = getVirtualEnvironmentName()
    const [engine, version] = parseRubyEngineAndVersion(core.getInput('ruby-version'))

+    if (!platform.startsWith('windows') && version.match(/(mingw|mswin)$/)) {
+      throw new Error(`ruby-${version} is only available on windows`)
+    }


This shouldn't be necessary, getAvailableVersions + validateRubyEngineAndVersion will already check that below

That's fine. I thought a specific message might be helpful, I'll remove.

eregon · 2020-02-22T13:11:50Z

windows.js

@@ -46,6 +48,25 @@ export async function install(platform, ruby) {
  return rubyPrefix
 }

+// all standard msvc OpenSSL builds use C:\Program Files\Common Files\SSL
+// as per openssl/openssl
+function mswinSSL(hostedRuby) {


Could we just set SSL_CERT_FILE instead like it's done for 2.2 and 2.3 above?

With 2.2. and 2.3, the reason for using the ENV settings is that the OpenSSL package used pointed to a folder specific to the system the builder used.

The path for normal Windows OpenSSL builds is actually in the openssl/openssl code. MSYS2 changes it to reference the location of the exe using OpenSSL. Actions should have the files located there, but I don't want to assume that.

So, I'd prefer to set up OpenSSL as expected, and consider the 2.2 and 2.3 builds the exceptions.

I would think creating an empty C:\\Program Files\\Common Files\\SSL\\certs directory has no effect, so I would just skip that and only check the cert.pem.
I'd prefer to simplify and just check C:\\Program Files\\Common Files\\SSL\\cert.pem exists and otherwise throw.

If the cert.pem file was missing, would the ruby -ropen-uri -e 'puts open(%{https://rubygems.org/}) { |f| f.read(1024) }' check file? If so, I think we can use that as our check of "OpenSSL works and can check certificates", without any extra code in windows.js.

If the cert.pem file was missing, would the ruby -ropen-uri -e 'puts open(%{https://rubygems.org/}) { |f| f.read(1024) }'

It fails. Just tried locally after renaming my cert file in 'C:\Program Files\Common Files\SSL`.

C:\Program Files\Common Files\SSL\certs

It is a correctly set up OpenSSL, and people may not check for its existence...

I think the check in the workflow is enough then.
If people rely on a directory being in the image, that's their responsibility.
setup-ruby should care about providing a working Ruby, including a working OpenSSL C extension, but not try to "lint" the whole system. There might be exceptions if we know something that is particularly unreliable, but for now I think the open-uri check above is good enough for testing OpenSSL.

My concern was more about having all the Ruby builds setup the same. If some correctly connect a cert file, all should. Same for the directory. I'll check.

X509::DEFAULT_CERT_DIR exists on all builds, see:
https://github.com/MSP-Greg/ruby-setup-ruby-info/runs/460598415

head builds are failing, due to an issue with ruby/openssl. Fixed there, not pushed to ruby/ruby yet.

I see, it just fails if we don't setup those files: https://github.com/ruby/setup-ruby/runs/465758532
I thought C:\\Program Files\\Common Files\\SSL\\cert.pem would already exist but apparently not.

index.js

eregon · 2020-02-22T13:18:08Z

windows-versions.js

@@ -24,5 +24,6 @@ export const versions = {
  "2.6.4": "https://github.com/oneclick/rubyinstaller2/releases/download/RubyInstaller-2.6.4-1/rubyinstaller-2.6.4-1-x64.7z",
  "2.6.5": "https://github.com/oneclick/rubyinstaller2/releases/download/RubyInstaller-2.6.5-1/rubyinstaller-2.6.5-1-x64.7z",
  "2.7.0": "https://github.com/oneclick/rubyinstaller2/releases/download/RubyInstaller-2.7.0-1/rubyinstaller-2.7.0-1-x64.7z",
-  "head": "https://github.com/oneclick/rubyinstaller2/releases/download/rubyinstaller-head/rubyinstaller-head-x64.7z"
+  "head": "https://github.com/MSP-Greg/ruby-loco/releases/download/ruby-master/ruby-mingw.7z",


I'm a bit hesitant to switch from rubyinstaller-head to ruby-loco here, because all release Windows builds are from RubyInstaller, so it creates some inconsistency.

Maybe we should allow both?
How about having head refer to rubyinstaller-head and mingw + mswin refer to ruby-loco builds?

eregon · 2020-02-22T13:22:12Z

Ref: #22

1. Add ruby-loco mingw and mswin 2. Split workflow jobs into common and specials 3. Adjust workflow for mswin - 'ridk version' & 'c extension' steps

MSP-Greg · 2020-02-22T21:38:10Z

So I did a force push and nothing ran... Not my day.

eregon · 2020-02-24T20:48:57Z

So I did a force push and nothing ran... Not my day.

I can trigger actions on pull_request if that helps.
Given it's triggering quite a lot of jobs I was a bit hesitant, since anyway they should run the fork's actions too on push.

eregon · 2020-02-24T20:57:51Z

.github/workflows/test.yml

+        cfg:
+          - { ruby: rubinius, os: ubuntu-18.04   }
+          - { ruby: mingw   , os: windows-latest }
+          - { ruby: mswin   , os: windows-latest }


Interesting syntax, that's cool :)
It's unfortunate we have to copy the steps, but I guess there is no other way.

eregon

This looks great, thank you, I'll merge this.

eregon · 2020-02-24T22:04:49Z

@MSP-Greg I documented those in the README:
739b731
Does that sound right?

eregon · 2020-02-24T22:07:44Z

Released as https://github.com/ruby/setup-ruby/releases/tag/v1.20.0 and v1 bumped.

MSP-Greg · 2020-02-24T22:31:43Z

@eregon

Thanks. On to the package stuff.

mingw and mswin are ruby-head builds using the MSYS2 and MSVC toolchains on Windows.

I think that's clear, but we (along with most people in the ruby org) know about them. Maybe something like:

On Windows, ruby-head & mingw are builds of master/head using MSYS2/MinGW gcc, which is the standard compiler used for Windows Rubies. An mwsin build is also available, which is compiled with MSVC. Both compilers are tested against in ruby/ruby.

It needs to be short, but hopefully it explains the builds for someone who knows nothing about Ruby on Windows... I'll defer to your judgement.

IMO, the people that need to know what mswin is already know. One can't even use mswin with RuboCop, as jaro-winkler won't compile with it, and the current release doesn't force mswin to use the 'pure Ruby' implementation, it just fails...

MSP-Greg · 2020-02-25T01:57:07Z

Interesting syntax, that's cool :)

See actions/runner#343

Officially supported and soon to be improved.

eregon · 2020-02-29T10:52:03Z

@MSP-Greg Excellent! I wonder if we could merge again the test steps to avoid duplication with the new semantics for include:.

From #24 (comment)

MSP-Greg · 2020-02-29T14:43:23Z

Should roll out to all accounts sometime over the next week or two.

It does take one to two weeks...

ENV['ImageOS'] is now available on Ubuntu & Windows. I think os.platform is still needed, but the combination of ImageOS and ImageVersion lets one determine exactly what 'virtual environment' image one is running on. For instance, on windows-latest, ImageOS is win19, and on ubuntu-latest, ImageOS is ubuntu18. On macOS, neither setting exists...

larskanis · 2020-08-28T15:19:36Z

I didn't notice this change, but I like it! Now all interesting Windows Ruby versions are available through one action - great work!

eregon reviewed Feb 22, 2020

View reviewed changes

index.js Outdated Show resolved Hide resolved

eregon reviewed Feb 22, 2020

View reviewed changes

Add ruby-loco mingw and mswin

eabdf80

1. Add ruby-loco mingw and mswin 2. Split workflow jobs into common and specials 3. Adjust workflow for mswin - 'ridk version' & 'c extension' steps

MSP-Greg force-pushed the ruby-loco-mingw-mswin branch from 0c63d83 to eabdf80 Compare February 22, 2020 21:13

eregon reviewed Feb 24, 2020

View reviewed changes

eregon approved these changes Feb 24, 2020

View reviewed changes

eregon merged commit 20324f7 into ruby:master Feb 24, 2020

eregon mentioned this pull request Feb 24, 2020

Windows - ruby-head, ruby-mswin #22

Closed

MSP-Greg deleted the ruby-loco-mingw-mswin branch February 24, 2020 22:14

eregon added a commit that referenced this pull request Feb 29, 2020

Clarify about mingw and mswin

6044e83

From #24 (comment)

Add ruby-loco mingw and mswin #24

Add ruby-loco mingw and mswin #24

Conversation

MSP-Greg commented Feb 18, 2020

eregon commented Feb 18, 2020

MSP-Greg commented Feb 18, 2020

ioquatix commented Feb 19, 2020

MSP-Greg commented Feb 19, 2020 • edited Loading

MSP-Greg commented Feb 19, 2020 • edited Loading

eregon commented Feb 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eregon Feb 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eregon Feb 24, 2020 • edited Loading

Choose a reason for hiding this comment

eregon Feb 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eregon commented Feb 22, 2020 • edited Loading

MSP-Greg commented Feb 22, 2020

eregon commented Feb 24, 2020

Choose a reason for hiding this comment

eregon left a comment

Choose a reason for hiding this comment

eregon commented Feb 24, 2020

eregon commented Feb 24, 2020

MSP-Greg commented Feb 24, 2020

MSP-Greg commented Feb 25, 2020

eregon commented Feb 29, 2020

MSP-Greg commented Feb 29, 2020

larskanis commented Aug 28, 2020

MSP-Greg commented Feb 19, 2020 •

edited

Loading

MSP-Greg commented Feb 19, 2020 •

edited

Loading

eregon Feb 22, 2020 •

edited

Loading

eregon Feb 24, 2020 •

edited

Loading

eregon Feb 22, 2020 •

edited

Loading

eregon commented Feb 22, 2020 •

edited

Loading