actions / runner-images

GitHub Actions runner images
MIT License
9.17k stars 2.84k forks source link

Switch Unix tools from Git internal tools to MSYS2 tools on Windows images #1525

Closed AlenaSviridenko closed 3 years ago

AlenaSviridenko commented 3 years ago

Breaking changes

"Git For Windows" Unix tools will be replaced by "MSYS2" Unix tools on Windows images. Currently, folders C:\Program Files\Git\bin and C:\Program Files\Git\usr\bin and C:\Program Files\Git\mingw64\bin are added to PATH and it expose a ton of Unix tools (Git For Windows Internal tools) to PATH. We would like to remove those folders from PATH because one day they can be removed by Git maintainers. A lot of people already depend on those tools so we would like to replace Git Unix tools by MSYS2 Unix tools to reduce impact.

Target date

Deployment will start on November, 2 and changes propagation will take 2-3 days. This change was rolled back and should be reevaluated

The motivation for the changes

Per communication with Git For Windows maintainers, we should not add Git Internal tools to PATH and rely on it. They could be removed in future and break a lot of people.

More details can be found in this PR by @dscho https://github.com/actions/virtual-environments/pull/1435.

All discussions can be found in this issue, also in MSYS2 issue https://github.com/actions/virtual-environments/issues/1572 and in the final PR https://github.com/actions/virtual-environments/pull/1648

Possible impact

The most of the tools are identical between "Git For Windows" Internal and MSYS2 but some minor versions could be different and it can affect some builds.

Mitigation ways

You will be able to return "Git For Windows" Unix tools to PATH using the following powershell step:

maxim-lobanov commented 3 years ago

These changes will remove some Unix tools (that come with Git) from PATH on Windows machines. See the full list of tools in post above. This change won't affect such tools like bash and tar.

@bryanmacfarlane @konradpabjan , @damccorm do you know if any of these tools can be used on Windows by actions/toolkit or azure-pipelines-task-lib or and it can affect you somehow? After quick look, I see that you mostly use fs Node.JS module and default Windows commands. The single unix command that you use is tar but it won't be affected since it is located in Windows/System32.

damccorm commented 3 years ago

As far as I know we should be good with this change from the Azure Pipelines side

joshmgross commented 3 years ago

The single unix command that you use is tar but it won't be affected since it is located in Windows/System32.

@maxim-lobanov I believe Windows 2016 relies on the Git Unix tar since only Windows 2019 has the Windows tar (see https://docs.microsoft.com/en-us/virtualization/community/team-blog/2017/20171219-tar-and-curl-come-to-windows).

tar is necessary for actions/cache

thboop commented 3 years ago

Besides tar on earlier versions of windows, to the best of my knowledge I think we should be good for the toolkit. We will definitely want to manually install that before we remove this from path.

Individual actions or users may be using these utilities though, there isn't a great way to audit that. We should plan on rolling out slowly and rolling back in case we start seeing issues.

We do use stuff like git-lfs but there appears to be a non internal git copy that takes precedent (the /cmd one)

Application git-lfs.exe                                    2.28.0.1          C:\Program Files\Git\cmd\git-lfs.exe
Application git-lfs.exe                                    0.0.0.0           C:\Program Files\Git\mingw64\bin\git-lfs.…
maxim-lobanov commented 3 years ago

@joshmgross , that is good point. I think we can put to the separate folder and put this folder to the PATH, right? actions/cache should find it: https://github.com/actions/toolkit/blob/c9819f79d2a27c1f26ef7ea85ff44346e7d60741/packages/cache/src/internal/tar.ts#L25

The reason for this change is that we shouldn't rely on Unix tools that come from Git, they can be removed in future. Initially, all these tools were added to PATH unexpectedly. You can find more details in https://github.com/actions/virtual-environments/pull/1435#issuecomment-679163324

shivammathur commented 3 years ago

@maxim-lobanov similar to tar, would it be possible to keep printf in PATH. It is useful to output text with color as Write-Host parameters -ForegroundColor and -BackgroundColor have no effect.

dscho commented 3 years ago

We do use stuff like git-lfs but there appears to be a non internal git copy that takes precedent (the /cmd one)

As long as you do not try to call it via git-lfs.exe, but via git lfs (or even better: implicitly via the common smudge/clean filters), the internal copy will be picked up by Git. My PR won't change that.

dscho commented 3 years ago

Re: tar and printf: I really hate to spoil the party, but it is really, like really not guaranteed that Git for Windows will ship with these!

If you want to make sure that those are shipped, and officially supported, it will need to be handled via C:\Program Files\Git\cmd, and I will have to be convinced that this does not increase my maintenance burden.

One plan, for example, is to use BusyBox at some stage in the future, which does have optional support for tar (and it also supports printf, but it might not come with a printf.exe). Your reliance on these Unix tools in Git for Windows would be broken by that change. That's why I highly recommended against relying on them: you are using unsupported features as if they were supported ones.

maxim-lobanov commented 3 years ago

Hello everyone, We are a bit concerned that removing Git Unix tools from Windows images could cause unpredictable impact, also it doesn't look like a good idea to remove Git Unix tools from PATH but support hardcoded list of tools to have in PATH (like tar and printf as was asked above) so we would like to continue some discussion here.

Summary & Details: Currently, PATH contains the following folders with Unix tools (in order of PATH priority): C:\Program Files\Git\bin, C:\windows\system32, C:\Program Files\Git\cmd, C:\Program Files\Git\mingw64\bin, C:\Program Files\Git\usr\bin.

Per discussion with Git for Windows maintainers, we shouldn't add Git Unix tools to PATH because those tools can be unexpectedly removed in future. It means that we should remove the following folders from PATH: C:\Program Files\Git\mingw64\bin and C:\Program Files\Git\usr\bin. Since these folders are located at the end of PATH, binaries which are resolved from them, will become unavailable. This change can cause issues and impact since some customers / tools depend on those binaries. At least, we must not remove tar and printf from PATH and potentially some other tools that we are not aware about right now.

Possible options: 1) Accept this change in current view (remove all Unix tools from PATH except tar, printf, ???). 2) Replace those two Git folders (C:\Program Files\Git\usr\bin and C:\Program Files\Git\mingw64\bin) by MSYS Unix binaries. We can add C:\msys64\usr\bin folder to PATH. This folder contains pretty the same list of binaries like C:\Program Files\Git\usr\bin with minimal differences and mostly the same versions. If we add C:\msys64\usr\bin at the end of PATH, removing Git tools from PATH should be pretty smoothly since the most tools will continue to work as previously. (Unix tools that are resolved from System32 folder right now, will continue to work as expected since system32 is located before in PATH)

