neilconway opened a new pull request, #26582: URL: https://github.com/apache/datafusion/pull/26582
## Which issue does this PR close? - Related to #25088, #25148 ## Rationale for this change Normal `cargo` commands reject circular dependencies between packages, but they allow cycles through dev-dependencies. This has bitten us in the past, which is why the `dev/depcheck` tool and its associated CI job were added to detect dev-dependency cycles. Running `dev/depcheck` in CI is relatively expensive, in part because it depends on the `cargo` crate, which has a large dependency tree. Building and running `depcheck` takes ~130 seconds on a 4 vCPU container, and also requires an `ubuntu-latest` runner. Cargo 1.90 added support for multiple packages in a single `cargo publish`, which does check for dev-dependency cycles. It follows that we can use a wrapper script over `cargo publish --workspace --dry-run` to replace `depcheck`. This is more efficient and can run on `ubuntu-slim`. Moreover, it avoids the need to maintain a separate tool and ensures that the CI check is consistent with how packages are actually published. (We should probably also switch to using multi-package `cargo publish` to make releases, but I'll defer that for now.) One downside is that error reporting when a cycle is found is less helpful, but I don't think that is a major concern. ## What changes are included in this PR? * Delete `dev/depcheck` * Change `ci/scripts/check_circular_dependencies.sh` to call `cargo publish --dry-run --workspace` * The "Circular Dependency Check" now runs on `ubuntu-slim` ## What is the testing strategy for this PR? CI / tooling only. Manual local testing: - Small test workspace with a dev-dependency cycle: `cargo test` passes, and `cargo package` / `cargo publish --workspace --dry-run` fail. - Same workspace without the cycle: packaging passes. - Same workspace with a dev-dependency that has a path but no version: packaging passes, because Cargo drops that dependency. - DataFusion main: the dry run passes in 15 seconds and skips the 11 `publish = false` crates. - DataFusion with a dev-dependency cycle added: `cargo check --tests` passes, and the dry run fails. - DataFusion with a normal-dependency cycle added: `cargo check` fails by itself. - DataFusion bumped to unpublished versions: the dry run passes. - Bumped versions plus the dev-dependency cycle: the dry run fails. ## Are there any user-facing changes? No. -- 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]
