Skip to content

Commit b59349c

Browse files
fix[duckdb]: build cpp.rs only if changed (#6951)
Only build rust files if they change. Add a CI check for this Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
1 parent 2a0d1b7 commit b59349c

3 files changed

Lines changed: 49 additions & 12 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
name: "Build and verify rebuild is a no-op"
2+
description: "Run a cargo command, then re-run it to verify all artifacts are cached"
3+
inputs:
4+
command:
5+
description: "The cargo build command to run and verify (--message-format json is appended on the verification run)"
6+
required: true
7+
runs:
8+
using: "composite"
9+
steps:
10+
- name: "Build"
11+
shell: bash
12+
run: ${{ inputs.command }}
13+
- name: "Verify rebuild is a no-op"
14+
shell: bash
15+
run: |
16+
stale=$(${{ inputs.command }} \
17+
--message-format json 2>/dev/null \
18+
| jq -r 'select(.reason == "compiler-artifact" and .fresh == false) | .target.name')
19+
if [ -n "$stale" ]; then
20+
echo "ERROR: Rebuild recompiled crates that should have been cached:"
21+
echo "$stale"
22+
exit 1
23+
fi

.github/workflows/ci.yml

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,9 @@ jobs:
236236
run: rustup target add wasm32-unknown-unknown
237237
- name: Install cargo-hack
238238
uses: taiki-e/install-action@cargo-hack
239-
- name: Rust Build (${{matrix.config.name}})
240-
run: ${{matrix.config.env.rustflags}} cargo hack build --locked ${{matrix.config.args}} --ignore-private
239+
- uses: ./.github/actions/check-rebuild
240+
with:
241+
command: "${{matrix.config.env.rustflags}} cargo hack build --locked ${{matrix.config.args}} --ignore-private"
241242
- name: "Make sure no files changed after build"
242243
run: |
243244
git status --porcelain
@@ -497,14 +498,12 @@ jobs:
497498
- uses: ./.github/actions/setup-rust
498499
with:
499500
repo-token: ${{ secrets.GITHUB_TOKEN }}
500-
- name: Build CUDA crates
501-
run: |
502-
cargo build --locked --all-features --all-targets \
503-
-p vortex-cuda \
504-
-p vortex-cub \
505-
-p vortex-nvcomp \
506-
-p gpu-scan-cli \
507-
-p vortex-test-e2e-cuda
501+
- uses: ./.github/actions/check-rebuild
502+
with:
503+
command: >-
504+
cargo build --locked --all-features --all-targets
505+
-p vortex-cuda -p vortex-cub -p vortex-nvcomp
506+
-p gpu-scan-cli -p vortex-test-e2e-cuda
508507
- name: Clippy CUDA crates
509508
run: |
510509
cargo clippy --locked --all-features --all-targets \
@@ -658,6 +657,10 @@ jobs:
658657
if: matrix.os != 'windows-x64'
659658
run: |
660659
cargo nextest run --locked --workspace --all-features --no-fail-fast --exclude vortex-bench --exclude xtask --exclude vortex-sqllogictest
660+
- uses: ./.github/actions/check-rebuild
661+
if: matrix.os != 'windows-x64'
662+
with:
663+
command: "cargo test --locked --workspace --all-features --no-run --exclude vortex-bench --exclude xtask --exclude vortex-sqllogictest"
661664

662665
- name: Alert incident.io
663666
if: failure() && github.event_name == 'push' && github.ref == 'refs/heads/develop'
@@ -780,6 +783,9 @@ jobs:
780783
cmake -S vortex-cxx/examples -B vortex-cxx/examples/build -DCMAKE_BUILD_TYPE=Release
781784
cmake --build vortex-cxx/examples/build --parallel $(nproc)
782785
vortex-cxx/examples/build/hello-vortex vortex-cxx/examples/goldenfiles/example.vortex
786+
- uses: ./.github/actions/check-rebuild
787+
with:
788+
command: "cargo build --locked -p vortex-cxx --lib"
783789

784790
sqllogic-test:
785791
name: "SQL logic tests"
@@ -805,7 +811,9 @@ jobs:
805811
run: |
806812
./vortex-sqllogictest/slt/tpch/generate_data.sh
807813
cargo test -p vortex-sqllogictest --test sqllogictests
808-
814+
- uses: ./.github/actions/check-rebuild
815+
with:
816+
command: "cargo test -p vortex-sqllogictest --test sqllogictests --no-run"
809817

810818
wasm-integration:
811819
name: "wasm-integration"

vortex-duckdb/build.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,13 @@ fn c2rust(crate_dir: &Path, duckdb_include_dir: &Path) {
316316
exit(1);
317317
}
318318
};
319-
if let Err(e) = bindings.write_to_file(crate_dir.join("src/cpp.rs")) {
319+
let out_path = crate_dir.join("src/cpp.rs");
320+
let new_contents = bindings.to_string();
321+
let write = match fs::read_to_string(&out_path) {
322+
Ok(existing) => existing != new_contents,
323+
Err(_) => true,
324+
};
325+
if write && let Err(e) = fs::write(&out_path, new_contents) {
320326
println!("cargo:error=Failed to write Rust bindings: {e}");
321327
exit(1);
322328
}

0 commit comments

Comments
 (0)