Skip to content

Add warning about using SHELLOPTS to set igncr#22

Merged
jon-turney merged 1 commit intocygwin:masterfrom
tobil4sk:readme-SHELLOPTS-warning
Apr 6, 2025
Merged

Add warning about using SHELLOPTS to set igncr#22
jon-turney merged 1 commit intocygwin:masterfrom
tobil4sk:readme-SHELLOPTS-warning

Conversation

@tobil4sk
Copy link
Copy Markdown
Contributor

@tobil4sk tobil4sk commented Feb 4, 2025

By default, SHELLOPTS is a shell variable and moving it to the environment causes all shell options to propagate to child shells. This may break scripts in a confusing way, so it is not a good idea to recommend it without at least providing a warning.

Perhaps cygwin-install-action could provide an option to set up BASH_ENV with a file containing set -o igncr. setup-ocaml does this when configuring cygwin.

See:

@jon-turney
Copy link
Copy Markdown
Member

jon-turney commented Apr 6, 2025

Thanks.

It sounds like really we should only be recommending using '-o igncr', since having SHELLOPTS in the environment causes this propagation, so I might do a bit of tweaking on the wording...

@tobil4sk tobil4sk deleted the readme-SHELLOPTS-warning branch April 6, 2025 21:49
@tobil4sk
Copy link
Copy Markdown
Contributor Author

tobil4sk commented Apr 6, 2025

Thanks!

It sounds like really we should only be recommending using '-o igncr'

Yes, this sounds like the best idea to me

@gstrauss
Copy link
Copy Markdown
Contributor

gstrauss commented Apr 7, 2025

FYI: I use shell: D:\cygwin\bin\bash.exe --noprofile --norc -o igncr -eo pipefail '{0}'
in lighttpd .github/workflows/ci.yml
https://github.com/lighttpd/lighttpd1.4/blob/master/.github/workflows/ci.yml#L260

@jon-turney
Copy link
Copy Markdown
Member

Well, ideally, GitHub would give us control of the line endings written into the file whose name is substituted for {0}, but that seems unlikely...

If I recall correctly, igncr is a special Cygwin thing that we patch into bash, to deal with this sort or situation, so maybe that needs to be mentioned as well.

I was musing if the brief usage at the top of the README needs an example to demonstrate how to write it with a multiline YAML literal block like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants