Fix PATH on Windows #20

eregon · 2020-02-09T17:15:30Z

See setup-ruby removes chocolatey from PATH #19

eregon · 2020-02-09T17:51:11Z

windows.js

@@ -62,7 +62,8 @@ async function linkMSYS2() {
 }

 export function setupPath(msys2, rubyPrefix) {
-  let path = process.env['PATH'].split(';')
+  const originalPath = process.env['PATH'].split(';')
+  let path = originalPath.slice()

  // Remove conflicting dev tools from PATH
  path = path.filter(e => !e.match(/\b(Chocolatey|CMake|mingw64|OpenSSL|Strawberry)\b/))


@MSP-Greg @slonopotamus What do you think is the best way to proceed?

Remove this line entirely

Make it an option to clean to path, and choose a default value

Remove only Chocolatey from this line

What are the risks if we keep any of them in PATH?
Ideally I think a setup-* action shouldn't remove anything in PATH except a conflicting tool (Ruby, done below).

What are the risks if we keep any of them in PATH?

Confused people.

Ideally I think a setup-* action shouldn't remove anything in PATH except a conflicting tool.

I think that would be best. If people want to use MSYS2, they can use a windows specific action or MSYS2 commands.

But as mentioned, A windows specific option to clean the path and enable MSYS2 might be helpful. It's easy, and that's what the majority of Windows Ruby CI (that requires MinGW compile tools) is doing.

But as mentioned, A windows specific option to clean the path and enable MSYS2 might be helpful. It's easy, and that's what the majority of Windows Ruby CI (that requires MinGW compile tools) is doing.

I think most people expect MSYS2 to be available by default.

And I would think there are cases where one wants both MSYS2 and installing packages with choco. Or is that always problematic/a mistake?

With that thinking, if we add an option it would just be an option to clean the PATH, and otherwise leave it untouched (except adding/removing Ruby). I'd still add MSYS2 no matter what, as I don't think there is a need for "no MSYS2" (and therefore "cannot compile C extensions").

@slonopotamus For context, what do you want to install with Chocolatey?
Is it something that could be installed with MSYS2's pacman instead?

Do you need to be able to install C extensions on Ruby on Windows?

@MSP-Greg Would installing via pacman generally work better than with Chocolatey for use with Ruby?

@eregon

Would installing via pacman generally work better than with Chocolatey for use with Ruby?

I've helped a lot of extension gems/repos with Windows CI, and I don't recall ever using a 'Chocolatey package' for compiling. But, some repos may use it for other requirements.

slonopotamus · 2020-02-09T18:38:59Z

For context, what do you want to install with Chocolatey?
Is it something that could be installed with MSYS2's pacman instead?

Do you need to be able to install C extensions on Ruby on Windows?

Just some random software, totally Ruby-unrelated: graphviz, gnuplot, imagemagick.

What I am actually trying to do is to convert asciidoctor-diagram appveyor.yml to GH-Actions.

MSP-Greg · 2020-02-09T18:45:50Z

@slonopotamus

What I am actually trying to do is to convert asciidoctor-diagram appveyor.yml to GH-Actions.

32 bit Ruby? Do you need that?

I've asked for 32 bit MSYS2 tools on Actions, but they can't seem to decide about MSYS2.

slonopotamus · 2020-02-09T18:52:55Z

32 bit Ruby?

Err... I don't understand where you see that. As I said, I use Chocolatey for non-Ruby software.

MSP-Greg · 2020-02-09T19:03:49Z

@slonopotamus

The matrix (shown below) is all 32 bit Ruby builds. Normally, 64 bit has a -x64 suffix. For instance, see https://github.com/ruby/psych/blob/master/appveyor.yml.

If you're not compiling anything for Ruby, it doesn't matter, but Actions is only 64 bit Rubies.

environment:
  matrix:
    - ruby_version: "25"
    - ruby_version: "24"
    - ruby_version: "23"

eregon · 2020-02-09T19:04:32Z

Just some random software, totally Ruby-unrelated: graphviz, gnuplot, imagemagick.

There are MSYS2 packages for all these 3: https://packages.msys2.org/base

I think the advantage of using those (anyone correct me if I'm wrong, I'm not very familiar with Windows) is if any Ruby gem compiles against one of them there would be a much higher chance for it to work, since it would use the same kind of toolchain Ruby was compiled with.

What does Chocolatey uses for the build toolchain? mingw-x64? MSYS2? Something else?

slonopotamus · 2020-02-09T19:06:57Z

Okay, no, I'm not trying to use 32-bit Ruby. Here's my work-in-progress workflow file (it doesn't work properly yet): https://github.com/slonopotamus/asciidoctor-diagram/blob/gh-actions/.github/workflows/ci.yml

MSP-Greg · 2020-02-09T19:11:50Z

What does Chocolatey uses for the build toolchain?

That's not specific to Chocolatey. One can install MSYS2 with Chocolatey. So, what it has is what GH decided to pre-install (I think).

slonopotamus · 2020-02-09T19:12:41Z

What does Chocolatey uses for the build toolchain?

Chocolatey is used to just install Windows software. It's not a build environment.

There are MSYS2 packages for all these 3

The point about Chocolatey is that it is a convenient way to install and update programs on Windows without the need to go to multiple websites searching for installers. You may see Chocolatey on non-programmer machines, unlike MSYS2 that is kinda specific thing. You would normally use MSYS2 tools from within a MSYS2 shell. This is not the case of Chocolatey, it mostly provides native Windows software.

eregon · 2020-02-09T19:34:15Z

Right, but those precompiled Chocolatey packages must have been compiled with some toolchain.
I guess C toolchains might be mostly compatible, but I wouldn't have much hope for C++.

As seen in https://github.com/eregon/setup-ruby-test/runs/434763137, there is a toolchain pre-installed in Chocolatey. I'm not sure which one it is but I'd guess MinGW.

OK, I agree we should let users use Chocolatey if they want to.

So how about not removing Chocolatey from the PATH, and see how that goes?
If there are weird errors due to conflicts with MSYS2 we'll need to reconsider this and e.g., have an option about this.

I can also add an option to remove nothing but Ruby from the PATH.
That seems a good safeguard.

@MSP-Greg

What are the risks if we keep any of them in PATH?

Confused people.

Could you give an example of what could go wrong if we remove nothing but Ruby from the PATH, and still have MSYS2 paths at the start of the PATH?

slonopotamus · 2020-02-09T19:42:46Z

Right, but those precompiled Chocolatey packages must have been compiled with some toolchain.

Chocolatey doesn't provide build infrastructure, it only handles installation step. You can just wrap pre-existing MSI package in Chocolatey and you don't need to know how and with what tools that MSI was created.

So how about not removing Chocolatey from the PATH, and see how that goes?

I think everything should be fine as long as you add whatever is needed for Ruby at the beginning of PATH.

eregon · 2020-02-09T21:03:28Z

OK, I propose to just not remove anything from PATH then, except the system Ruby (no point to have 2 Ruby in PATH).
That's what I tried in c30207d and the (limited) tests here seem fine. At least installing json keeps working.

@MSP-Greg Is that OK with you?
Do you think we need/should have an option to clean the PATH as previous behavior?
I think I would still want that option disabled by default (= don't remove), by the reasoning that a setup-* action should not remove tools, only add (or replace for the specific tool it's adding, here Ruby).

MSP-Greg · 2020-02-09T21:36:05Z

Is that OK with you?

Yes

Do you think we need/should have an option to clean the PATH as previous behavior?

I'd suggest waiting on it. We can always add to the README about windows compile issues...

* It's quite verbose (22 lines) and there is no quiet flag.

* This might lead to more conflicts, but the flip side of a setup action removing capabilities seems worse and unexpected. * See #19

* It already ran on the feature branch.

eregon · 2020-02-09T22:51:42Z

Merged and added notes in the README about Windows in 4f28d1c

eregon force-pushed the fix-windows-path branch 7 times, most recently from 34f3cbd to 04a6a07 Compare February 9, 2020 17:42

eregon mentioned this pull request Feb 9, 2020

setup-ruby removes chocolatey from PATH #19

Closed

eregon commented Feb 9, 2020

View reviewed changes

eregon added 4 commits February 9, 2020 23:37

Silence the output of 7z on Windows

71f024b

* It's quite verbose (22 lines) and there is no quiet flag.

Print entries removed from PATH on Windows

e15ecc8

Do not remove any dev tools from PATH on Windows

55dff25

* This might lead to more conflicts, but the flip side of a setup action removing capabilities seems worse and unexpected. * See #19

Don't run CI twice when merging to master

53b22d2

* It already ran on the feature branch.

eregon force-pushed the fix-windows-path branch from c30207d to 53b22d2 Compare February 9, 2020 22:39

eregon merged commit 53b22d2 into master Feb 9, 2020

eregon deleted the fix-windows-path branch February 16, 2020 15:43

chrisd8088 mentioned this pull request Feb 18, 2022

MSYS2 Bash shell seems to override PATH settings #293

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix PATH on Windows #20

Fix PATH on Windows #20

eregon commented Feb 9, 2020

eregon Feb 9, 2020

MSP-Greg Feb 9, 2020 •

edited

Loading

eregon Feb 9, 2020 •

edited

Loading

eregon Feb 9, 2020

MSP-Greg Feb 9, 2020

slonopotamus commented Feb 9, 2020

MSP-Greg commented Feb 9, 2020

slonopotamus commented Feb 9, 2020

MSP-Greg commented Feb 9, 2020

eregon commented Feb 9, 2020

slonopotamus commented Feb 9, 2020

MSP-Greg commented Feb 9, 2020

slonopotamus commented Feb 9, 2020 •

edited

Loading

eregon commented Feb 9, 2020

slonopotamus commented Feb 9, 2020

eregon commented Feb 9, 2020 •

edited

Loading

MSP-Greg commented Feb 9, 2020

eregon commented Feb 9, 2020

Fix PATH on Windows #20

Fix PATH on Windows #20

Conversation

eregon commented Feb 9, 2020

eregon Feb 9, 2020

Choose a reason for hiding this comment

MSP-Greg Feb 9, 2020 • edited Loading

Choose a reason for hiding this comment

eregon Feb 9, 2020 • edited Loading

Choose a reason for hiding this comment

eregon Feb 9, 2020

Choose a reason for hiding this comment

MSP-Greg Feb 9, 2020

Choose a reason for hiding this comment

slonopotamus commented Feb 9, 2020

MSP-Greg commented Feb 9, 2020

slonopotamus commented Feb 9, 2020

MSP-Greg commented Feb 9, 2020

eregon commented Feb 9, 2020

slonopotamus commented Feb 9, 2020

MSP-Greg commented Feb 9, 2020

slonopotamus commented Feb 9, 2020 • edited Loading

eregon commented Feb 9, 2020

slonopotamus commented Feb 9, 2020

eregon commented Feb 9, 2020 • edited Loading

MSP-Greg commented Feb 9, 2020

eregon commented Feb 9, 2020

MSP-Greg Feb 9, 2020 •

edited

Loading

eregon Feb 9, 2020 •

edited

Loading

slonopotamus commented Feb 9, 2020 •

edited

Loading

eregon commented Feb 9, 2020 •

edited

Loading