actions / runner

The Runner for GitHub Actions :rocket:
https://github.com/features/actions
MIT License
4.77k stars 931 forks source link

Optionally use *nix line endings for Windows scripts #912

Open me-and opened 3 years ago

me-and commented 3 years ago

When running command line programs on Windows, the temporary script that gets created is forced to use Windows-style line endings. I'd like that behaviour to be configurable: it's a very sensible default, but it causes problems when using non-default shells that expect *nix style line endings.

For context, I maintain a number of Cygwin packages, and I'd like to be able to use the GitHub runner infrastructure to build those. That requires using Cygwin Bash, which doesn't support CRLF line endings in scripts. I'd like to be able to specify Cygwin's Bash as the shell to run commands in, but currently that fails.

Currently, I'm using commands like this (slightly contrived but working example):

run: |
  C:\tools\cygwin\bin\bash.exe -c 'cygport moreutils.cygport build'
  C:\tools\cygwin\bin\bash.exe -c 'cygport moreutils.cygport compile'
  C:\tools\cygwin\bin\bash.exe -c 'cygport moreutils.cygport test'

I'd like to be able to do things like this instead:

run: |
  cygport moreutils.cygport build
  cygport moreutils.cygport compile
  cygport moreutils.cygport test
shell: bash

Currently that works, apart from the line ending issue – the final argument gets a \r attached to it, which causes the command to not be recognised.

The behaviour here is triggered by ScriptHandler.cs line 241. I imagine the new behaviour would work like the current behaviour by default, but with a new optional eol parameter that can appear everywhere the current shell parameter can (e.g. defaults.run.eol and jobs.<job_id>.steps[*].eol). That parameter would accept arguments of crlf or lf (and perhaps more esoteric options) to force particular line ending behaviour.

ethomson commented 2 years ago

I don't really love the idea of adding a new eol parameter here, if I'm honest, but I am super sympathetic to this problem. I wonder if we might want to be a little smarter and when the shell is bash we use \n instead of \r\n? 🤔

We might also be able to do some trimming at the end.

Something for us to take a look at in the new year. Thanks for the suggestion.

me-and commented 2 years ago

Yep, entirely fine with some other solution! An eol parameter was the idea that jumped to my mind, as it seemed to offer maximum flexibility while still being entirely predictable and not changing the current behaviour for anyone, but anything that lets me run a script on Windows without CRLF line endings would be good for me.

me-and commented 2 years ago

I have just discovered an alternative solution, at least for my purposes: apparently Cygwin Bash has a igncr option which will tell it to ignore carriage returns, to avoid exactly this problem.

I sorta feel like this is a more general problem than just my specific use case, but equally clearly nobody else has grumbled before, so I think I don't have strong feelings either way about whether this actually gets some general fix that helps with non-Cygwin use cases, or if this just gets closed as not being needed after all.