mbutrovich commented on code in PR #2779: URL: https://github.com/apache/iceberg-rust/pull/2779#discussion_r3632422876
########## mise.toml: ########## @@ -0,0 +1,277 @@ +# 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. + +min_version = "2026.7.5" +monorepo_root = true + +[monorepo] +config_roots = ["bindings/python"] + +[vars] +cargo_binstall_version = "1.20.1" +cargo_public_api_version = "0.51.0" +rust_msrv = "1.94" + +[settings.python] +# Python is also a build dependency: the DataFusion bindings need libpython. +# Refuse to fall back to a locally compiled interpreter with a different layout. +compile = false +precompiled_flavor = "install_only_stripped" + +# Cargo CLI tools and cargo-binstall are declared on the tasks that use them, not globally. +# Otherwise mise-action installs them in unrelated jobs (for example, Python CI), +# increasing setup time and exposing every job to crates.io rate limits. +[tools] +actionlint = "1.7.12" +go = "1.26.5" +jq = "1.8.2" +mdbook = "0.4.36" +python = "3.12.13" +ruff = "0.15.20" +rust = [ + { version = "nightly-2026-03-05", components = [ + "rustfmt", + "clippy", + ] }, + { version = "{{ vars.rust_msrv }}" }, +] +shellcheck = "0.11.0" +taplo = "0.9.3" +typos = "1.48.0" +uv = "0.9.3" + +[tasks.build] +description = "Build all workspace targets and features" +run = "cargo build --all-targets --all-features --workspace" + +[tasks.build-no-default-features] +description = "Build the core iceberg crate without default features" +run = "cargo build -p iceberg --no-default-features" + +[tasks.check-license] +description = "Check Apache license headers" +run = "go run github.com/apache/skywalking-eyes/cmd/[email protected] -v info -c .licenserc.yaml header check" Review Comment: `check-license` runs `go run ...skywalking-eyes/cmd/[email protected]`, which needs the Go toolchain and compiles license-eye from source on every fresh machine. The old CI used the prebuilt skywalking-eyes action. This adds a large toolchain to every contributor's install and to every `mise run lint`, and the source build is slower, which matches the "CI is slower with mise" point raised earlier. It also reintroduces the compile-from-source pattern that #2793 just removed, whereas the stated goal of this PR is consistent pinned tools that let the cache and binstall shortcut installs. Can we install a prebuilt license-eye through mise (aqua/ubi/go backend) instead of `go run`? ########## .github/workflows/audit.yml: ########## @@ -28,21 +28,21 @@ on: paths: - "**/Cargo.toml" - "**/Cargo.lock" + - ".github/workflows/audit.yml" + - "mise.toml" pull_request: paths: - "**/Cargo.toml" - "**/Cargo.lock" - ".github/workflows/audit.yml" + - "mise.toml" schedule: - cron: '0 0 * * *' permissions: - # All other permissions are set to none contents: read - checks: write Review Comment: The permissions block drops `checks: write` and `issues: write`, and `rustsec/audit-check` is replaced by `cargo audit`. On the nightly `schedule` run, `audit-check` files a GitHub issue when a new advisory appears; `cargo audit` only fails the run. So a new advisory found by the cron now produces a red run with no issue and no notification. Is that the intended behavior? ########## mise.toml: ########## @@ -0,0 +1,277 @@ +# 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. + +min_version = "2026.7.5" +monorepo_root = true + +[monorepo] +config_roots = ["bindings/python"] + +[vars] +cargo_binstall_version = "1.20.1" +cargo_public_api_version = "0.51.0" +rust_msrv = "1.94" + +[settings.python] +# Python is also a build dependency: the DataFusion bindings need libpython. +# Refuse to fall back to a locally compiled interpreter with a different layout. +compile = false +precompiled_flavor = "install_only_stripped" + +# Cargo CLI tools and cargo-binstall are declared on the tasks that use them, not globally. +# Otherwise mise-action installs them in unrelated jobs (for example, Python CI), +# increasing setup time and exposing every job to crates.io rate limits. +[tools] +actionlint = "1.7.12" +go = "1.26.5" +jq = "1.8.2" +mdbook = "0.4.36" +python = "3.12.13" +ruff = "0.15.20" +rust = [ + { version = "nightly-2026-03-05", components = [ + "rustfmt", + "clippy", + ] }, + { version = "{{ vars.rust_msrv }}" }, +] +shellcheck = "0.11.0" +taplo = "0.9.3" +typos = "1.48.0" +uv = "0.9.3" + +[tasks.build] +description = "Build all workspace targets and features" +run = "cargo build --all-targets --all-features --workspace" + +[tasks.build-no-default-features] +description = "Build the core iceberg crate without default features" +run = "cargo build -p iceberg --no-default-features" + +[tasks.check-license] +description = "Check Apache license headers" +run = "go run github.com/apache/skywalking-eyes/cmd/[email protected] -v info -c .licenserc.yaml header check" + +[tasks.check-fmt] +description = "Check Rust formatting" +run = "cargo fmt --all -- --check" + +[tasks.check-clippy] +description = "Run Clippy for all workspace targets and features" +run = "cargo clippy --all-targets --all-features --workspace -- -D warnings" + +[tasks.fix-toml] +description = "Format TOML files" +run = "taplo fmt" + +[tasks.check-toml] +description = "Check TOML formatting" +run = "taplo fmt --check" + +[tasks.check-lockfile] +description = "Check that Cargo.lock is up to date" +run = "cargo update --workspace --locked" + +[tasks.cargo-machete] +description = "Find unused Cargo dependencies" +run = "cargo machete" +tools.cargo-binstall = "{{ vars.cargo_binstall_version }}" +tools."cargo:cargo-machete" = { version = "0.7.0", depends = "rust" } + +[tasks.check-typos] +description = "Check spelling" +run = "typos" + +[tasks.check-actionlint] +description = "Check GitHub Actions workflow syntax" +run = "actionlint" + +[tasks.check-audit] +description = "Audit Rust dependencies for known vulnerabilities" +run = "cargo audit" +tools.cargo-binstall = "{{ vars.cargo_binstall_version }}" +tools."cargo:cargo-audit" = "0.22.2" +tools.rust = "{{ vars.rust_msrv }}" + +[tasks.check-standalone] +description = "Check every workspace crate without feature unification" +run = ''' +#!/usr/bin/env bash +set -euo pipefail + +while IFS= read -r package; do + echo "Checking $package..." + cargo check -p "$package" --all-targets +done < <( + cargo metadata --no-deps --format-version 1 | + jq -r '.packages[].name' +) +''' + +[tasks.check-msrv] +description = "Check the workspace with the minimum supported Rust version" +run = ''' +#!/usr/bin/env bash +set -euo pipefail + +manifest_msrv=$(awk -F'"' '/^rust-version[[:space:]]*=/ { print $2; exit }' Cargo.toml) +configured_msrv="{{ vars.rust_msrv }}" +if [[ "$manifest_msrv" != "$configured_msrv" ]]; then + echo "ERROR: Cargo.toml rust-version ($manifest_msrv) does not match mise rust_msrv ($configured_msrv)" >&2 + exit 1 +fi + +cargo check --workspace +''' +tools.rust = "{{ vars.rust_msrv }}" + +[tasks.generate-public-api] +description = "Regenerate public API snapshots for publishable crates" +run = ''' +#!/usr/bin/env bash +set -euo pipefail + +while IFS=$'\t' read -r crate manifest; do + crate_dir=$(dirname "$manifest") + echo "Generating public API for $crate..." + cargo public-api -p "$crate" --all-features -ss > "$crate_dir/public-api.txt" +done < <( + cargo metadata --no-deps --format-version 1 | + jq -r '.packages[] | select(.publish == null) | [.name, .manifest_path] | @tsv' +) +''' +tools."cargo:cargo-public-api" = { version = "{{ vars.cargo_public_api_version }}", depends = "rust" } + +[tasks.check-public-api] +description = "Check public API snapshots for publishable crates" +run = ''' +#!/usr/bin/env bash +set -euo pipefail + +tmp_dir=$(mktemp -d) +trap 'rm -rf "$tmp_dir"' EXIT +failed=0 + +while IFS=$'\t' read -r crate manifest; do + crate_dir=$(dirname "$manifest") + generated="$tmp_dir/$crate.txt" + echo "Checking public API for $crate..." + cargo public-api -p "$crate" --all-features -ss > "$generated" + if ! diff -u "$crate_dir/public-api.txt" "$generated"; then + echo "ERROR: Public API for $crate has changed. Run 'mise run generate-public-api' to update." + failed=1 + fi +done < <( + cargo metadata --no-deps --format-version 1 | + jq -r '.packages[] | select(.publish == null) | [.name, .manifest_path] | @tsv' +) + +exit "$failed" +''' +tools."cargo:cargo-public-api" = { version = "{{ vars.cargo_public_api_version }}", depends = "rust" } + +[tasks.lint] +description = "Run the checks from the CI lint job" +run = [ + { task = "check-license" }, + { task = "check-toml" }, + { task = "check-fmt" }, + { task = "check-lockfile" }, + # Structured task references do not install the referenced task's local tools. + # Start a new mise run so cargo-machete is installed and activated first. + "mise run cargo-machete", Review Comment: `lint` calls cargo-machete with a nested `mise run cargo-machete` while the other steps use `{ task = ... }`. This is correct, because a `{ task = ... }` reference does not install that task's own tools, and the comment explains it. The risk is future edits: a new tool-bearing task added as `{ task = ... }` will silently skip its tool install. A short note next to the tool declarations would help. -- 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]
