Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 9 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,17 @@ jobs:
python-version: '3.13'

- name: Install uv
uses: astral-sh/setup-uv@v7
uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7

- name: Install CLI tools
run: |
for tool in mbake ty; do uv tool install ${tool}; done
# Pin ruff to the same version as the dev dependency group in
# pyproject.toml so the linter behaves identically across the uv tool,
# the synced virtualenv, and developer PATH. Bump both sites together.
for tool in mbake; do uv tool install ${tool}; done
# ty is not installed here: `make typecheck` runs it via
# `uv tool run ty@$(TY_VERSION)`. TY_VERSION in the Makefile is the
# sole ty version declaration; update it there.
# Pin ruff to the version the Makefile invokes (RUFF_VERSION) so the
# linter behaves identically across the uv tool, the synced
# virtualenv, and developer PATH; bump both sites together.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
uv tool install ruff==0.15.12
npm install -g markdownlint-cli2

Expand Down Expand Up @@ -85,7 +88,7 @@ jobs:
# where the gate step is skipped (secret-less fork PRs, and
# push-to-main where the gate is PR-only).
- name: Upload coverage artifact
uses: actions/upload-artifact@v4
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
with:
name: coverage-report
path: coverage.xml
Expand Down
11 changes: 8 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@ UV ?= $(shell command -v uv 2>/dev/null || printf '%s/.local/bin/uv' "$$HOME")
RUFF_VERSION ?= 0.15.12
RUFF ?= $(UV) tool run --from ruff==$(RUFF_VERSION) ruff
TYPOS_VERSION ?= 1.48.0
# Pin ty. `make typecheck` invokes it via `uv tool run ty@$(TY_VERSION)`, so
# TY_VERSION is the sole ty version declaration (CI runs `make typecheck` and
# installs no separate ty). Bump it here to move local and CI checks together.
TY_VERSION ?= 0.0.32
TY ?= $(UV) tool run ty@$(TY_VERSION)
UV_ENV = UV_CACHE_DIR=.uv-cache UV_TOOL_DIR=.uv-tools
TOOLS = $(MDFORMAT_ALL) ty $(MDLINT) $(NIXIE) $(UV)
TOOLS = $(MDFORMAT_ALL) $(MDLINT) $(NIXIE) $(UV)
PY_SOURCES := $(sort $(shell find lading scripts -type f -name '*.py' -print))
VENV_TOOLS = interrogate pytest
PYLINT_PYTHON ?= pypy
Expand Down Expand Up @@ -83,8 +88,8 @@ lint: build $(UV) interrogate ## Run linters
$(UV) run interrogate --fail-under 100 lading
$(PYLINT) $(PYLINT_TARGETS)

typecheck: build ty ## Run typechecking
ty check --python-version 3.13 $(PY_SOURCES)
typecheck: build $(UV) ## Run typechecking
$(UV_ENV) $(TY) check --python-version 3.13 $(PY_SOURCES)

markdownlint: spelling $(MDLINT) ## Lint Markdown files and enforce spelling
find . -type f -name '*.md' \
Expand Down
6 changes: 5 additions & 1 deletion docs/developers-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,6 @@ manifest rewrites, while publish only probes freshness read-only via
`cargo metadata --locked --manifest-path ... --format-version=1`. It returns a
`LockfileFreshness` result that distinguishes fresh lockfiles, lockfiles that
Cargo says need updating under `--locked`, and unrelated Cargo failures.

