Vampire / setup-wsl

A GitHub action to install and setup a Linux distribution for the Windows Subsystem for Linux (WSL)
Apache License 2.0
107 stars 21 forks source link

Document crlf glitches #5

Closed kousu closed 2 years ago

kousu commented 3 years ago

Hi! First, let me just say: THANK YOU SO MUCH FOR THIS. It's saved me a ton of time already.

While implementing testing on WSL, I ran into the bizarre error ./.ci.sh: line 4: $'\r': command not found, e.g. https://github.com/neuropoly/spinalcordtoolbox/runs/1606955337?check_suite_focus=true from https://github.com/neuropoly/spinalcordtoolbox/blob/af36bdedc2ee789a370f101bfec868cdd7ea1e76/.ci.sh.

This is because this runs Linux on top of Windows, so actions/checkout@v2 mangles the line-endings and then Linux sees \rs and gets confused. To fix this, I needed to force core.autocrlf off:

    steps:
    - uses: Vampire/setup-wsl@v1
      with:
        distribution: ${{matrix.container}}
    - name: Use unix line endings
      shell: bash
      run: |
        # Github's actions/checkout@v2 when run on Windows mangles the line-endings to DOS-style
        # but we're running Linux *on top* of Windows, so we need to not mangle them!
        # https://github.com/actions/checkout/issues/135#issuecomment-602171132
        # https://github.com/actions/virtual-environments/issues/50#issuecomment-663920265
        git config --global core.autocrlf false
        git config --global core.eol lf

There's an open bug about it: https://github.com/actions/checkout/issues/135 / https://github.community/t/git-config-core-autocrlf-should-default-to-false/16140/7 but Github doesn't seem interested in fixing it; I bet this is a pretty common gotcha specifically when using WSL, so do you think you could document it in your docs? I see you're very aware of the issue but like most people I had no clue I would run into it until it showed up in my logs, and it took even longer to figure out I needed to add shell: bash to the suggested fix counteract your advice to set the default shell to wsl (I only figured it out by half guessing; I saw it here and tried it and it worked).

I see people in those threads, including you, recommending .gitattributes. In my case, maybe something like this would have done:

# .gitattributes
*.sh text eol=lf

That's certainly a fix, but my problem comes narrowly from trying to layer Linux on top of Windows, and with docker in the mix. In 99.99% of WSL installs, our users are going to be installing using git from inside WSL, so the glitch will never show up. It's only because CI's actions/checkout@v2 is running in its own container-on-Windows and deciding that that means it should mangle line endings, so I would rather counteract it in our CI script instead of doing it project-wide.

jmoguillansky-gpsw commented 3 years ago

I had a similar issue as well, and I used the gitattributes fix.
Even though autocrlf may not be enabled by default in wsl, it might be in a user's installation, so perhaps better to enforce that autocrlf is disabled for bash scripts (via a gitattributes file)

Vampire commented 2 years ago

Hey there, sorry for the long delay. I thought about this a bit.

I'm sorry, but I don't think I'll add that to the documentation of the action. One point is that I would recommend setting .gitattributes, then the next person comes and requests to change the advice to using autocrlf and so on.

This is a general Git issue. Imho you should always and in any Git repository that potentially ever is cloned on multiple operating systems (so basically each and every repository) add a .gitattributes file that pins *.bat line endings to CRLF and shell script line endings to LF. Even if you only ever clone on Windows without WSL you could get in trouble if you for example use Cygwin.

I simply think this is out of scope of the documentation for my action.

But I'll move this issue to the discussions section instead of closing it for better discoverability.