Merge pull request #1872 from EliahKagan/run-ci/workflow-readability

Clarify `pure-rust-build` cc1 wrapping

Author: Byron

.github/workflows/ci.yml

@@ -38,7 +38,7 @@ jobs:
           )
           apt-get update
           apt-get install --no-install-recommends -y -- "${prerequisites[@]}"
-        shell: bash
+        shell: bash  # This step needs `bash`, and the default in container jobs is `sh`.
       - name: Verify that we are in an environment with limited dev tools
         run: |
           set -x
@@ -70,31 +70,55 @@ jobs:
           ! grep -qP '(?<!\blinux-raw)-sys\b' tree.txt
       - name: Wrap cc1 (and cc1plus if present) to record calls
         run: |
+          set -o noclobber  # Catch any collisions with existing entries in /usr/local.
+
+          # Define the wrapper script for a compiler driver (for cc1 or cc1plus). This wrapper
+          # records calls, then delegates to the executable it wraps. When recording calls, writes
+          # to the log are synchronized so fragments of separate log entries aren't interleaved,
+          # even in concurrent runs. This wrapper knows what executable it is wrapping because,
+          # when deployed, this wrapper (or a symlink) replaces that executable, which will itself
+          # have been moved aside by being renamed with a `.orig` suffix, so this can call it.
           cat >/usr/local/bin/wrapper1 <<'EOF'
-          #!/bin/sh -e
+          #!/bin/sh
+          set -e
           printf '%s\n' "$0 $*" |
             flock /run/lock/wrapper1.fbd136bd-9b1b-448d-84a9-e18be53ae63c.lock \
             tee -a -- /var/log/wrapper1.log ~/display >/dev/null  # We'll link ~/display later.
           exec "$0.orig" "$@"
           EOF
 
+          # Define the script that performs the wrapping. This script shall be run once for each
+          # executable to be wrapped, renaming it with a `.orig` suffix and replacing it with a
+          # symlink to the wrapper script, defined above.
           cat >/usr/local/bin/wrap1 <<'EOF'
-          #!/bin/sh -e
+          #!/bin/sh
+          set -e
           dir="$(dirname -- "$1")"
           base="$(basename -- "$1")"
           cd -- "$dir"
           mv -- "$base" "$base.orig"
           ln -s -- /usr/local/bin/wrapper1 "$base"
           EOF
 
-          chmod +x /usr/local/bin/wrap1 /usr/local/bin/wrapper1
+          # Define a helper file that, when sourced, wires up the `~/display` symlink `wrapper1`
+          # uses to report calls as GitHub Actions step output (in addition to writing them to a
+          # log file). This is needed because stdout and stderr are both redirected elsewhere when
+          # the wrapper actually runs, and `/dev/tty` is not usable. This must be sourced in the
+          # same step as the `cargo` command that causes wrapped executables to be run, because
+          # different steps write to different pipe objects. (This also needs the shell that
+          # sourced it to remain running. But that is not the cause of the underlying limitation.)
+          cat >/usr/local/bin/set-display.sh <<'EOF'
+          ln -s -- "/proc/$$/fd/1" ~/display
+          EOF
+
+          chmod +x /usr/local/bin/wrapper1 /usr/local/bin/wrap1
           mkdir /run/lock/wrapper1.fbd136bd-9b1b-448d-84a9-e18be53ae63c.lock
 
           find /usr/lib/gcc \( -name cc1 -o -name cc1plus \) \
             -print -exec /usr/local/bin/wrap1 {} \;
       - name: Build max-pure with limited dev tools and log cc1
         run: |
-          ln -s -- "/proc/$$/fd/1" ~/display  # Bypass `cc1` redirection.
+          . /usr/local/bin/set-display.sh
           cargo install --debug --locked --no-default-features --features max-pure --path .
       - name: Show logged C and C++ compilations (should be none)
         run: |
@@ -226,7 +250,6 @@ jobs:
       - name: Compare expected and actual failures
         run: |
           # Fail on any differences, even unexpectedly passing tests, so they can be investigated.
-          # (If the job is made blocking for PRs, it may make sense to make this less stringent.)
           git --no-pager diff --no-index --exit-code --unified=1000000 --color=always -- `
             etc/test-fixtures-windows-expected-failures-see-issue-1358.txt actual-failures.txt
 
@@ -265,7 +288,7 @@ jobs:
           dpkg --add-architecture ${{ matrix.runner-arch }}
           apt-get update
           apt-get install --no-install-recommends -y -- "${prerequisites[@]}"
-        shell: bash
+        shell: bash  # This step needs `bash`, and the default in container jobs is `sh`.
       - uses: actions/checkout@v4
       - name: Install Rust via Rustup
         run: |
@@ -428,7 +451,7 @@ jobs:
 
     defaults:
       run:
-        shell: bash  # Without specifying this, we don't get `-o pipefail`.
+        shell: bash  # Without this, the shell here is `bash` but without `-o pipefail`.
 
     steps:
       - name: Find this workflow

.github/workflows/release.yml

@@ -17,7 +17,7 @@ permissions:
 
 defaults:
   run:
-    shell: bash
+    shell: bash  # Use `bash` even in the Windows jobs.
 
 jobs:
   # Create a draft release, initially with no binary assets attached.