Tools with changes versions (3 tools)

``` awk (5.0.0 -> 5.1.1) gawk (5.0.0 -> 5.1.1) grep (3.1 -> 3.0) ```

Tools that will be removed (54 tools)

``` ahost antiword blocked-file-util brotli certtool connect create-shortcut edit_test edit_test_dll ex fido2-assert fido2-cred fido2-token git-askyesno git-credential-helper-selector git-receive-pack git-upload-archive git-upload-pack lzmadec lzmainfo nano odt2txt pdftotext pluginviewer proxy-lookup psktool rnano rview rvim sasldblistusers2 saslpasswd2 srptool ssh ssh-pageant tig unxz view vim vimdiff WhoUses winpty winpty-agent winpty-debugserver wintoast wish wish86 xmlwf xxd xz xzcat xzdec zipcmp zipmerg ziptool ```

**
Tools without changes (223 tools)**

``` [ arch b2sum base32 base64 basename basenc facto test bunzip2 bzcat bzip2 bzip2recover captoinfo cat chattr chcon chgrp chmod chown chroot cksum cmp column comm cp csplit cut cygcheck cygpath cygwin-console-helper d2u date dd df diff diff3 dir dircolors dirmngr dirmngr-client dirname dos2unix du dumpsexp env envsubst expr false file fmt fold getconf getfacl getopt gettext gkill glib-compile-schemas gpg gpg-agent gpgconf gpg-connect-agent gpg-error gpgparsemail gpgscm gpgsm gpgtar gpgv gpg-wks-server groups gsettings gzip head hmac256 hostid id install join kbxutil kill ldd ldh less lessecho lesskey link ln locale locate logname ls lsattr mac2unix md5sum minidumper mintty mkdir mkfifo mkgroup mknod mkpasswd mktemp mount mpicalc msgattrib msgcat msgcmp msgcomm msgconv msgen msgfilter msgfmt msggrep msginit msgmerge msgunfmt msguniq mv nettle-hash nettle-lfib-stream nettle-pbkdf2 ngettext nice nl nohup nproc numfmt od p11-kit p11tool passwd paste pathchk pinentry pinentry-w32 pinky pkcs1-conv pldd printenv printf ps pr psl ptx pwd readlink realpath rebase recode-sr-latin rm rmdir runcon sdiff sed seq setfacl setmetamode sexp-conv sha1sum sha224sum sha256sum sha384sum sha512sum shred shuf sleep split ssp stat strace stty sum sync tac tail tar toe touch tput tr true truncate trust tset tsort tty tzset u2d umount uname unexpand uniq unix2dos unix2mac unlink unzip unzipsfx users vdir watchgnupg wc which who xargs xgettext yat2m yes zipinfo clear dash gapplication gdbus gencat gio-querymodules gobject-query infocmp infotocap regtool tabs tclsh tclsh86 tic tee ```

3) Please share any other suggestions We don't have much experience with Unix tools on Windows but the second option looks like a good way to keep the most of the tools to work as previously and stop relying on Git Unix binaries cc: @dscho as `Git for Windows` maintainer cc: @MSP-Greg and @eine as MSYS experts cc: @alepauly @AlenaSviridenko
MSP-Greg commented 3 years ago

I don't see any problems with this.

ruby/setup-ruby and MSP-Greg/setup-ruby-pkgs are enabling MSYS2 early in their code, and have switched to using the MSYS2 bash shell.

Re option 2 above, you may need to also add C:\msys64\mingw64\bin. It's been a while since I've compared the tools in each, as I've always used both MSYS2 paths in CI and locally. Some tools exist in both.

Note that there are two issues, first, what exe is run from a command/ps1 prompt, and the second, does this affect the use of bash shell with Windows runners.

dscho commented 3 years ago

I am in favor of replacing C:\Program Files\Git\usr\bin (and ...\ming64\bin) in PATH by their MSYS2 equivalents: in contrast to Git for Windows, which only so happens to ship those tools at the moment, MSYS2 does support the indicated use cases explicitly.

