From 947e169a699e090a3e8c8d21d64e3f5a685f0b81 Mon Sep 17 00:00:00 2001 From: Kumar Ujjawal Date: Thu, 17 Sep 2026 13:16:24 +0530 Subject: [PATCH] dev: include dependency checks in the local lint suite Expose the two dependency checks from the "Dependencies" workflow as shared scripts and run them from `dev/rust_lint.sh` as well, so a contributor sees a dependency cycle or an unused dependency before pushing instead of only in CI. `ci/scripts/check_circular_dependencies.sh` runs `cargo run --locked` in `dev/depcheck`, which is outside the root workspace and locates the root manifest from its own directory. `ci/scripts/check_unused_dependencies.sh` runs `cargo machete --with-metadata` from the repository root and requires `cargo-machete` without installing it. Both keep their command's output and exit status. `CARGO_MACHETE_VERSION="0.9"` moves into the shared tool version file. The workflow loads it into the environment and passes it to the existing install action, keeping the same `0.9` series selector. The lint runner installs a missing `cargo-machete` with `--version ^0.9`, the range Cargo requires for that selector, and registers both scripts as read-only steps after the security audit and before the Rust documentation build. Job IDs, names, containers, checkout options, action revisions, triggers, and permissions are unchanged. The depcheck step drops its `working-directory` because the script enters `dev/depcheck` itself. Partial progress on #21048. --- .github/workflows/dependencies.yml | 12 ++++--- ci/scripts/check_circular_dependencies.sh | 34 ++++++++++++++++++++ ci/scripts/check_unused_dependencies.sh | 39 +++++++++++++++++++++++ ci/scripts/utils/tool_versions.sh | 1 + dev/rust_lint.sh | 3 ++ docs/source/contributor-guide/testing.md | 20 ++++++++++++ 6 files changed, 104 insertions(+), 5 deletions(-) create mode 100755 ci/scripts/check_circular_dependencies.sh create mode 100755 ci/scripts/check_unused_dependencies.sh diff --git a/.github/workflows/dependencies.yml b/.github/workflows/dependencies.yml index 6cab01c2a53c5..4d6549d941fcb 100644 --- a/.github/workflows/dependencies.yml +++ b/.github/workflows/dependencies.yml @@ -51,9 +51,7 @@ jobs: with: rust-version: stable - name: Check dependencies - working-directory: dev/depcheck - run: | - cargo run --locked + run: bash ci/scripts/check_circular_dependencies.sh detect-unused-dependencies: name: Detect Unused Dependencies @@ -62,9 +60,13 @@ jobs: image: amd64/rust steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Load tool versions + run: | + . ci/scripts/utils/tool_versions.sh + echo "CARGO_MACHETE_VERSION=${CARGO_MACHETE_VERSION}" >> "$GITHUB_ENV" - name: Install cargo-machete uses: taiki-e/install-action@3f74d7c16a4242f1c95561e98edc25d36adb4375 # v2.87.12 with: - tool: cargo-machete@0.9 + tool: cargo-machete@${{ env.CARGO_MACHETE_VERSION }} - name: Detect unused dependencies - run: cargo machete --with-metadata + run: bash ci/scripts/check_unused_dependencies.sh diff --git a/ci/scripts/check_circular_dependencies.sh b/ci/scripts/check_circular_dependencies.sh new file mode 100755 index 0000000000000..7cdc97b0d6ba7 --- /dev/null +++ b/ci/scripts/check_circular_dependencies.sh @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +# +# 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. + +# Checks for circular dependencies between DataFusion crates with +# `dev/depcheck`, the same way the "Circular Dependency Check" job does. + +set -euo pipefail + +SCRIPT_NAME="$(basename "${BASH_SOURCE[0]}")" +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ROOT_DIR="$(cd "${SCRIPT_DIR}/../.." && pwd)" + +# depcheck is outside the root workspace and finds the root manifest from its +# own directory, so run Cargo from there. +cd "${ROOT_DIR}/dev/depcheck" + +echo "[${SCRIPT_NAME}] \`cargo run --locked\` in dev/depcheck" +cargo run --locked diff --git a/ci/scripts/check_unused_dependencies.sh b/ci/scripts/check_unused_dependencies.sh new file mode 100755 index 0000000000000..0221d50dfd8da --- /dev/null +++ b/ci/scripts/check_unused_dependencies.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# +# 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. + +# Detects unused dependencies with `cargo machete`, the same way the +# "Detect Unused Dependencies" job does. + +set -euo pipefail + +SCRIPT_NAME="$(basename "${BASH_SOURCE[0]}")" +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ROOT_DIR="$(cd "${SCRIPT_DIR}/../.." && pwd)" + +source "${SCRIPT_DIR}/utils/tool_versions.sh" + +if ! command -v cargo-machete &> /dev/null; then + echo "[${SCRIPT_NAME}] cargo-machete is required. Install it with: cargo install cargo-machete --locked --version ^${CARGO_MACHETE_VERSION}" >&2 + exit 1 +fi + +cd "${ROOT_DIR}" + +echo "[${SCRIPT_NAME}] \`cargo machete --with-metadata\`" +cargo machete --with-metadata diff --git a/ci/scripts/utils/tool_versions.sh b/ci/scripts/utils/tool_versions.sh index 237b18b62ef40..b0a514edd05ff 100644 --- a/ci/scripts/utils/tool_versions.sh +++ b/ci/scripts/utils/tool_versions.sh @@ -22,3 +22,4 @@ PRETTIER_VERSION="2.7.1" LYCHEE_VERSION="0.23.0" +CARGO_MACHETE_VERSION="0.9" diff --git a/dev/rust_lint.sh b/dev/rust_lint.sh index 5862302d25ade..7e17dea9db527 100755 --- a/dev/rust_lint.sh +++ b/dev/rust_lint.sh @@ -113,6 +113,7 @@ ensure_tool "hawkeye" "cargo install hawkeye --locked" ensure_tool "typos" "cargo install typos-cli --locked" ensure_tool "lychee" "cargo install lychee --locked --version ${LYCHEE_VERSION}" ensure_tool "cargo-audit" "cargo install cargo-audit --locked" +ensure_tool "cargo-machete" "cargo install cargo-machete --locked --version ^${CARGO_MACHETE_VERSION}" run_step() { local name="$1" @@ -135,6 +136,8 @@ declare -a READONLY_STEPS=( "ci/scripts/check_asf_yaml_status_checks.py|false" "ci/scripts/markdown_link_check.sh|false" "ci/scripts/security_audit.sh|false" + "ci/scripts/check_circular_dependencies.sh|false" + "ci/scripts/check_unused_dependencies.sh|false" "ci/scripts/rust_docs.sh|false" ) diff --git a/docs/source/contributor-guide/testing.md b/docs/source/contributor-guide/testing.md index e50e0942d0220..a4332352fa3aa 100644 --- a/docs/source/contributor-guide/testing.md +++ b/docs/source/contributor-guide/testing.md @@ -254,6 +254,26 @@ The audit fetches the RustSec advisory database. A new advisory or a different [cargo-audit]: https://github.com/rustsec/rustsec/blob/main/cargo-audit/README.md +## Dependency Checks + +CI runs two dependency checks, and `./dev/rust_lint.sh` runs both: + +- `ci/scripts/check_circular_dependencies.sh` builds and runs [`dev/depcheck`], + which fails on dependency cycles between DataFusion crates. +- `ci/scripts/check_unused_dependencies.sh` runs `cargo machete --with-metadata` + from the repository root. The lint suite installs [cargo-machete] with the + version in `ci/scripts/utils/tool_versions.sh` if it is missing. + +To run either check on its own: + +```shell +./ci/scripts/check_circular_dependencies.sh +./ci/scripts/check_unused_dependencies.sh +``` + +[`dev/depcheck`]: https://github.com/apache/datafusion/tree/main/dev/depcheck +[cargo-machete]: https://github.com/bnjbvr/cargo-machete + ## Benchmarks ### Criterion Benchmarks