Skip to content

ci: keep RUSTFLAGS consistent to not rebuild everything#1423

Merged
kkysen merged 1 commit intomasterfrom
kkysen/ci-keep-RUSTFLAGS-consistent
Oct 22, 2025
Merged

ci: keep RUSTFLAGS consistent to not rebuild everything#1423
kkysen merged 1 commit intomasterfrom
kkysen/ci-keep-RUSTFLAGS-consistent

Conversation

@kkysen
Copy link
Contributor

@kkysen kkysen commented Oct 20, 2025

RUSTFLAGS was being changed only in the cargo test step to link z3. This meant that cargo build was thrown out and redone, making everything take way longer. This moves the z3 addition to RUSTFLAGS to where we install z3 and also only sets it for macOS, where it's needed (for brew).

@kkysen kkysen requested a review from fw-immunant October 20, 2025 22:01
@kkysen kkysen force-pushed the kkysen/ci-keep-RUSTFLAGS-consistent branch from cf491f3 to f7003da Compare October 20, 2025 22:07
# `bash` needed b/c macOS ships with bash 3, which doesn't support arrays properly.
brew install -q ninja gpg llvm@${{ matrix.clang-version }} bash z3
echo "Z3_SYS_Z3_HEADER=/opt/homebrew/include/z3.h" >> $GITHUB_ENV
# It's important that we keep `RUSTFLAGS` consistent between different steps
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also put -D warnings here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're skipped for the test_translator.py step, although that step doesn't rerun cargo so it doesn't trigger a rebuild. There's an existing comment there explaining why exactly it's skipped there (I forgot off the top of my head).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For that step, do we still want the Z3 flags?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need it? It doesn't hurt, right? For test_translator.py, we specifically don't want -D warnings because the test files inherit that, too, which we didn't want.

@fw-immunant
Copy link
Contributor

Looks like some kind of quoting issue with RUSTFLAGS:

Run export RUSTFLAGS="$RUSTFLAGS -D warnings"
  export RUSTFLAGS="$RUSTFLAGS -D warnings"
  export RUSTDOCFLAGS="-D warnings"
  # Don't build with `--all-features` as `--all-features` includes `--features llvm-static`,
  # which we don't want to test here (see https://github.com/immunant/c2rust/issues/500).
  cargo build --release
  shell: /opt/homebrew/bin/bash -e {0}
  env:
    UV_CACHE_DIR: /Users/runner/work/_temp/setup-uv-cache
    CACHE_ON_FAILURE: false
    CARGO_INCREMENTAL: 0
    Z3_SYS_Z3_HEADER: /opt/homebrew/include/z3.h
    RUSTFLAGS: "-Clink-arg=-L/opt/homebrew/lib -Clink-arg=-Wl,-rpath,/opt/homebrew/lib"
error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `rustc - --crate-name ___ --print=file-names '"-Clink-arg=-L/opt/homebrew/lib' '-Clink-arg=-Wl,-rpath,/opt/homebrew/lib"' -D warnings --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=cfg` (exit status: 1)
  --- stderr
  error: multiple input filenames provided (first two filenames are `-` and `"-Clink-arg=-L/opt/homebrew/lib`)

@kkysen
Copy link
Contributor Author

kkysen commented Oct 21, 2025

Looks like some kind of quoting issue with RUSTFLAGS:

Run export RUSTFLAGS="$RUSTFLAGS -D warnings"
  export RUSTFLAGS="$RUSTFLAGS -D warnings"
  export RUSTDOCFLAGS="-D warnings"
  # Don't build with `--all-features` as `--all-features` includes `--features llvm-static`,
  # which we don't want to test here (see https://github.com/immunant/c2rust/issues/500).
  cargo build --release
  shell: /opt/homebrew/bin/bash -e {0}
  env:
    UV_CACHE_DIR: /Users/runner/work/_temp/setup-uv-cache
    CACHE_ON_FAILURE: false
    CARGO_INCREMENTAL: 0
    Z3_SYS_Z3_HEADER: /opt/homebrew/include/z3.h
    RUSTFLAGS: "-Clink-arg=-L/opt/homebrew/lib -Clink-arg=-Wl,-rpath,/opt/homebrew/lib"
error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `rustc - --crate-name ___ --print=file-names '"-Clink-arg=-L/opt/homebrew/lib' '-Clink-arg=-Wl,-rpath,/opt/homebrew/lib"' -D warnings --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=cfg` (exit status: 1)
  --- stderr
  error: multiple input filenames provided (first two filenames are `-` and `"-Clink-arg=-L/opt/homebrew/lib`)

Where do you see that error? I misquoted something initially, but I amended it. Was it that? CI right now looks like it's just failing on the syn v2 thing in c2rust-instrument.

@fw-immunant
Copy link
Contributor

Looks like some kind of quoting issue with RUSTFLAGS:

Where do you see that error? I misquoted something initially, but I amended it. Was it that? CI right now looks like it's just failing on the syn v2 thing in c2rust-instrument.

I see that in the latest macOS CI (https://github.com/immunant/c2rust/actions/runs/18666232932/job/53218050385?pr=1423).

`RUSTFLAGS` was being changed only in the `cargo test` step to link z3.
This meant that `cargo build` was thrown out and redone, making everything take way longer.
This moves the z3 addition to `RUSTFLAGS` to where we install `z3`
and also only sets it for macOS, where it's needed (for brew).
@kkysen kkysen force-pushed the kkysen/ci-keep-RUSTFLAGS-consistent branch from f7003da to 7699a68 Compare October 21, 2025 19:36
@kkysen
Copy link
Contributor Author

kkysen commented Oct 21, 2025

I see that in the latest macOS CI (https://github.com/immunant/c2rust/actions/runs/18666232932/job/53218050385?pr=1423).

Ah, I was looking at the Linux one. I think I fixed macOS now. It's just running into the existing syn v2 problem.

Copy link
Contributor

@fw-immunant fw-immunant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@kkysen kkysen merged commit 28cceff into master Oct 22, 2025
2 of 5 checks passed
@kkysen kkysen deleted the kkysen/ci-keep-RUSTFLAGS-consistent branch October 22, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants