andygrove commented on code in PR #5976: URL: https://github.com/apache/datafusion-comet/pull/5976#discussion_r4115829478
########## .github/actions/build-native-ci/action.yaml: ########## @@ -0,0 +1,75 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: Build or restore the Linux CI native library +description: 'Reuse an exact-input native library, otherwise build it with the CI profile' +runs: + using: composite + steps: + - name: Pin native build flags + shell: bash + run: echo 'RUSTFLAGS=-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd' >> "$GITHUB_ENV" + + # Call after checkout and setup-builder. Compute once, before Cargo writes + # generated Rust files, and use the same keys for both restore and save. + - name: Fingerprint native build inputs + id: key + shell: bash + run: python3 dev/ci/native-cache-key.py --profile ci --github-output "$GITHUB_OUTPUT" + + - name: Restore native library cache + id: binary-cache + uses: actions/cache/restore@v6 + with: + path: native/target/ci/libcomet.so + key: ${{ steps.key.outputs.binary-key }} + # Main restores the incremental cache below, including libcomet.so. + # A lookup avoids downloading the same library separately. + lookup-only: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} + + - name: Restore incremental Cargo cache + id: cargo-cache + if: steps.binary-cache.outputs.cache-hit != 'true' || (github.event_name == 'push' && github.ref == 'refs/heads/main') + uses: actions/cache/restore@v6 + with: + path: native/target + key: ${{ steps.key.outputs.source-key }} + restore-keys: ${{ steps.key.outputs.restore-prefix }} + + - name: Build native library (CI profile) + if: steps.binary-cache.outputs.cache-hit != 'true' || (github.event_name == 'push' && github.ref == 'refs/heads/main' && steps.cargo-cache.outputs.cache-hit != 'true') + shell: bash + run: | + cd native + # A library miss always builds, even on an exact incremental hit. + # Main also builds to populate a missing incremental entry; two exact + # hits already supply the library and leave neither cache to publish. + cargo build --locked --profile ci Review Comment: The file side of the fingerprint rests on things several of us have checked by hand. Nothing under `native/` uses `include_str!` or `include_bytes!`, no path dependency outside `native/` is in the default build, and contrib sources stay behind disabled features. Nothing keeps those true after this lands. Lance has no CI job at all, so moving `contrib-lance` into `default` would not trip anything. From then on, an edit to `contrib/lance/native/src/**` would reuse main's library without rebuilding it. Cargo already records what the build read. After `cargo build`, `native/target/ci/libcomet.d` lists every local source file the compile consumed, the `rerun-if-changed` paths from both build scripts, the generated protobuf modules, and `$JAVA_HOME/lib/server`. Registry and git dependencies are left out, which is fine because `Cargo.lock` pins them. Could this step run something like `python3 ../dev/ci/native-cache-key.py --check-depinfo target/ci/libcomet.d`? It would fail on any path that neither matches `NATIVE_LIBRARY_INPUTS` nor sits under `JAVA_HOME`, and it keeps the pattern list and matcher in one place. I tried this against a `libcomet.d` from a local debug build using this branch's matcher. All 289 inputs in it are covered, and a simulated `native/core/README.md` or `contrib/lance/native/src/lib.rs` entry gets flagged. It only runs when Cargo does, which is the right moment. Any change that introduces a new input also edits a tracked file, so it misses the key and builds. This is the file-side counterpart to viirya's point about the environment allowlist. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
