Add more robust `debug_assertions` verification to CI. (#751)

When a workspace uses `debug_assertions` for conditional compilation
(*like Xilem does*), especially of the `#[cfg(debug_assertions)]` kind,
it is not sufficient to just test with it either set to `true` or
`false`. Both need testing as we don't know in advance which path
contains faulty code.

An initial response to this issue was done in #431 which added some
`--release --all-targets` steps to the CI.

This PR here improves that verification in some key ways:
* In addition to the stable toolchain, also verify MSRV.
* Only turn off the `debug_assertions` configuration option as that is
the differentiator for our testing. The full `--release` option spends
time on optimizations. It may be worth doing occasional `--release`
tests to detect bugs in code optimization, but those checks aren't worth
it as part of the `cargo clippy` / `cargo check` bundle we run most
frequently.
* No longer use `--all-targets` which has known issues as explained in
the rationale section of the CI script. Instead we duplicate the regular
steps with `cargo-hack`.
* External dependencies are always built with `debug_assertions` set to
`true` which allows for cache reuse. This is a trade-off. I think the
cache reuse performance gains are worth it for regular CI runs. However
some external dependency might e.g. sneak in a higher MSRV requirement
into a `#[cfg(not(debug_assertions))]` block. Unlikely but possible.
This is again where a less frequent `--release` check would help. Maybe
scheduled, maybe just before publishing a new version. In any case,
future work.

Additionally there is a new env variable `USING_DEBUG_ASSERTIONS` and a
presence verification script `debug_assertions.sh`. These in combination
make the CI script generic across workspaces regardless of
`debug_assertions` usage. The env variable dictates whether we will
spend the time doing the extra compiling. The bash script will detect if
the variable goes out of sync with actual usage in code.

.github/debug_assertions.sh

@@ -0,0 +1,29 @@
+#!/bin/bash
+
+# Check all the standard Rust source files
+output=$(rg "debug_assertions" -g "*.rs" .)
+
+if [ -z "$output" ]; then
+	if [ "$USING_DEBUG_ASSERTIONS" = "true" ]; then
+		echo "Could not find any debug_assertions usage in Rust code."
+		echo "The CI script must be modified to not expect usage."
+		echo "Set USING_DEBUG_ASSERTIONS to false in .github/workflows/ci.yml."
+		exit 1
+	else
+		echo "Expected no debug_assertions usage in Rust code and found none."
+		exit 0
+	fi
+else
+	if [ "$USING_DEBUG_ASSERTIONS" = "true" ]; then
+		echo "Expected debug_assertions to be used in Rust code and found it."
+		exit 0
+	else
+		echo "Found debug_assertions usage in Rust code."
+		echo ""
+		echo $output
+		echo ""
+		echo "The CI script must be modified to expect this usage."
+		echo "Set USING_DEBUG_ASSERTIONS to true in .github/workflows/ci.yml."
+		exit 1
+	fi
+fi

.github/workflows/ci.yml

@@ -16,6 +16,8 @@ env:
   NO_WASM_PKGS: "--exclude masonry --exclude xilem"
   # Only some of our examples support Android (primarily due to extra required boilerplate).
   ANDROID_TARGETS: "-p xilem --example mason_android --example calc_android --example stopwatch_android --example variable_clock_android --example http_cats_android --example to_do_mvc_android"
+  # Whether the workspace contains Rust code using the debug_assertions configuration option.
+  USING_DEBUG_ASSERTIONS: "true"
 
 
 # Rationale
@@ -43,6 +45,9 @@ env:
 # The MSRV jobs run only cargo check because different clippy versions can disagree on goals and
 # running tests introduces dev dependencies which may require a higher MSRV than the bare package.
 #
+# If the workspace uses debug_assertions then we verify code twice, with it set to true or false.
+# We always keep it true for external dependencies so that we can reuse the cache for faster builds.
+#
 # We don't save caches in the merge-group cases, because those caches will never be re-used (apart
 # from the very rare cases where there are multiple PRs in the merge queue).
 # This is because GitHub doesn't share caches between merge queues and the main branch.
@@ -83,6 +88,9 @@ jobs:
       - name: check copyright headers
         run: bash .github/copyright.sh
 
+      - name: check debug_assertions presence
+        run: bash .github/debug_assertions.sh
+
       - name: install cargo-rdme
         uses: taiki-e/install-action@v2
         with:
@@ -122,15 +130,22 @@ jobs:
           save-if: ${{ github.event_name != 'merge_group' }}
 
       - name: cargo clippy
-        run: cargo hack clippy --workspace --locked --optional-deps --each-feature -- -D warnings
+        run: cargo hack clippy --workspace --locked --profile ci --optional-deps --each-feature -- -D warnings
 
       - name: cargo clippy (auxiliary)
-        run: cargo hack clippy --workspace --locked --optional-deps --each-feature --tests --benches --examples -- -D warnings
+        run: cargo hack clippy --workspace --locked --profile ci --optional-deps --each-feature --tests --benches --examples -- -D warnings
 
-      # Verify that we can build in release mode
+      - name: cargo clippy (no debug_assertions)
-      # TODO: Find a way for this to share artifacts with the above job?
+        if: env.USING_DEBUG_ASSERTIONS == 'true'
-      - name: cargo clippy (release)
+        run: cargo hack clippy --workspace --locked --profile ci --optional-deps --each-feature -- -D warnings
-        run: cargo clippy --release --workspace --locked --all-targets -- -D warnings
+        env:
+          CARGO_PROFILE_CI_DEBUG_ASSERTIONS: "false"
+
+      - name: cargo clippy (auxiliary) (no debug_assertions)
+        if: env.USING_DEBUG_ASSERTIONS == 'true'
+        run: cargo hack clippy --workspace --locked --profile ci --optional-deps --each-feature --tests --benches --examples -- -D warnings
+        env:
+          CARGO_PROFILE_CI_DEBUG_ASSERTIONS: "false"
 
   clippy-stable-wasm:
     name: cargo clippy (wasm32)
@@ -156,15 +171,22 @@ jobs:
           save-if: ${{ github.event_name != 'merge_group' }}
 
       - name: cargo clippy
-        run: cargo hack clippy --workspace ${{ env.NO_WASM_PKGS }} --locked --target wasm32-unknown-unknown --optional-deps --each-feature -- -D warnings
+        run: cargo hack clippy --workspace ${{ env.NO_WASM_PKGS }} --locked --profile ci --target wasm32-unknown-unknown --optional-deps --each-feature -- -D warnings
 
       - name: cargo clippy (auxiliary)
-        run: cargo hack clippy --workspace ${{ env.NO_WASM_PKGS }} --locked --target wasm32-unknown-unknown --optional-deps --each-feature --tests --benches --examples -- -D warnings
+        run: cargo hack clippy --workspace ${{ env.NO_WASM_PKGS }} --locked --profile ci --target wasm32-unknown-unknown --optional-deps --each-feature --tests --benches --examples -- -D warnings
 
-      # Verify that we can build in release mode
+      - name: cargo clippy (no debug_assertions)
-      # TODO: Find a way for this to share artifacts with the above job?
+        if: env.USING_DEBUG_ASSERTIONS == 'true'
-      - name: cargo clippy (release)
+        run: cargo hack clippy --workspace ${{ env.NO_WASM_PKGS }} --locked --profile ci --target wasm32-unknown-unknown --optional-deps --each-feature -- -D warnings
-        run: cargo clippy --release --workspace ${{ env.NO_WASM_PKGS }} --locked --target wasm32-unknown-unknown --all-targets -- -D warnings
+        env:
+          CARGO_PROFILE_CI_DEBUG_ASSERTIONS: "false"
+
+      - name: cargo clippy (auxiliary) (no debug_assertions)
+        if: env.USING_DEBUG_ASSERTIONS == 'true'
+        run: cargo hack clippy --workspace ${{ env.NO_WASM_PKGS }} --locked --profile ci --target wasm32-unknown-unknown --optional-deps --each-feature --tests --benches --examples -- -D warnings
+        env:
+          CARGO_PROFILE_CI_DEBUG_ASSERTIONS: "false"
 
   prime-lfs-cache:
     name: Prime LFS Cache
@@ -344,7 +366,13 @@ jobs:
           save-if: ${{ github.event_name != 'merge_group' }}
 
       - name: cargo check
-        run: cargo hack check ${{ env.RUST_MIN_VER_PKGS }} --locked --optional-deps --each-feature
+        run: cargo hack check ${{ env.RUST_MIN_VER_PKGS }} --locked --profile ci --optional-deps --each-feature
+
+      - name: cargo check (no debug_assertions)
+        if: env.USING_DEBUG_ASSERTIONS == 'true'
+        run: cargo hack check ${{ env.RUST_MIN_VER_PKGS }} --locked --profile ci --optional-deps --each-feature
+        env:
+          CARGO_PROFILE_CI_DEBUG_ASSERTIONS: "false"
 
   check-msrv-wasm:
     name: cargo check (msrv) (wasm32)
@@ -369,7 +397,13 @@ jobs:
           save-if: ${{ github.event_name != 'merge_group' }}
 
       - name: cargo check
-        run: cargo hack check ${{ env.RUST_MIN_VER_PKGS }} ${{ env.NO_WASM_PKGS }} --locked --target wasm32-unknown-unknown --optional-deps --each-feature
+        run: cargo hack check ${{ env.RUST_MIN_VER_PKGS }} ${{ env.NO_WASM_PKGS }} --locked --profile ci --target wasm32-unknown-unknown --optional-deps --each-feature
+
+      - name: cargo check (no debug_assertions)
+        if: env.USING_DEBUG_ASSERTIONS == 'true'
+        run: cargo hack check ${{ env.RUST_MIN_VER_PKGS }} ${{ env.NO_WASM_PKGS }} --locked --profile ci --target wasm32-unknown-unknown --optional-deps --each-feature
+        env:
+          CARGO_PROFILE_CI_DEBUG_ASSERTIONS: "false"
 
   doc:
     name: cargo doc

Cargo.toml

@@ -122,3 +122,8 @@ accesskit = "0.17.0"
 accesskit_winit = "0.23.0"
 nv-flip = "0.1.2"
 time = "0.3.36"
+
+[profile.ci]
+inherits = "dev"
+[profile.ci.package."*"]
+debug-assertions = true # Keep always on for dependencies for cache reuse.