From 318a199cc5e2359097bac20b7c1122500e34f13f Mon Sep 17 00:00:00 2001 From: Moritz Angermann Date: Sun, 1 Mar 2026 11:30:52 +0900 Subject: [PATCH] fix: CI timing overwrites and metrics plot generation Two issues from the QUIET mode CI run (#121): 1. **Timing overwrites:** The `test: stage2` dependency triggers the PHONY stage2 recipe again. Since everything is already built, the re-run completes in seconds, overwriting the real stage2 timings (16m 31s) with near-zero durations (3m 36s). Fix: guard PHASE_START/PHASE_END macros to skip writing if the phase already completed successfully (status=0). Failed phases can still be retried. 2. **Metrics plots:** The "Generate Metrics plots" CI step used `nix-shell -p ...` which is not available in the devx environment. Fix: replace `nix-shell` with `nix build --print-out-paths` to get the store path, then invoke the binary directly. Applied to both the matplotlib plot generation and the awscli R2 upload steps. --- .github/workflows/ci.yml | 20 ++++++++++++++------ Makefile | 28 ++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cfc8b07194b3..21e88111dd41 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -110,20 +110,24 @@ jobs: - name: Generate metrics plots if: ${{ !cancelled() }} continue-on-error: true - shell: devx {0} + shell: bash run: | - # Generate separate build and test SVG plots - nix-shell -p 'python3.withPackages (ps: [ps.matplotlib])' --run 'python3 ./mk/plot-metrics.py _build/metrics _build/timing _build/metrics/metrics' + # Generate separate build and test SVG plots. + # Use bash (not devx) because nix is not available inside the devx shell. + PYTHON_MPL=$(nix build --impure --no-link --print-out-paths \ + --expr 'let pkgs = (builtins.getFlake "nixpkgs").legacyPackages.${builtins.currentSystem}; in pkgs.python3.withPackages (ps: [ps.matplotlib])') + "$PYTHON_MPL/bin/python3" ./mk/plot-metrics.py _build/metrics _build/timing _build/metrics/metrics - name: Upload metrics plots to R2 if: ${{ !cancelled() }} + continue-on-error: true env: AWS_ACCESS_KEY_ID: ${{ secrets.R2_METRICS_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.R2_METRICS_SECRET_ACCESS_KEY }} R2_ENDPOINT: ${{ secrets.R2_METRICS_ENDPOINT }} R2_PUBLIC_URL: ${{ vars.R2_METRICS_PUBLIC_URL }} R2_BUCKET: ${{ vars.R2_METRICS_BUCKET_NAME }} - shell: devx {0} + shell: bash run: | RUN_ID="${{ github.run_id }}" PLAT="${{ matrix.plat }}" @@ -132,17 +136,21 @@ jobs: export AWS_ENDPOINT_URL="${R2_ENDPOINT}" export AWS_DEFAULT_REGION=auto + # Use bash (not devx) because nix is not available inside the devx shell. + AWSCLI=$(nix build --no-link --print-out-paths nixpkgs#awscli2) + AWS="$AWSCLI/bin/aws" + # Upload build plot if it exists if [[ -f "_build/metrics/metrics-build.svg" ]]; then BUILD_PATH="runs/${RUN_ID}/${PLAT}-dynamic${DYN}-build.svg" - nix-shell --keep AWS_ACCESS_KEY_ID --keep AWS_SECRET_ACCESS_KEY --keep AWS_ENDPOINT_URL --keep AWS_DEFAULT_REGION -p awscli2 --run "aws s3 cp _build/metrics/metrics-build.svg s3://${R2_BUCKET}/${BUILD_PATH} --content-type 'image/svg+xml'" || echo "Failed to upload build plot (non-fatal)" + "$AWS" s3 cp _build/metrics/metrics-build.svg "s3://${R2_BUCKET}/${BUILD_PATH}" --content-type 'image/svg+xml' || echo "Failed to upload build plot (non-fatal)" echo "BUILD_PLOT_URL=${R2_PUBLIC_URL}/${BUILD_PATH}" >> $GITHUB_ENV fi # Upload test plot if it exists if [[ -f "_build/metrics/metrics-test.svg" ]]; then TEST_PATH="runs/${RUN_ID}/${PLAT}-dynamic${DYN}-test.svg" - nix-shell --keep AWS_ACCESS_KEY_ID --keep AWS_SECRET_ACCESS_KEY --keep AWS_ENDPOINT_URL --keep AWS_DEFAULT_REGION -p awscli2 --run "aws s3 cp _build/metrics/metrics-test.svg s3://${R2_BUCKET}/${TEST_PATH} --content-type 'image/svg+xml'" || echo "Failed to upload test plot (non-fatal)" + "$AWS" s3 cp _build/metrics/metrics-test.svg "s3://${R2_BUCKET}/${TEST_PATH}" --content-type 'image/svg+xml' || echo "Failed to upload test plot (non-fatal)" echo "TEST_PLOT_URL=${R2_PUBLIC_URL}/${TEST_PATH}" >> $GITHUB_ENV fi diff --git a/Makefile b/Makefile index 1eee1329120b..daac2e7c8d2e 100644 --- a/Makefile +++ b/Makefile @@ -242,20 +242,36 @@ LOG_GROUP_START = @: LOG_GROUP_END = @: endif -# Phase timing: records start/end timestamps and status +# Phase timing: records start/end timestamps and status. +# +# Guards prevent overwrites when PHONY targets re-run without doing real work. +# For example, `test: stage2` triggers the PHONY stage2 recipe again; without +# guards, the no-op re-check would overwrite the real stage2 build timings with +# near-zero durations. +# +# Policy: +# - If a phase already completed successfully (status=0), skip re-timing. +# - If a phase failed (status=1) or never ran, (re)start timing. define PHASE_START @mkdir -p $(TIMING_DIR) - @date +%s > $(TIMING_DIR)/$(1).start + @if [ ! -f $(TIMING_DIR)/$(1).end ] || [ "$$(cat $(TIMING_DIR)/$(1).status 2>/dev/null)" != "0" ]; then \ + date +%s > $(TIMING_DIR)/$(1).start; \ + rm -f $(TIMING_DIR)/$(1).end $(TIMING_DIR)/$(1).status; \ + fi endef define PHASE_END_OK - @date +%s > $(TIMING_DIR)/$(1).end - @echo "0" > $(TIMING_DIR)/$(1).status + @if [ ! -f $(TIMING_DIR)/$(1).end ]; then \ + date +%s > $(TIMING_DIR)/$(1).end; \ + echo "0" > $(TIMING_DIR)/$(1).status; \ + fi endef define PHASE_END_FAIL - @date +%s > $(TIMING_DIR)/$(1).end - @echo "1" > $(TIMING_DIR)/$(1).status + @if [ ! -f $(TIMING_DIR)/$(1).end ]; then \ + date +%s > $(TIMING_DIR)/$(1).end; \ + echo "1" > $(TIMING_DIR)/$(1).status; \ + fi endef define NORMALIZE_FP