Skip to content

feat(ci): check only affected Rust packages #2963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
poroh wants to merge 1 commit into from
Closed

feat(ci): check only affected Rust packages #2963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,16 @@ jobs:
steps:
- name: Checkout code
uses: actions/checkout@v4
with:
# actions/checkout fetches only one commit by default; a depth of 0 removes
# that limit and fetches the full history. The `check-affected`
# cargo-make task needs that history to find HEAD's common ancestor with
# origin/main.
fetch-depth: 0

# The container can see the mounted workspace as owned by a different UID.
- name: Configure Git safe directory
run: git config --global --add safe.directory "$GITHUB_WORKSPACE"

# PostgreSQL is required because sqlx proc macros (sqlx::query!) connect to
# a local database at compile time to validate SQL queries and return types.
Expand All @@ -1114,11 +1124,13 @@ jobs:
sudo -u postgres psql -c "ALTER USER root WITH SUPERUSER;"
createdb root
- name: Run clippy
run: cargo make --no-workspace clippy-flow

- name: Run carbide lints
run: cargo make carbide-lints
# `check-affected` selects packages and replaces separate CI steps for
# Clippy, carbide-lints, and isolated default-feature builds. Formatting,
# workspace dependency, license, and ban checks remain global below.
- name: Run affected Rust checks
env:
AFFECTED_BASE: origin/main
run: cargo make --no-workspace check-affected

- name: Check TOML formatting
run: taplo fmt --check || echo "Please format toml files"
Expand All @@ -1135,9 +1147,6 @@ jobs:
- name: Check bans
run: cargo make --no-workspace check-bans

- name: Check isolated package builds
run: cargo xtask check-isolated-package-builds

- name: Check repository is clean
run: bash scripts/check-repo-clean.sh "Core pre-build checks"

Expand Down
25 changes: 19 additions & 6 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,27 @@ cargo make format-nightly # Also sort imports
> `carbide-lints`. The stable toolchain pinned in `rust-toolchain.toml` is used
> for everything else.

### Top-level Makefile (rest-api entrypoint)
#### Affected-package CI

When reviewing changes that add, remove, rename, or repurpose shared Rust build
inputs, verify that `is_global_path()` in
`crates/xtask/src/affected_packages.rs` remains up to date. Currently matched
shared inputs include the root Cargo files and `Cargo.lock`, cargo-make files,
Rust toolchain configuration, `.cargo/`, CI configuration, custom lint and xtask
code, and `include/`. Add any newly introduced or repurposed shared generated or
configuration directories to the predicate. If a changed path cannot be mapped
safely to exactly one workspace package, affected-package selection must fall
back to the full workspace.
Comment on lines +129 to +137

@coderabbitai coderabbitai Bot Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Clarify the directory categories in this guidance.

Line 134’s shared generated or configuration directories is ambiguous, so the is_global_path() maintenance rule is easy to misread. Please spell the categories out explicitly.

✏️ Proposed fix 
- Rust toolchain configuration, `.cargo/`, CI configuration, custom lint and xtask
- code, and `include/`. Add any newly introduced or repurposed shared generated or
- configuration directories to the predicate.
+ Rust toolchain configuration, `.cargo/`, CI configuration, custom lint and xtask
+ code, and `include/`. Add any newly introduced or repurposed shared
+ generated/configuration directories to the predicate.

As per path instructions, review Markdown for correctness, clarity, spelling, grammar, working links, and whether commands/examples are realistic and safe.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
When reviewing changes that add, remove, rename, or repurpose shared Rust build
inputs, verify that `is_global_path()` in
`crates/xtask/src/affected_packages.rs` remains up to date. Currently matched
shared inputs include the root Cargo files and `Cargo.lock`, cargo-make files,
Rust toolchain configuration, `.cargo/`, CI configuration, custom lint and xtask
code, and `include/`. Add any newly introduced or repurposed shared generated or
configuration directories to the predicate. If a changed path cannot be mapped
safely to exactly one workspace package, affected-package selection must fall
back to the full workspace.
When reviewing changes that add, remove, rename, or repurpose shared Rust build
inputs, verify that `is_global_path()` in
`crates/xtask/src/affected_packages.rs` remains up to date. Currently matched
shared inputs include the root Cargo files and `Cargo.lock`, cargo-make files,
Rust toolchain configuration, `.cargo/`, CI configuration, custom lint and xtask
code, and `include/`. Add any newly introduced or repurposed shared
generated/configuration directories to the predicate. If a changed path cannot be mapped
safely to exactly one workspace package, affected-package selection must fall
back to the full workspace.
🧰 Tools
🪛 LanguageTool

[grammar] ~134-~134: Use a hyphen to join words.
Context: ...ny newly introduced or repurposed shared generated or configuration directories t...