MSP-Greg commented 3 years ago

@dscho

Thanks. What is the suggested way to add Git to path, C:\Program Files\Git\bin or C:\Program Files\Git\cmd? I assume cmd, as it doesn't have bash in the folder?

EDIT: I believe the current path contains:

C:\Program Files\Git\bin

then, later in the path:

C:\Program Files\Git\cmd
C:\Program Files\Git\mingw64\bin
C:\Program Files\Git\usr\bin
dscho commented 3 years ago

If you want to be able to use Git for Windows' Bash, you need the Git\bin directory. Otherwise, the Git\cmd directory would be the appropriate one.

eine commented 3 years ago

Initially, all these tools were added to PATH unexpectedly.

@maxim-lobanov, IMHO, this is not correct. Instead, I'd say it is because of lack of expertise, and specially lack of attention. I've been following this repository since it was created, and there were discussions about this same issue before (related to MSYS2 and/or WSL). This has been going around for about a year already (before Actions were "officially" out of beta). Overall, many different employees seem to be doing their best effort, but I feel that none of them is being scheduled the time for properly understanding the issue, providing a stable solution and maintaining it. Until managers understand the severe impact of all these arbitrary "fixes", the product is going to suffer (even more), regardless of all the effort that engineers and contributors are devoting.

As the maintainer of Git for Windows said, none of the mingw/mingw64 tools shipped with it should be used at all, under any circumstance. Should anyone be doing it, they should learn about the development tools they should be using instead, and fix it ASAP. Should their workflows be broken suddenly, that's a problem on their side, because they followed an ugly and hacky approach (see https://github.com/actions/virtual-environments/pull/1435#issuecomment-682026800). GitHub Actions might be blamed for making it easier to unexperienced users doing mistakes and learning inadequate practices. However, I believe that's something GitHub is not worried about, given that Actions are written in JavaScript only. So, this should be fixed ASAP, regardless of the impact: "the longer you wait, the bigger the impact will be" (https://github.com/actions/virtual-environments/pull/1435#issuecomment-680155827).

However, given that the mechanism for communicating these breaking changes seems to be "good luck finding an specific changelog", it might be better to do nothing until that is addressed.


From a wider point of view, most if not all the users that are relying on Git for Windows' internal GNU/Linux tools, do likely want to use MSYS2 instead. In MSYS2, all those utilities are explicitly expected to be used (https://github.com/actions/virtual-environments/issues/1525#issuecomment-688466066). Plus, there is a plethora of packages available through pacman, which typically install much faster than using chocolatey (another tool which has not been discussed). Hence, the natural and shameless migration would have been to drop MSYS2 in the PATH as a replacement of Git for Windows, when MSYS2 was added some months ago (not now) (see https://github.com/actions/virtual-environments/pull/342#issuecomment-580969845). Instead, there was no feedback about the proposal, the implementation was done by a third-party contributor, attending his own needs only, and no maintainer (employee or contributor) took care of it.

Very fortunately, some months ago actions/toolkit#318 was partially addressed, and it is now possible to preset a custom shell, even before the file/script exists. See, for instance:

  defaults:
    run:
      # We define a custom shell script here, although `msys2.cmd` does neither exist nor is it available in the PATH yet
      shell: msys2 {0}
  steps:

  # We use a JS Action, which calls the system terminal or other custom terminals directly, if required
  - uses: msys2/setup-msys2@v2
    with:
      update: true
      install: base-devel git

  # We want to change the configuration of the git command that actions/checkout will be using (since it is not possible to set autocrlf through the action yet, see actions/checkout#226).
  - run: git config --global core.autocrlf input
    shell: bash

  # This uses the default git in the environment, thus Git for Windows
  - uses: actions/checkout@v2

  # This uses the default shell, which is msys2 (defined above, and "installed" by msys2/setup-msys2)
  - run: git describe --dirty

At the same time, maintainers of MSYS2 were really pro-active and put lots of love into the Action. Updates and fixes are addressed in few hours, and all their knowledge about the environment is seen in the Action. From user's perspective, even if 0.5-1 GB of ad-hoc packages need to be installed, startup takes no longer than 90s.

Summarizing, I believe that both Git for Windows and MSYS2 can and should be dropped from the environment. There should be an Action for installing Git for Windows. That Action should also be available as a library, in case other Actions rely on it. Nevertheless, I believe that Actions such as actions/checkout should be compatible with any git (i.e. with the one available through msys/setup-msys2 and/or in WSL too).

Alternatively, Git for Windows, MSYS2 and WSL might coexist, along with handy built-in setup scripts and/or custom entry shells. That would be ideal. Unfortunately, that needs a thoughful proposal and discussion, instead of fixing partial issues as they come by and looking at limited scopes.


Disclaimer: I am not an MSYS2 expert by any means. I am an average user who maintains a couple of unofficial PKGBUILD scripts. My involvement with GitHub Actions and MSYS2 was the following so far:

During that period, I maintained my fork of setup-msys2 alive, in the hope that the "official" solution would get to be good enough for ditching the Action. That didn't happen. At the end of May, msys2/msys2-installer#5 was opened. It is worth mentioning that @MSP-Greg is apparently not aware of the most basic netiquette. He/she tries to sneak by splitting the discussions in multiple unnecessary threads and NOT pinging users which he/she knows would provide arguments discussing his/her proposals. I tried to explain what taking care of a codebase implies: https://github.com/actions/virtual-environments/pull/916#issuecomment-632715194. He/she responded with, msys2/msys2-installer#5, another effort to mess the discussion. So, at some point, I was fed up with trying to have an (at least) decent official MSYS2 solution on GitHub's side.

Fortunately, the reaction of MSYS2 maintainers was lovely. In a couple of weeks, there was an official Action and automatic caching was enabled. In July, lots of enhancements were contributed. Now, I help maintain msys2/setup-msys2, and I understand the codebase, but I'm not an expert on the technical details.

Note that the enhancements in setup-msys2 are compatible with the built-in MSYS2 install provided in this repo (see https://github.com/msys2/setup-msys2#release), but I am not aware of anyone using it since the startup time is typically worse than just ignoring it (see https://github.com/msys2/setup-msys2/issues/23#issuecomment-639287350).


Please, please, someone (employee, contributor, maintainer...) step back and think about the complexity of "having bash and git in Windows". If you are not familiar with some or any of the tools, that's ok. I believe that maintainers of Git for Windows, MSYS2, Chocolatey, etc. all will be delighted to have some public and clarifying discussion about how should all of them coexist in GitHub Actions (both, in the environments and in the ecosystem). But, please, stop doing fast and not properly thought modifications (after this issue, which is NEEDED). Users can solve their own typically complex ad-hoc problems faster than you (or anyone else) can even understand them. IMHO, you should focus on providing an stable environment that makes sense (any sense, you define it).

MSP-Greg commented 3 years ago

@eine

@MSP-Greg is apparently not aware of the most basic netiquette

I could say the same.

Re removing MSYS2:

  1. MSYS2 was/is installed on AppVeyor, and it was used for several years. Why is it now not appropriate for Actions to do the same? AppVeyor did not update their images often, which did cause issues. Given that Actions images are updated at least a few times a month, issues are almost nonexistent.

  2. You mention 90 seconds. For many repos CI, that is a significant amount of time. Additionally, the size of the MSYS2 download is far greater than all the other code needed for the CI.

  3. In some respects, the idea of removing MSYS2 is a partial application of the idea that CI shouldn't have any software installed. That idea doesn't seem to be considered practical.

  4. People are certainly able to use the MSYS2 action if it meets their needs.

  5. Since all these issues are related to Path, users or the Actions team can adjust it to met their needs. As long as software doesn't add files to the default Windows locations, and it isn't placed in Path, it's invisible to the system.

  6. I think many Actions users are using MSYS2 for the compile tools. Having them installed is beneficial, and updating them is simple and fast.

Hence, I feel that MSYS2 should remain installed.

My name is Greg, and I identify myself as male.

dscho commented 3 years ago

@eine @MSP-Greg I really value seeing both of your perspectives spelled out clearly. Maybe we could meet real quick on https://meet.jit.si/GHA-MSYS2 to hash some things out "in person"?

dscho commented 3 years ago

Maybe we could meet real quick on https://meet.jit.si/GHA-MSYS2 to hash some things out "in person"?

I'm going to log off for the day, but maybe we can agree on a time to meet face to face, to resolve any conflicts and work on a mutually satisfactory solution?

MSP-Greg commented 3 years ago

@dscho

Re meet.jit.si, never used it, just installed on my phone.

Obviously, I feel strongly that MSYS2 should be installed on the Actions Windows image(s).

Past that, I'm not sure. I've worked with MSYS2 for a while, build packages locally (most recently OpenSSL 3.0.0), but I'm not a strong c type. I've also used it to build Ruby master/main, and the build has been widely used in Ruby related CI for a few years. It was originally built and stored on AppVeyor, now it's also built on Actions.

I think you, @eine, and myself have dealt with MSYS2 enough to always 'make it work'. With custom actions, we can match the needs of other users and preset the environment for them. @eine and I are involved with separate actions to accomplish that.

Assuming that having MSYS2 installed but not enabled is ok, the issue here is what makes sense for the majority of Actions users. I'm not sure I know the answer to that.

To me, two main issues. How to configure the bash shell, and conflicts.

JFYI, I'm -0500 (central US)

eine commented 3 years ago

@MSP-Greg is apparently not aware of the most basic netiquette

I could say the same.

@MSP-Greg, sure, you are entitled. However, I explained why I thought it and what is the implication for other users. This is not about you and me, but about how our actions affect other users. I'd be so glad if you could warn them properly. Right now it's a childish "you too", again.

  1. MSYS2 was/is installed on AppVeyor, and it was used for several years. Why is it now not appropriate for Actions to do the same? AppVeyor did not update their images often, which did cause issues. Given that Actions images are updated at least a few times a month, issues are almost nonexistent.

This is a false premise that has been discussed and replied before. Please, bring the relevent references instead of trying to tendentiously make a null point, again. Basic netiquette.

  1. You mention 90 seconds. For many repos CI, that is a significant amount of time. Additionally, the size of the MSYS2 download is far greater than all the other code needed for the CI.

Again, false assertions without referencing relevant discussion which you are aware of. Bring them. Basic netiquette.

I did not mention, it is tested by tens, hundreds, thousands of users. Because of your decisions, the built-it solution needs up to 7-12min. It was not my decision/suggestion to ditch the built-in installation. A maintainer of MSYS2 suggested it after being surprised by all the packages installed by default: https://github.com/msys2/setup-msys2/issues/23#issuecomment-639287350. Nonetheless, I agree with the decision, of course.

Note to readers, "for many repos CI" and "all the other code" do mean "his many repos" and "all his other code". There has been no analysis, at least he has not done any public, about the truthfulness of those mantras.

  1. In some respects, the idea of removing MSYS2 is a partial application of the idea that CI shouldn't have any software installed. That idea doesn't seem to be considered practical.

Ideally, MSYS2 would be available as a built-in resource. I said that. I did never propose that CI shouldn't have any software. That's something that you seem to have proposed out of nowhere.

What I said, and I still propose, is that no one seems to be taking care of the elephant in the room. Then, better open the garage door and let it run free.

  1. People are certainly able to use the MSYS2 action if it meets their needs.

Some people NEED to use the MSYS2 Action for avoiding your opinionated and arbitrary decisions. In order to reduce those 90s for your specific use case, you are forcing a +300s penalty in the workflows of all the users which want to use an updated solution. Your reply is: https://github.com/msys2/setup-msys2/issues/23#issuecomment-639751737. And you were explicitly told that partial updates are NOT supported. This one is not about netiquette, but about basic etiquette, since it's other people's money that you are wasting.

Fortunately, thanks to MSYS2 maintainers, the Action can partially alleviate this severe design flaw.

  1. Since all these issues are related to Path, users or the Actions team can adjust it to met their needs. As long as software doesn't add files to the default Windows locations, and it isn't placed in Path, it's invisible to the system.

This is the premise of this issue. Here, "users" and "the Actions team" are discussing how to adjust the PATH for meeting their needs. We agree that removing everything from the PATH (and from the environment) is a solution for avoiding conflicts. However, I believe that's not the solution (integration) we are aiming for.

  1. I think many Actions users are using MSYS2 for the compile tools. Having them installed is beneficial, and updating them is simple and fast.

As explained several times, what you think is irrelevant, given your purposely limited point of view. Let me just place an counterexample:

- uses: msys2/setup-msys2@v2
  with:
    msystem: MINGW32
    update: true
- uses: ghdl/setup-ghdl-ci@nightly
  with:
    backend: mcode
- shell: msys2 {0}
  run: |
    ghdl --version

Currently, GHDL is the only open source solution for testing and synthesis of hardware designs written in VHDL.

Hence, I feel that MSYS2 should remain installed.

I disagree. MSYS2 should only remain installed if properly taken care of.

My name is Greg, and I identify myself as male.

Thanks.


@dscho, I prefer not to use either video or voice. However, I am open to engaging the discussion in any chat, pad, wiki... Nonetheless, as said, I believe I am not a relevant agent to take decisions. The discussion needs to be public and open to maintainers of other projects. Otherwise, we will be having this scope limited conversation again and again.

Two months ago, I proposed Greg to translate part of setup-msys2 to Powershell, so that it could be integrated in this environment (see https://github.com/msys2/msys2-installer/issues/5#issuecomment-656663839) and other environments. He passed, again.


@eine and I are involved with separate actions to accomplish that.

@MSP-Greg, as explained in https://github.com/actions/virtual-environments/issues/1525#issuecomment-689544018, I am involved with providing a usable environment in GitHub Actions that allows using MSYS2 without favouring a very specific subset of users and severily penalizing others. This is specially relevant because GitHub Actions are paid per minute, not per run.

The fact that I am involved in a general purpose Action is just a result of the poor built-in solution. I wish setup-msys2 did not exist. I am a maintainer of other projects, and I'd prefer to spend time on those. setup-msys2 is just a tool that I (and others) need for testing software on Windows.

It is quite tendencious on your side to put on the same page my involvement in setup-msys2 and your involvement in any of the actions you maintain. Neither the scope nor the motivation have any relation. Moreover, you were invited multiple times to help extending setup-msys2 for suiting your specific needs, instead of creating one or multiple custom Actions. You ignored those.

dscho commented 3 years ago

I prefer not to use either video or voice.

@eine fair enough. I only proposed it because I see that emotions run unnecessarily high, and in the past I found that a quick face-to-face conversation helped alleviate similar situations. There is just so much lost when using text-only communication, and it is very easy to make much more aggressive arguments than intended.

I am convinced that we can resolve this issue in a way that meets the needs of all (currently) involved parties.

eine commented 3 years ago

@dscho, just to clarify: I am not a native speaker, and my comments might sound harder than they should. I want them to be direct, and clear, not rude. I am neither angry, nor do I have any personal issue with Greg. I would love to collaborate and find a technical solution that fits all the use cases with as little conflicts as possible. That includes discussing technical details and objective decisions with Greg.

My frustration is motivated by how little attention is GitHub as a company paying to their GitHub Actions product. It came out of beta almost a year ago and the UX is still awful. They had (and still have) lots of references to learn from and to copy from. Yet, there is no captain sailing this boat. That is, Greg pushed his PRs and the environment is as it is now because of GitHub, not because of Greg. In any regular open source project, the review process would have arised all of this problems, and those would have been prevented.

EDIT

Of course, Greg (and any other) is absolutely entitled to replying "I don't want to do that", "I don't want to spend more time on this" or "I believe that employees should do it instead of me". Those are all valid points/replies in a discussion.

alepauly commented 3 years ago

Thank you all for the discussion here, whether it's about the specifics of the issue at hand (tools in path and MSYS2 install) or the more broad areas in which us at GitHub can do better, I'm finding a lot of what's here helpful.

I would love to collaborate and find a technical solution that fits all the use cases with as little conflicts as possible.

thank you @eine, I would love for us to be able to do that as well and based on the contributions from @MSP-Greg on a bunch of areas in this repo (thank you!) I hope he can join us too.

If it's alright with you all, I would like to defer the discussions about how we did things in the past and instead turn this issue around as a sample of how maybe we can do them going forward. For context, I manage the team that maintains this repo and delivers the images built from it. For better or worse there's some history to the repo and with it that history there are constraints in how we can make some changes since they might break users in unexpected ways (like when they inadvertently depend on tools in the path that were added there not sure how).

I'm obviously not an expert on MSYS2 or know well how users use it (and neither is anyone in our team, there are too many tools in here) thus for cases like this we depend on you all to help us make the right decisions.

I think the goal of @dscho's suggested fix (thank you!) is clear and we want to stick with it which is: get rid of Git internal tools from path.

The problem with just removing them as stated by @maxim-lobanov is that this will surely break customers so he proposed we make sure the MSYS2 tools are in the path to replace anything that gets lost from Git internal tools.

Am I correct by saying that Maxim's proposal is the correct first step so that we don't get into an emergency situation when Git for Windows stops shipping the tools?

From what I gather, it sounds like we should improve on how MSYS2 is installed so that it can be better used. I propose that we move that conversation to #1572 because I really want us to deliver MSYS2 in the best way possible such that it can cooperate with 3rd party Actions to get the most value for anyone needing to use it. @eine, @MSP-Greg - I would love for both of you to join me there so we can figure out what's best for MSYS2 delivery. I understand we might have rushed over decisions in previous iterations but we would like to give it another try.

eine commented 3 years ago

@alepauly, so glad to have a reference to talk to. As you said, let's leave the past away and try to do it well enough this time!

IMHO, the main concern to address is that bash does not exist on Windows per se. Yet, IIRC, in the beginning bash was the "default shell" on windows-latest. Not sure when that changed to powershell, or whether it was the same for other windows environments. Anyway, now, the documentation states the following:

https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#using-a-specific-shell The default shell on non-Windows platforms with a fallback to sh. When specifying a bash shell on Windows, the bash shell included with Git for Windows is used.

Hence, currently, this issue is not about some users relying on Git for Windows. In practice, all users of GitHub Actions which use shell: bash on Windows do currently depend on Git for Windows. Those are probably all the users relying on matrices with multiple OSs.

So, my first proposal is: instead of using a single bash by default on Windows, provide a mechanism in workflow files for users to define what bash is to be used. In practice, there are other shells on Linux that users might want to use. Therefore, it might be sensible to use sh instead of bash. Then, options might be:

I totally made those names up. For example, setup-msys2 provides a single entrypoint (msys2.cmd) and the three shells are selected through an environment variable.

In order to achieve this, some script would be required which parsed the option and configured the environment accordingly. Currently, we have a shell field that is global for each job. Hence, matching scripts need to be provided in the image only. No need to add new fields in the YAML.

See actions/toolkit#318. There, I proposed this feature to be exposed through the toolkit. However, there is no need to have it in the toolkit, as long as the functionality is provided as a built-in script in the environment. So, to start with, three scripts can be created only:

Then, you might make bash an alias of msys2.cmd (setting envvar to MINGW64), for providing a seamless replacement for "many" users, as @maxim-lobanov suggested.

After the entrypoint is clear, and the options are clearly explained, I believe it will be easier to focus on the specific details of each bash:


Regarding, Git for Windows, @dscho can you help me understand the philosophy? I.e., does it provide any feature which is not available in MSYS2 (IDK, git-lfs maybe?). I thought that the main motivation for Git for Windows is providing a more lightweight environment, instead of requiring a full MSYS2 install for using git only. Hence, it would not make much sense to keep Git for Windows, as long as a usable MSYS2 exists. Nonetheless, please, prove me wrong. It might be needed for users that know why they are using it.


I did neither use WSL, but there have been some discussion in other threads, and I believe that a user was creating some Action similar to setup-msys2. EDIT: see #50

maxim-lobanov commented 3 years ago

Hello everyone! We have published PR for changes that we have discussed (that was proposed in last message from @eine ): https://github.com/actions/virtual-environments/pull/1648

These changes should improve UX and keep backward compatibility. We will be very appreciate for your review @eine @dscho @MSP-Greg

dscho commented 3 years ago

Regarding, Git for Windows, @dscho can you help me understand the philosophy? I.e., does it provide any feature which is not available in MSYS2 (IDK, git-lfs maybe?)

@eine Git for Windows is intended as a stand-alone solution, not an entire distribution with package management and more options most users will ever care about. It comes with a graphical installer that is tailored to Git users' needs. Therefore, it is definitely not a replacement for MSYS2, nor will MSYS2 ever be an appropriate replacement for Git for Windows.

eine commented 3 years ago

Therefore, it is definitely not a replacement for MSYS2, nor will MSYS2 ever be an appropriate replacement for Git for Windows.

Is that true in a CLI context too? That is, apart from the graphical installer (which is not visible to GitHub Actions users), is there any other feature that Git for Windows' git CLI interface provides, which is not provided by MSYS2's git CLI package?

dscho commented 3 years ago

[...] apart from the graphical installer (which is not visible to GitHub Actions users), is there any other feature that Git for Windows' git CLI interface provides, which is not provided by MSYS2's git CLI package?

Yes. As I mentioned, the user experience of Git for Windows does not involve package management. For example, the MinGit subset of Git for Windows, as well as the Portable Git edition, come with a bundled, specific Git LFS version, and likewise Git Credential Manager (Core) is bundled, too.

To re-iterate: Git for Windows caters to a different (although slightly overlapping) audience than MSYS2.

bkaidbb commented 3 years ago

This ended up breaking all my CI builds though this post was easy to find and the fix turned out to be quick to find and fix as well.

I had a bash script: reportgenerator "-reports:**/coverage.opencover.xml" "-targetdir:./coverage" "-reporttypes:Cobertura"

and per this article I updated it with: reportgenerator "-reports:**//coverage.opencover.xml" "-targetdir:./coverage" "-reporttypes:Cobertura" as MSYS2 likes to mess with paths.

Still patiently waiting for over a year now for PublishCodeCoverageV2 to be released so I can remove that step.

dscho commented 3 years ago

@bkaidbb you might want to consider using MSYS2_ARG_CONV_EXCL to prevent MSYS2 from trying to translateyour Unix-style paths.

ihnorton commented 3 years ago

This appears to be rolling out now? And it seems to break azure pipelines which select Python with UsePythonVersion because:

I am seeing above in Current image version: '20201102.0'.

maxim-lobanov commented 3 years ago

Yep, we are working to rollback this image due to an issue with python. See details in https://github.com/actions/virtual-environments/issues/1986#issuecomment-722158566

eine commented 3 years ago

@maxim-lobanov can you please consider NOT adding any Git4Windows, MSYS2, MINGW... to the PATH, and instead letting users set their shells to bash, if that is what they want?

dscho commented 3 years ago

@eine I think the concern is that that would break even more users. See e.g. https://github.com/actions/virtual-environments/issues/1525#issuecomment-722034529

maxim-lobanov commented 3 years ago

@eine , I think we have already discussed it previously. A lot of actions (including official GitHub actions and Azure DevOps tasks) depend on those tools in PATH. Even if those tasks are updated to avoid using Unix tools in PATH, a lot of customers will continue to use previous versions of those tasks that still depend on it. We don't see the way to get rid of Unix tools in PATH in near future.

eine commented 3 years ago

I think the concern is that that would break even more users.

@dscho, at this point, I believe any change will break many users. May it be this week, or in a few months. IMHO it is better to document how to proceed, rather than trying to deal with the multiple conflicts that will arise. Adding C:/msys64/mingw64/bin to the PATH should be a single line, but the complexity relies on the implication of that action.

Note that the C:/msys64/mingw64/bin distributed in this environment is NOT a plain minimal MSYS2 install. That would be an acceptable replacement to Git4Windows' internal tools, to some extent. Unfortunately, this environment includes up to 1-2GB of other extra tools (#1572).

A lot of actions (including official GitHub actions and Azure DevOps tasks) depend on those tools in PATH.

We don't see the way to get rid of Unix tools in PATH in near future.

That's why I suggested previously that the relevant tools (https://github.com/actions/virtual-environments/issues/1525#issuecomment-688395017) are explicitly symlinked, if that's the purpose. I agree it's very uncomfortable to maintain. But, honestly, I see no other solution if you want a direct replacement without neither missing nor added tooling in the PATH.

EDIT

mkdir /c/unix-tools
for tool in LIST_OF_DESIRED_UNIX_TOOLS; do
  ln -s /c/msys64/mingw64/bin/$tool /c/unix-tools/$tool
done
export PATH=$PATH:/c/unix-tools
michaelsbradleyjr commented 3 years ago

I read through all the discussion above and tried some workarounds based on info here and in linked PRs and issues, but I'm faced with a problem that I can't reproduce locally:

My Windows laptop and VMs, as well as those of my coworkers, are using a vanilla Git for Windows bash environment (plus some tools installed with e.g. scoop). Until recently, the Windows environment on GitHub Actions was similar enough that complex builds that worked locally also worked in GitHub Actions Windows environments. But in recent days they started breaking and eventually I found this GitHub issue and I'm 99.9999999% confident that breakage is related to the changes involved in this issue.

When it breaks it usually has to do with something deep inside the build system of an upstream repo (git submodule): a configure script wrote a Makefile, which runs a .tcl script and somewhere along the way a /d/ style path got crosswise wth a D:\ style path and it fails. But that upstream repo didn't change, so it's something in our code or the Windows environment, and then we find that a GHA build that used to work on Windows (same commit in our repo) no longer does.

Please tell me there's some simple way to roll back to a Git for Windows bash environment. This kind of thing is especially difficult and painful when (1) you barely use Windows except to make sure a project can build on Windows per project requirements, (2) you barely know anything about msys, mingw, etc. except that a vanilla Git for Windows bash environment "just works".

maxim-lobanov commented 3 years ago

@michaelsbradleyjr, changes in this issue shouldn't affect you because they were rolled back 7 days ago and for now bash come from Git For Windows as previously before these changes.

michaelsbradleyjr commented 3 years ago

@michaelsbradleyjr, changes in this issue shouldn't affect you because they were rolled back 7 days ago and for now bash come from Git For Windows as previously before these changes.

Something is still not working as expected, though:

Here is a successful run on all platforms including Windows: https://github.com/status-im/nim-sqlcipher/runs/1295372559?check_suite_focus=true

Here's a run where it doesn't work on Windows because a path is getting mangled: https://github.com/status-im/nim-sqlcipher/runs/1386262236?check_suite_focus=true

The branch for the failing run is one commit ahead and only involved addition of a comment; afaict nothing else changed in our repo nor upstream, so the conclusion (maybe wrong, I'll grant) is that something has changed in the Windows environment. I've easily spent 30+ hours trying to poke at this in all sorts of ways. At this point I've been asked to move on and our team will live with builds/tests on Windows being broken in GitHub Actions and that for Windows deliverables we'll have to build on one of our local Windows machines... but that's really terrible.

maxim-lobanov commented 3 years ago

@michaelsbradleyjr Both builds definitely use Git for Windows, you can see it in step details: Screen Shot 2020-11-11 at 9 05 08 PM

I see a bunch of other update between these images, see image diff: https://github.com/actions/virtual-environments/pull/1875/files. Is there anything that could potentially impact you? if suggestion above doesn't help, feel free to log new issue in repository

michaelsbradleyjr commented 3 years ago

That's definitely a good question question. I had previously compared this list and this list, which seems to be an equivalent to the diff you linked, but I reviewed the latter just in case. I don't see anything that could potentially impact me.

In the successful run this path (or /d/ equiv) is calculated and resolved inside a .tcl script:

D:/a/nim-sqlcipher/nim-sqlcipher/vendor/sqlcipher/src/shell.c.in

In the failing run the same path is calculated wrongly as:

/d/a/nim-sqlcipher/nim-sqlcipher/vendor/sqlcipher/D:/a/nim-sqlcipher/nim-sqlcipher/vendor/sqlcipher/src/shell.c.in

It looks like it has something to do with an automatic path transformation related to MSYS2 or Git for Windows, but I'm currently at a complete loss how to diagnose it further since I can't reproduce it locally and I'm not sure what else to poke at with echos, etc. in GitHub Actions.

Thanks for your time.

michaelsbradleyjr commented 3 years ago

I just manually updated Git for Windows locally — it periodically checks for and prompts you to install the latest release, but when I checked just now it was behind by a couple of releases.

After the update, I'm encountering the exact same problem locally as on GitHub Actions (per my comments above).

I'll see what I can figure out re: changes in Git for Windows.

eine commented 3 years ago

you barely use Windows except to make sure a project can build on Windows per project requirements

I strongly suggest using a PKGBUILD file for MSYS2's MINGW shells, rather than trying to manually describe all the dependency installation and build process.

dscho commented 3 years ago

you barely use Windows except to make sure a project can build on Windows per project requirements

I strongly suggest using a PKGBUILD file for MSYS2's MINGW shells, rather than trying to manually describe all the dependency installation and build process.

I think that's missing the point. @michaelsbradleyjr is not trying to build an MSYS2 package.

eine commented 3 years ago

@dscho, I didn't suggest creating/maintaining an MSYS2 package, but using a PKGBUILD file for dealing with setting up the dependencies. Michael said they are only interested in having the project built, not having it distributed in any usable package.

dscho commented 3 years ago

@eine the report said that they do not even have MSYS2 installed on their agents, and that they want to spend as little time on the Windows side as possible, which I understand. So I think that doubling down on that PKGBUILD idea will get us exactly nowhere.

eine commented 3 years ago

@dscho, I'm not sure I understand your reasoning. They have MSYS2 installed, since they were hit by changes to the virtual environment in this repo. Moreover, they will use MSYS2 by default (as soon as this issue is closed), because they set the shell to bash. In that context, using pacman for installing dependencies seems to be the most straightforward solution for spending as little time as possible on the Windows side. PKGBUILD files are nothing other than bash scripts with a few fields, precisely for specifying the dependencies on different envs, instead of dealing with package names manually.

Alternatively, they might modify their workflows for using Git for Windows internal tools even after this issue is done. However, I thought the purpose of this change was precisely to avoid people using an env that can change without prior notice.

dscho commented 3 years ago

They have MSYS2 installed

That's simply not true.

eine commented 3 years ago

They have MSYS2 installed

That's simply not true.

Oh, you meant locally. That's correct. I thought we were talking about GitHub Actions.

dscho commented 3 years ago

They have MSYS2 installed

That's simply not true.

Oh, you meant locally. That's correct. I thought we were talking about GitHub Actions.

I was. They were. We're talking about self-hosted agents.

eine commented 3 years ago

I followed the references in https://github.com/actions/virtual-environments/issues/1525#issuecomment-725547009 and I didn't identify those as self-hosted agents. Could you please clarify how you reached that conclusion? My understanding is that self-hosted agents don't pull the virtual environment provided in this repo.