From e1555bd054c13bd141ca168fa677108e5b25a215 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 16 Nov 2024 18:47:54 -0500 Subject: [PATCH 1/8] Have pure-rust-build job find what compiles C code This adds more checks to the `pure-rust-build` CI job to reveal what dependencies of `max-pure` are building C code. The new checks are failing. They relate to #1681 and should pass once that bug is fixed. The main goal is to produce information helpful for #1681. As currently written, the new steps do not fail the job, because the failing steps have `continue-on-error: true`. This is so that regressions (that would fail the preexisting steps) remain readily detected. This should ideally be temporary; the new steps, if kept, should eventually be strengthened so they can fail the job. Currently this includes both regular and build dependencies. There are two new checks: 1. Search the `max-pure` dependency tree for packages that require a C or C++ compiler, or that are in practice only likely to be used to build C or C++ code. If any are found, display the whole tree, with matching lines highlighted, and fail the step. 2. After all steps that need it to be in working order, break GCC in the container by removing the `cc1` binary, then attempt a clean `max-pure` build, to reveal the first failure, if any, and let it fail the step. The `gcc` command itself is needed, because `rustc` calls the linker through it, even when no non-Rust code is compiled as part of the build. As discussed in #1664 comments, installing GCC in a way that is not broken but omits the ability to compile even C code, while it may be possible, is not commonly done and may require long running build steps or a new Docker image. Fortunately, the `gcc` command is just a "compiler driver"; the C compiler in GCC is `cc1`, and this can be removed without breaking most uses of GCC that don't involve actual compilation. This likewise removes `cc1plus` if found, even though it may not be present since we already verified that `g++` is not installed. (The deletion of `cc1` is the important part.) Since the job now cleans and starts a build that fails due to the absence of `cc1`, this removes the `rust-cache` step. (Caching would probably still confer some benefit, since it caches dependencies. Even after running `cargo clean` between the builds, most of these should be reacquired when building with `cc1`. However, to make the whole thing easier to reason about, the step is removed. It can be re-added if the job runs too slowly.) This considers any `-sys` crate to be a dependency that needs to build C. This is in principle not always the case, since they may use existing shared library binaries, though there may still be stub code that has to be built in C. The relevance of the new steps varies depending on precisely how one conceptualizes "pure" in `max-pure`, which is another reason for them to start out as `continue-on-error`. The `-sys` crates this finds in the `max-pure` dependency tree are: - `libsqlite3-sys` via `rusqlite`. This was reported in #1681 and strongly seems to be unintended for `max-pure`. - `linux-raw-sys` via `rustix` and `xattr`. It's less clear whether this should be considered impure, since its purpose is to interact with an operating system facility, and it may be comparable to the use of `libc` (on Unix-like systems). However, this does seem like it would not be able to build without a C compiler. The dependency tree also has an occurrence of the `cc` crate without a related `-sys` dependency, as required by `ring`. The `ring` crate contains a C test program that is built when building `ring`. That is currently the first build error shown in the output of the "Do max-pure build without being able to compile C or C++" step. --- .github/workflows/ci.yml | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bcae2edb8bc..e4fde179cc8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,8 +55,24 @@ jobs: done - name: Install Rust via Rustup run: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile minimal - - uses: Swatinem/rust-cache@v2 - - run: /github/home/.cargo/bin/cargo install --debug --locked --no-default-features --features max-pure --path . + - name: Add Rust tools to path + run: echo "PATH=$HOME/.cargo/bin:$PATH" >> "$GITHUB_ENV" + - name: Generate dependency tree + run: cargo tree --locked --no-default-features --features max-pure > tree.txt + - name: Scan for dependencies that build C or C++ code + run: | + pattern='.*\b(-sys|cc|cmake|pkg-config|vcpkg)\b.*' + ! GREP_COLORS='ms=30;48;5;214' grep --color=always -Ex -C 1000000 -e "$pattern" tree.txt + continue-on-error: true + - name: Do max-pure build with minimal environment + run: cargo install --debug --locked --no-default-features --features max-pure --path . + - name: Clear target + run: cargo clean + - name: Break GCC's ability to compile C or C++ + run: find /usr/lib/gcc \( -name cc1 -o -name cc1plus \) -print -delete + - name: Do max-pure build without being able to compile C or C++ + run: cargo install --debug --locked --no-default-features --features max-pure --path . + continue-on-error: true test: runs-on: ubuntu-latest From 69db5b942aa27073cfb2d4a58050d7f84226899b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 16 Nov 2024 23:28:37 -0500 Subject: [PATCH 2/8] Log `cc1` calls instead of deleting `cc1` --- .github/workflows/ci.yml | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e4fde179cc8..5d0686fd791 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,7 +40,7 @@ jobs: apt-get update apt-get install --no-install-recommends -y -- "${prerequisites[@]}" shell: bash - - name: Verify environment is sufficiently minimal for the test + - name: Verify that we are in an environment with minimal dev tools run: | set -x for pattern in cmake g++ libssl-dev make pkgconf pkg-config; do @@ -64,14 +64,32 @@ jobs: pattern='.*\b(-sys|cc|cmake|pkg-config|vcpkg)\b.*' ! GREP_COLORS='ms=30;48;5;214' grep --color=always -Ex -C 1000000 -e "$pattern" tree.txt continue-on-error: true - - name: Do max-pure build with minimal environment - run: cargo install --debug --locked --no-default-features --features max-pure --path . - - name: Clear target - run: cargo clean - - name: Break GCC's ability to compile C or C++ - run: find /usr/lib/gcc \( -name cc1 -o -name cc1plus \) -print -delete - - name: Do max-pure build without being able to compile C or C++ + - name: Wrap cc1 (and cc1plus if present) to record calls + run: | + cat >/usr/local/bin/wrapper1 <<'EOF' + #!/bin/sh + printf '%s %s\n' "$0" "$*" >>/var/log/wrapper1.log + exec "$0.orig" "$@" + EOF + + cat >/usr/local/bin/wrap1 <<'EOF' + #!/bin/sh -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 + + find /usr/lib/gcc \( -name cc1 -o -name cc1plus \) \ + -print -exec /usr/local/bin/wrap1 {} \; + - name: Build max-pure with minimal dev tools and log cc1 run: cargo install --debug --locked --no-default-features --features max-pure --path . + - name: Show logged C and C++ compilations (should be none) + run: | + ! cat /var/log/wrapper1.log continue-on-error: true test: From 90a59f9af13fa0a1f1cc4950c4f5716c9696e86d Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 17 Nov 2024 00:27:44 -0500 Subject: [PATCH 3/8] Synchronize the log writes --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5d0686fd791..d66c7915e90 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,7 +68,9 @@ jobs: run: | cat >/usr/local/bin/wrapper1 <<'EOF' #!/bin/sh - printf '%s %s\n' "$0" "$*" >>/var/log/wrapper1.log + printf '%s %s\n' "$0" "$*" | + flock /run/lock/wrapper1.fbd136bd-9b1b-448d-84a9-e18be53ae63c.lock \ + tee -a /var/log/wrapper1.log >/dev/null # TODO: Also log to terminal device? exec "$0.orig" "$@" EOF @@ -82,6 +84,7 @@ jobs: EOF chmod +x /usr/local/bin/wrap1 /usr/local/bin/wrapper1 + 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 {} \; From 59e343b4eef59ccdf02fbf1835e2742f49704468 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 17 Nov 2024 00:51:00 -0500 Subject: [PATCH 4/8] Try to also log to terminal, interleaved with cargo output So the `cc1` calls can also be seen in context. `/dev/tty` is used instead of writing to standard output because standard output is expectd to be redirected and may in some cases even be consulted by code in a build script, whereas we really do want to write to the terminal. This also: - Has the `wrap1` script use `-e` so that it will stop at the first error. No command before the last one should fail. If the `flock` command fails, either due to a lock-related error or due to an error in `tee`, we want to know about it. I should've done this before, but it's more important now that I'm also tee-ing to `/dev/tty`: it's not obvious that this would work on CI. - Tweaks the code style of the `printf` command, to make the command slightly more readable, and make clearer that this is really just being used as a portable alternative to `echo`. --- .github/workflows/ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d66c7915e90..46d37d0a5f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,10 +67,10 @@ jobs: - name: Wrap cc1 (and cc1plus if present) to record calls run: | cat >/usr/local/bin/wrapper1 <<'EOF' - #!/bin/sh - printf '%s %s\n' "$0" "$*" | + #!/bin/sh -e + printf '%s\n' "$0 $*" | flock /run/lock/wrapper1.fbd136bd-9b1b-448d-84a9-e18be53ae63c.lock \ - tee -a /var/log/wrapper1.log >/dev/null # TODO: Also log to terminal device? + tee -a /var/log/wrapper1.log /dev/tty >/dev/null exec "$0.orig" "$@" EOF From 37a519a3704dc27370e41083e4d65197533381f7 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 17 Nov 2024 01:29:56 -0500 Subject: [PATCH 5/8] GHA doesn't have `/dev/tty`, let's try this --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 46d37d0a5f6..74dac3f08ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,7 +70,7 @@ jobs: #!/bin/sh -e printf '%s\n' "$0 $*" | flock /run/lock/wrapper1.fbd136bd-9b1b-448d-84a9-e18be53ae63c.lock \ - tee -a /var/log/wrapper1.log /dev/tty >/dev/null + tee -a -- /var/log/wrapper1.log ~/ci-stdout >/dev/null exec "$0.orig" "$@" EOF @@ -85,6 +85,7 @@ jobs: chmod +x /usr/local/bin/wrap1 /usr/local/bin/wrapper1 mkdir /run/lock/wrapper1.fbd136bd-9b1b-448d-84a9-e18be53ae63c.lock + ln -s -- "/proc/$$/fd/1" ~/ci-stdout find /usr/lib/gcc \( -name cc1 -o -name cc1plus \) \ -print -exec /usr/local/bin/wrap1 {} \; From d7d80cf829dbd7d0658a2ec8f4a58ffb6e9f3fde Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 17 Nov 2024 01:33:06 -0500 Subject: [PATCH 6/8] Some troubleshooting --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 74dac3f08ca..e44d41b2635 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,6 +85,7 @@ jobs: chmod +x /usr/local/bin/wrap1 /usr/local/bin/wrapper1 mkdir /run/lock/wrapper1.fbd136bd-9b1b-448d-84a9-e18be53ae63c.lock + (set -x; ls -l "/proc/$$/fd/1") # See what's going on. ln -s -- "/proc/$$/fd/1" ~/ci-stdout find /usr/lib/gcc \( -name cc1 -o -name cc1plus \) \ From f0c7d429abcf8c42c67c6214f4ba6f777b53e3a1 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 17 Nov 2024 01:45:07 -0500 Subject: [PATCH 7/8] Set display symlink in the same step Because it uses the `$$` PID of the per-step shell. (I think the pipe object it goes to may differ across steps too.) This change gets interleaved high-level `cargo` output and lower level `cc1` logging, without any other lower level commands as would be obtained by telling `cargo` to produce more verbose output. Those two kinds of output are displayed together from the `cargo install` step. The full log of all `cc1` commands is again displayed (unless there are no C or C++ builds, then it wouldn't be created) in the subsequent step. At this point, I think this approach is working better than the preceding approach of deleting `cc1` (and, if present, `cc1plus`) so `cargo` fails if anything uses it. This makes it easier to figure out everything that will try to run `gcc` in ways that result in `gcc` running `cc1` (or `cc1plus`), which should usually only happen in order to compile C (or C++) code or to perform a closely related operation. The following two changes are deliberately not made at this time: - Caching with the `rust-cache` action is not added back to this job. Although the build is no longer inefficient and complex (as in the approach of building, cleaning, breaking GCC, and building again) here there is a simpler reason not to cache: we want to find out if *any* build step, including for dependencies, is compiling C (or C++). Caching could potentially hide that, if a cached build artifact that had done so were able to be reused. - No attempt is made to deparallelize the build. It would be useful to do something like `-j1` so that the interleaved log output would always pertain to an immediately adjacent step. However, that is usually already the case, and native code builds are not necessarily successfully deparallelized just by telling `cargo` to only do one task at a time, since they use custom commands (and even sometimes build systems) controlled through per-crate `build.rs` code and build dependencies. Also, since we're not caching, allowing parallelism may be useful to preserve speed. (The `flock` command is only locking logging, not the actual compilations.) --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e44d41b2635..5215114aa99 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,7 +70,7 @@ jobs: #!/bin/sh -e printf '%s\n' "$0 $*" | flock /run/lock/wrapper1.fbd136bd-9b1b-448d-84a9-e18be53ae63c.lock \ - tee -a -- /var/log/wrapper1.log ~/ci-stdout >/dev/null + tee -a -- /var/log/wrapper1.log ~/display >/dev/null # We'll link ~/display later. exec "$0.orig" "$@" EOF @@ -85,13 +85,13 @@ jobs: chmod +x /usr/local/bin/wrap1 /usr/local/bin/wrapper1 mkdir /run/lock/wrapper1.fbd136bd-9b1b-448d-84a9-e18be53ae63c.lock - (set -x; ls -l "/proc/$$/fd/1") # See what's going on. - ln -s -- "/proc/$$/fd/1" ~/ci-stdout find /usr/lib/gcc \( -name cc1 -o -name cc1plus \) \ -print -exec /usr/local/bin/wrap1 {} \; - name: Build max-pure with minimal dev tools and log cc1 - run: cargo install --debug --locked --no-default-features --features max-pure --path . + run: | + ln -s -- "/proc/$$/fd/1" ~/display # Bypass `cc1` redirection. + cargo install --debug --locked --no-default-features --features max-pure --path . - name: Show logged C and C++ compilations (should be none) run: | ! cat /var/log/wrapper1.log From f45f23ea720b2458368caec8c6ec4f17531ec5d1 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 17 Nov 2024 02:39:49 -0500 Subject: [PATCH 8/8] Clarify step names This changes "minimal" to "limited" in the human-readable step names that characterize the omission of some dev tools from the container environment in `pure-rust-build`. - "Minimal" has connotations of being a lower bound (so no matter how much one has, if it is at least so much, it satisfies a minimal requirement), while "limited" emphasizes the restriction. - `minimal` is a profile name for `rustup`, while no profile name coincides with or is strongly similar to "limited". --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5215114aa99..32d6177c148 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,7 +40,7 @@ jobs: apt-get update apt-get install --no-install-recommends -y -- "${prerequisites[@]}" shell: bash - - name: Verify that we are in an environment with minimal dev tools + - name: Verify that we are in an environment with limited dev tools run: | set -x for pattern in cmake g++ libssl-dev make pkgconf pkg-config; do @@ -88,7 +88,7 @@ jobs: find /usr/lib/gcc \( -name cc1 -o -name cc1plus \) \ -print -exec /usr/local/bin/wrap1 {} \; - - name: Build max-pure with minimal dev tools and log cc1 + - name: Build max-pure with limited dev tools and log cc1 run: | ln -s -- "/proc/$$/fd/1" ~/display # Bypass `cc1` redirection. cargo install --debug --locked --no-default-features --features max-pure --path .