(QB_NEW_EN_HYPHEN)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@AGENTS.md` around lines 129 - 137, Clarify the maintenance guidance for
is_global_path() in crates/xtask/src/affected_packages.rs by replacing the vague
phrase with explicit categories of shared paths; spell out which kinds of
generated or configuration directories should be treated as global, and keep the
existing examples of root Cargo files, Cargo.lock, cargo-make files, Rust
toolchain config, .cargo/, CI config, custom lint/xtask code, and include/
aligned with that list. Make the wording unambiguous so contributors can tell
when a newly introduced shared directory must be added, and preserve the
fallback rule that any path not safely attributable to exactly one workspace
package should select the full workspace.

Sources: Path instructions, Linters/SAST tools


### Top-level Makefile entrypoints

A top-level [`Makefile`](Makefile) at the repo root provides a thin
discoverable entrypoint for the `rest-api/` Go services. It just
delegates to `rest-api/Makefile`.
discoverable entrypoint for selected Core workflows and the `rest-api/` Go
services.

```bash
make help # default goal: list rest-* targets
make help # default goal: list available targets
make core/check-affected # check affected Rust packages and workspace dependents
make rest-build # build rest-api Go binaries
make rest-test # run rest-api unit tests
make rest-lint # lint rest-api
Expand All @@ -142,8 +155,8 @@ make rest-kind-reset # spin up the local kind dev cluster (~10 min)
make rest-api/<target> # pass any target through to rest-api/Makefile
```

Core (Rust) tasks are not in this Makefile; use cargo and `cargo make`
directly as documented above.
Other Core (Rust) tasks use cargo and `cargo make` directly as documented
above.

## Coding Conventions

Expand Down
21 changes: 17 additions & 4 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,20 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
# Top-level Makefile for the rest-api/ Go services.
# Top-level Makefile for Core and rest-api workflows.
#
# Thin discoverable entrypoint that delegates to rest-api/Makefile.
# rest-api/Makefile continues to work directly; this file is an
# additive convenience layer.
# Thin discoverable entrypoint that delegates Core tasks to cargo-make and Rest
# tasks to rest-api/Makefile. Both underlying entrypoints continue to work
# directly; this file is an additive convenience layer.
#
# Run `make help` (default goal) for the inventory of targets.

SHELL := /bin/bash

.DEFAULT_GOAL := help

AFFECTED_BASE ?= origin/main

# =============================================================================
# Help (default goal)
# =============================================================================
Expand All @@ -38,12 +40,23 @@ help: ## Show this help and exit (default goal)
@echo "Container images (build from a clean clone):"
@grep -E '^images[a-zA-Z0-9_-]*:.*## ' $(MAKEFILE_LIST) | awk 'BEGIN{FS=":.*?## "} {printf " %-26s %s\n", $$1, $$2}'
@echo ""
@echo "Core (Rust):"
@grep -E '^core/check-affected:.*## ' $(MAKEFILE_LIST) | awk 'BEGIN{FS=":.*?## "} {printf " %-26s %s\n", $$1, $$2}'
@echo ""
@echo "Rest (Go services in rest-api/):"
@grep -E '^rest-[a-zA-Z0-9_-]+:.*## ' $(MAKEFILE_LIST) | awk 'BEGIN{FS=":.*?## "} {printf " %-26s %s\n", $$1, $$2}'
@echo " rest-api/<target> Pass any target through to rest-api/Makefile"
@echo ""
@echo " cat rest-api/Makefile See all rest-api/ targets directly"

# =============================================================================
# Core (Rust; delegate to cargo-make)
# =============================================================================

.PHONY: core/check-affected
core/check-affected: ## Run Clippy, custom lints, and isolated builds for affected Rust packages
AFFECTED_BASE="$(AFFECTED_BASE)" cargo make --no-workspace check-affected

# =============================================================================
# Getting started (build host setup)
# =============================================================================
Expand Down
44 changes: 44 additions & 0 deletions Makefile.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,50 @@ workspace = false
description = "Check that workspace packages build independently with default features"
script = ["cargo xtask check-isolated-package-builds"]

[tasks.check-affected]
workspace = false
description = "Run compilation checks for changed Rust packages and their workspace dependents"
script = '''
set -eu

base="${AFFECTED_BASE:-origin/main}"
affected_packages="$(cargo xtask affected-packages --base "${base}")"

if [ -z "${affected_packages}" ]; then
echo "No affected Rust packages were selected" >&2
exit 1
fi

echo "Affected Rust packages:"
printf ' %s\n' "${affected_packages}"

set --
while IFS= read -r package; do
[ -n "${package}" ] || continue
set -- "$@" -p "${package}"
done <<EOF
${affected_packages}
EOF

# These checks use different feature sets and compiler drivers. Keep their Cargo
# artifacts separate so one check does not invalidate another check's cache.
target_dir="${CARGO_TARGET_DIR:-${REPO_ROOT}/target}"

echo "Checking isolated builds for affected packages"
CARGO_TARGET_DIR="${target_dir}/isolated-checks" \
cargo xtask check-isolated-package-builds "$@"

echo "Running Clippy for affected packages"
cargo make --no-workspace install-clippy
CARGO_TARGET_DIR="${target_dir}/clippy" \
cargo clippy --locked --all-targets --all-features "$@"

echo "Running carbide-lints for affected packages"
cargo make --no-workspace setup-carbide-lints
CARGO_TARGET_DIR="${target_dir}/carbide-lints" \
cargo carbide-lints --all-targets --all-features "$@"
'''

[tasks.check-licenses]
workspace = false
description = "Check cargo-deny against licenses we're using to validate that we're not introducing new licenses inadvertently"
Expand Down
Loading
Loading