The publish pre-flight domain reaches both operations through the
`LockfileInspectionRepository` port (issue #82) rather than holding a command
runner: `_validate_lockfile_freshness` and `_collect_stale_lockfiles` in
Expand All @@ -398,6 +397,11 @@ counterpart to the bump-side `bump_lockfiles.LockfileRepository`; together they
complete issue #82's separation of lockfile VCS/filesystem concerns from the
command domain.

`_collect_stale_lockfiles` deliberately classifies every tracked lockfile
rather than short-circuiting on the first stale result (issue #83), so the
raised error can list each stale lockfile with its repair command and the
operator repairs the workspace in a single pass.

`LockfileDiscoveryError` inherits `LadingError`; its messages include the git
failure detail.

Expand Down
12 changes: 12 additions & 0 deletions docs/lading-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,18 @@ Together with the bump-side `LockfileRepository`, the two ports keep
VCS, filesystem, and cargo execution concerns out of the lockfile
domain logic (issue #82).

`_collect_stale_lockfiles` deliberately classifies every tracked
`Cargo.lock` rather than short-circuiting on the first stale one
(issue #83). When stale lockfiles are found, `_build_stale_lockfile_message`
composes a diagnostic message that lists each offending lockfile
alongside its own `cargo generate-lockfile --manifest-path ...` repair
command; `_validate_lockfile_freshness` then raises it as a
`PublishPreflightError`, so the operator can remediate the whole
workspace in a single pass rather than re-running the pre-flight once
per lockfile. Only unexpected failures from `cargo metadata --locked` —
those not attributable to a stale lockfile — raise immediately on
first occurrence, leaving the aggregation path unaffected.

### Publish Preflight Sequence

The preflight sequence diagram illustrates the pre-flight checks that run
Expand Down
18 changes: 12 additions & 6 deletions docs/users-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,23 @@ lading publish
```

Before running `cargo check` and `cargo test`, `lading publish` validates that
all git-tracked `Cargo.lock` files are fresh under `--locked` mode. If any
lockfile is stale — for example after a `lading bump` that regenerated a nested
workspace lockfile — the command exits with code 1 and prints a repair command:
all git-tracked `Cargo.lock` files are fresh under `--locked` mode. It probes
every tracked lockfile rather than stopping at the first stale one — for
example after a `lading bump` that regenerated one or more nested workspace
lockfiles — so a single run reports the whole workspace. If any lockfile is
stale, the command exits with code 1 and lists each stale lockfile alongside
its own repair command:

```text
```plaintext
Tracked Cargo.lock files are stale after manifest version changes.
Run the following to repair:
This commonly happens after running `lading bump`; repair each stale lockfile directly:
- <path>/Cargo.lock
cargo generate-lockfile --manifest-path <path>/Cargo.toml
- <nested>/Cargo.lock
cargo generate-lockfile --manifest-path <nested>/Cargo.toml
```

Run the repair command, commit the updated lockfile, then re-run
Run each repair command, commit the updated lockfiles, then re-run
`lading publish`.

To require a clean working tree before running the pre-flight checks, pass
Expand Down
7 changes: 7 additions & 0 deletions lading/commands/publish_preflight.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,13 @@ def _collect_stale_lockfiles(
) -> list[Path]:
"""Classify tracked lockfiles; raise immediately on error, return stale paths.

Every tracked lockfile is classified rather than short-circuiting on the
first stale result (issue #83): the aggregated error message lists each
stale lockfile with its repair command, so the operator fixes the whole
workspace in one pass instead of replaying the pre-flight per lockfile.
The extra ``cargo metadata --locked`` probes are cheap relative to that
replay loop. Unexpected (non-stale) failures still raise immediately.

Raises
------
PublishPreflightError
Expand Down
4 changes: 3 additions & 1 deletion scripts/typos_rollout.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ def _string_list(table: cabc.Mapping[str, object], key: str) -> tuple[str, ...]:
if not isinstance(value, list) or not all(isinstance(item, str) for item in value):
message = f"{key!r} must be a list of strings"
raise TypeError(message)
return tuple(sorted(set(value)))
# The guard above has already proven every element is a str, so cast the
# validated list directly rather than filtering it a second time.
return tuple(sorted(set(typ.cast("list[str]", value))))


def _table(document: cabc.Mapping[str, object], key: str) -> cabc.Mapping[str, object]:
Expand Down
13 changes: 13 additions & 0 deletions tests/bdd/features/cli.feature
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,19 @@ Feature: Lading CLI scaffolding
Then the CLI exits with code 1
And the stderr contains "cargo generate-lockfile --manifest-path"

Scenario: Publish pre-flight aggregates every stale Cargo.lock file
Given a workspace directory with configuration
And cargo metadata describes a sample workspace
And publish pre-flight finds multiple stale tracked Cargo.lock files
When I invoke lading publish with that workspace
Then the CLI exits with code 1
And the stderr contains "Tracked Cargo.lock files are stale after manifest version changes."
And the stderr contains workspace path "Cargo.lock"
And the stderr contains workspace path "sub/Cargo.lock"
And the stderr contains "cargo generate-lockfile --manifest-path"
And the stderr contains workspace path "Cargo.toml"
And the stderr contains workspace path "sub/Cargo.toml"

Scenario: Publish pre-flight skips configured cargo test crates
Given a workspace directory with configuration
And cargo metadata describes a sample workspace
Expand Down
18 changes: 18 additions & 0 deletions tests/bdd/steps/test_common_steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,24 @@ def then_stderr_contains(cli_run: dict[str, typ.Any], expected: str) -> None:
assert expected in cli_run["stderr"]


@then(parsers.parse('the stderr contains workspace path "{relative}"'))
def then_stderr_contains_workspace_path(
cli_run: dict[str, typ.Any], relative: str
) -> None:
"""Assert stderr contains the absolute path ``relative`` under the workspace.

The workspace root is a per-test temporary directory, so scenarios cannot
hard-code its absolute path. Resolving ``relative`` against
``cli_run["workspace"]`` lets a scenario assert a specific lockfile or
manifest path — distinguishing, for example, the workspace-root
``Cargo.lock`` from a nested crate's — rather than a substring both share.
"""
expected = str(cli_run["workspace"] / relative)
assert expected in cli_run["stderr"], (
f"expected workspace path {expected!r} in stderr:\n{cli_run['stderr']}"
)


@then(parsers.parse('the workspace manifest version is "{version}"'))
def then_workspace_manifest_version(
cli_run: dict[str, typ.Any],
Expand Down
34 changes: 34 additions & 0 deletions tests/bdd/steps/test_publish_given_steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,40 @@ def given_publish_preflight_finds_stale_lockfile(
)


@given("publish pre-flight finds multiple stale tracked Cargo.lock files")
def given_publish_preflight_finds_multiple_stale_lockfiles(
workspace_directory: Path,
preflight_overrides: dict[tuple[str, ...], ResponseProvider],
) -> None:
"""Simulate several tracked lockfiles that all fail locked validation.

Two tracked lockfiles (the workspace root and a nested ``sub`` crate) are
staged on disk with adjacent manifests so discovery returns both, and a
single prefix-matched ``cargo metadata --locked`` stub reports each as
stale. This exercises the aggregated, no-short-circuit stale report.
"""
nested = workspace_directory / "sub"
nested.mkdir(parents=True, exist_ok=True)
(nested / "Cargo.toml").write_text("[package]\n", encoding="utf-8")
(workspace_directory / "Cargo.lock").write_text("# stale lock\n", encoding="utf-8")
(nested / "Cargo.lock").write_text("# stale lock\n", encoding="utf-8")
preflight_overrides["git", "ls-files", "**/Cargo.lock", "Cargo.lock"] = (
_CommandResponse(exit_code=0, stdout="Cargo.lock\nsub/Cargo.lock\n")
)
preflight_overrides[
"cargo",
"metadata",
"--locked",
"--manifest-path",
] = _CommandResponse(
exit_code=101,
stderr=(
"error: cannot update the lock file because --locked was passed "
"to prevent this"
),
)


@given(parsers.parse('cargo test fails with compiletest artifact "{relative_path}"'))
def given_cargo_test_fails_with_artifact(
workspace_directory: Path,
Expand Down
Loading
Loading