Skip to content

Commit

Permalink
Run Cygwin CI workflow commands in login shells
Browse files Browse the repository at this point in the history
This passes --login to the bash shell used to run commands in the
Cygwin environment on CI. This eliminates the need to work around a
partly broken environment, and the extra code what was used to do
that is accordingly removed. There are two benefits of this change:

- The PATH is correct: Cygwin's /usr/local/bin and /usr/bin are
  present at the beginning of PATH. Otherwise, it is easy to get
  /usr/bin at the front, but rather involved to get /usr/local/bin
  to precede it. Because Python on Cygwin puts scripts/executables
  such as the upgraded "pip" and the "pytest" command in
  /usr/local/bin, it is valuable to have that directory in the PATH
  and best to have it before /usr/bin. (I have set CYGWIN_NOWINPATH
  to omit other directories, since finding any of the commands to
  be run in the Cygwin environment outside that environment is
  unintended.)

- Every step automatically has correct temporary directories: When
  Cygwin commands were not being run in login shells, they didn't
  automatically get correct values for TMP and TEMP for their
  environment. To work around this, those environment variables
  were set globally, for every step. But that caused them to refer
  to nonexistent locations for steps such as actions/checkout. Most
  likely this would not cause any errors, but it did cause copious
  warnings about a nonexistent temporary directory, which risked
  obscuring other potentially important output. Now that Cygwin
  commands run in login shells, both the few non-Cygwin steps, and
  the steps run in the Cygwin enviroment, all get correct temporary
  directories (with TMP and TEMP set in the prewritten startup
  script the login shell uses).

A theoretical disadvantage of this is that login shells take
slightly longer to start up, but that delay is insigificant in
this application. A more significant disadvantage is that setting
the -x shell option the way it was done before would produce a lot
of noise at the beginning of the output for every command-running
step. To work around that, -x is omitted from the value of "shell"
and "set -x" is added at the end of the startup script for login
shells, so it runs before each step's "payload" command, but
without applying to the commands run in the startup script itself.
  • Loading branch information
EliahKagan committed Oct 14, 2023
1 parent 10e2960 commit e8956e5
Showing 1 changed file with 15 additions and 9 deletions.
24 changes: 15 additions & 9 deletions .github/workflows/cygwin-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,36 @@ jobs:
fail-fast: false

env:
CHERE_INVOKING: 1
TMP: "/tmp"
TEMP: "/tmp"
CHERE_INVOKING: "1"
CYGWIN_NOWINPATH: "1"

defaults:
run:
shell: C:\cygwin\bin\bash.exe --noprofile --norc -exo pipefail -o igncr "{0}"
shell: C:\cygwin\bin\bash.exe --login --norc -eo pipefail -o igncr "{0}"

steps:
- name: Force LF line endings
run: |
git config --global core.autocrlf false # Affects the non-Cygwin git.
shell: bash
shell: bash # Use Git Bash instead of Cygwin Bash for this step.

- uses: actions/checkout@v4
with:
fetch-depth: 0
submodules: recursive

- uses: cygwin/cygwin-install-action@v4
- name: Install Cygwin
uses: cygwin/cygwin-install-action@v4
with:
packages: python39 python39-pip python39-virtualenv git
add-to-path: false # No need to change $PATH outside the Cygwin environment.

- name: Special configuration for Cygwin's git
- name: Arrange for verbose output
run: |
# Arrange for verbose output but without shell environment setup details.
echo 'set -x' >~/.bash_profile
- name: Special configuration for Cygwin git
run: |
git config --global --add safe.directory "$(pwd)"
git config --global core.autocrlf false
Expand All @@ -57,7 +63,7 @@ jobs:
- name: Install project and test dependencies
run: |
python -m pip install ".[test]"
pip install ".[test]"
- name: Show version and platform information
run: |
Expand All @@ -71,4 +77,4 @@ jobs:
- name: Test with pytest
run: |
python -m pytest --color=yes -p no:sugar --instafail -vv
pytest --color=yes -p no:sugar --instafail -vv

0 comments on commit e8956e5

Please sign in to comment.