diff --git a/docs/issues/open/1778-migrate-to-rust-edition-2024.md b/docs/issues/closed/1778-migrate-to-rust-edition-2024.md similarity index 99% rename from docs/issues/open/1778-migrate-to-rust-edition-2024.md rename to docs/issues/closed/1778-migrate-to-rust-edition-2024.md index b5858816e..ac9059f75 100644 --- a/docs/issues/open/1778-migrate-to-rust-edition-2024.md +++ b/docs/issues/closed/1778-migrate-to-rust-edition-2024.md @@ -1,10 +1,10 @@ --- doc-type: issue issue-type: task -status: in-review +status: closed priority: p3 github-issue: 1778 -spec-path: docs/issues/open/1778-migrate-to-rust-edition-2024.md +spec-path: docs/issues/closed/1778-migrate-to-rust-edition-2024.md branch: "1778-migrate-to-rust-edition-2024" related-pr: 1784 last-updated-utc: 2026-05-14 18:30 diff --git a/docs/issues/open/1780-refactor-pre-push-checks-performance-and-verbosity.md b/docs/issues/closed/1780-refactor-pre-push-checks-performance-and-verbosity.md similarity index 99% rename from docs/issues/open/1780-refactor-pre-push-checks-performance-and-verbosity.md rename to docs/issues/closed/1780-refactor-pre-push-checks-performance-and-verbosity.md index 689c058ba..ba8c638c7 100644 --- a/docs/issues/open/1780-refactor-pre-push-checks-performance-and-verbosity.md +++ b/docs/issues/closed/1780-refactor-pre-push-checks-performance-and-verbosity.md @@ -1,10 +1,10 @@ --- doc-type: issue issue-type: enhancement -status: planned +status: closed priority: p1 github-issue: 1780 -spec-path: docs/issues/open/1780-refactor-pre-push-checks-performance-and-verbosity.md +spec-path: docs/issues/closed/1780-refactor-pre-push-checks-performance-and-verbosity.md branch: "1780-refactor-pre-push-checks-performance-and-verbosity" related-pr: null last-updated-utc: 2026-05-13 21:00 diff --git a/docs/issues/open/1786-tighten-lint-config.md b/docs/issues/open/1786-tighten-lint-config.md new file mode 100644 index 000000000..c4660fdac --- /dev/null +++ b/docs/issues/open/1786-tighten-lint-config.md @@ -0,0 +1,181 @@ +--- +doc-type: issue +issue-type: task +status: planned +priority: p2 +github-issue: 1786 +spec-path: docs/issues/open/1786-tighten-lint-config.md +branch: "1786-tighten-lint-config" +related-pr: 1784 +last-updated-utc: 2026-05-15 08:00 +semantic-links: + skill-links: + - create-issue + related-artifacts: + - Cargo.toml + - .cargo/config.toml +--- + + + +# Issue #1786 - Migrate lint configuration to `[workspace.lints]` in Cargo.toml + +## Goal + +Replace the ad-hoc lint configuration spread across `.cargo/config.toml` RUSTFLAGS and +`torrust-linting` command-line arguments with a single authoritative `[workspace.lints]` +section in `Cargo.toml`, following the idiomatic Cargo approach used in `torrust-index`. + +## Background + +Lint enforcement is currently split across three places: + +1. **`.cargo/config.toml` RUSTFLAGS** — carries rust-group denials (`-D warnings`, + `-D future-incompatible`, `-D rust-2018-idioms`, etc.). These apply to every cargo + invocation (build, test, check) but are invisible without reading the config file. + +2. **`torrust-linting` clippy runner** — passes `-D clippy::correctness`, + `-D clippy::suspicious`, `-D clippy::complexity`, `-D clippy::perf`, + `-D clippy::style`, `-D clippy::pedantic` on the command line. These are only + active when the linter tool runs; `cargo clippy` invoked directly does not + apply them. + +3. **`[lints.clippy]` on the root `[package]`** — the root `Cargo.toml` already has a + `[lints.clippy]` section for the main binary package only; this is _not_ a + `[workspace.lints]` and does not propagate to other workspace members. It also + contains `needless_return = "allow"` with a `# temp allow this lint` comment, + suggesting it was added as a temporary workaround rather than a deliberate policy + decision. The original reason and whether the underlying callsites have since been + fixed is unknown; this must be investigated before the section is migrated or removed. + +This fragmentation was raised in PR #1784 review by @da2ce7, who referenced the +`torrust-index` configuration as the target state. + +Cargo 1.64+ supports `[workspace.lints]`, the idiomatic way to declare workspace-wide +lint policy in a single, visible, version-controlled location. + +## Scope + +### In Scope + +- Add `[workspace.lints.rust]` to the root `Cargo.toml` with the lint groups currently + expressed as RUSTFLAGS. +- Add `[workspace.lints.clippy]` to the root `Cargo.toml` with the clippy groups + currently passed by `torrust-linting`, plus `nursery = "warn"` as suggested in the + PR review. +- Remove the now-redundant lint entries from `RUSTFLAGS` in `.cargo/config.toml`. +- Remove the root `[lints.clippy]` package-level section (superseded by workspace lints). +- Fix any new warnings or errors that surface once `nursery = "warn"` and + `all = "deny"` take effect (expected to be small; most lints are already enforced). +- Investigate the `needless_return = "allow"` entry (see T7 below) and resolve it. +- Coordinate with `torrust-linting`: either remove the redundant `-D clippy::X` flags + from the clippy runner (cleaner) or document that they are intentional redundancy + (safety net). A follow-up PR to `torrust-linting` may be needed. + +### Out of Scope + +- Changes to any other lint policy beyond migrating the existing set. +- Enabling additional deny-level lints beyond what is listed in the Background section. +- Changes to `torrust-linting` beyond removing the now-redundant clippy group flags. +- MSRV changes (tracked separately in #1787). + +## Implementation Plan + +Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. + +| ID | Status | Task | Notes / Expected Output | +| --- | ------ | ------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------ | +| T1 | TODO | Add `[workspace.lints.rust]` to root `Cargo.toml` | Mirrors current RUSTFLAGS entries; `rust-2024-compatibility` added | +| T2 | TODO | Add `[workspace.lints.clippy]` to root `Cargo.toml` | Matches torrust-index config; `nursery = "warn"`, `all = "deny"` | +| T3 | TODO | Remove redundant RUSTFLAGS lint entries from `.cargo/config.toml` | Only lint-related entries removed; other rustflags (e.g. `-D unused`) migrated too | +| T4 | TODO | Remove root `[lints.clippy]` package section from `Cargo.toml` | Superseded by `[workspace.lints.clippy]` | +| T5 | TODO | Fix any new lint failures from `nursery = "warn"` / `all = "deny"` | `cargo clippy --workspace --all-targets --all-features` must pass cleanly | +| T6 | TODO | Update `torrust-linting` to remove redundant `-D clippy::X` flags | Open a separate PR in `torrust-linting`; document decision if deferred | +| T7 | TODO | Investigate and resolve `needless_return = "allow"` in `Cargo.toml` | See Background; decide: fix callsites and remove the allow, or keep it with documented rationale | +| T8 | TODO | Verify all quality gates pass | `linter all`, doc tests, full test suite, pre-push hook | + +## Progress Tracking + +### Workflow Checkpoints + +- [ ] Spec drafted in `docs/issues/drafts/` +- [x] Spec reviewed and approved by user/maintainer +- [x] GitHub issue created and issue number added to this spec +- [ ] Implementation completed +- [ ] Automatic verification completed (`linter all`, relevant tests, and pre-push checks) +- [ ] Manual verification scenarios executed and recorded (status + evidence) +- [ ] Acceptance criteria reviewed after implementation and updated with evidence +- [ ] Reviewer validated acceptance criteria and updated checkboxes +- [ ] Committer verified spec progress is up to date before commit +- [ ] Issue closed and spec moved from `docs/issues/open/` to `docs/issues/closed/` + +### Progress Log + +- 2026-05-15 07:00 UTC - Agent - Spec drafted, triggered by @da2ce7 review comment on PR #1784 +- 2026-05-15 08:00 UTC - Agent - GitHub issue #1786 created; spec moved from drafts/ to open/ + +## Acceptance Criteria + +- [ ] AC1: `[workspace.lints.rust]` in `Cargo.toml` covers all groups previously in RUSTFLAGS +- [ ] AC2: `[workspace.lints.clippy]` in `Cargo.toml` covers all groups previously passed by `torrust-linting`, plus `nursery = "warn"` and `all = "deny"` +- [ ] AC3: `.cargo/config.toml` no longer contains lint-related RUSTFLAGS entries +- [ ] AC4: The root package `[lints.clippy]` section is removed +- [ ] AC5: `cargo clippy --workspace --all-targets --all-features` exits `0` with no warnings +- [ ] AC6: `linter all` exits `0` +- [ ] AC7: All tests pass (`cargo test --workspace --all-targets --all-features`) +- [ ] AC8: Pre-push hook passes +- [ ] AC9: The `needless_return` allow is either removed (callsites fixed) or kept with a documented rationale replacing the `# temp allow this lint` comment +- [ ] AC10: Manual verification scenarios are executed and documented (status + evidence) +- [ ] AC11: Acceptance criteria are re-reviewed after implementation and reflect actual behavior + +## Verification Plan + +### Automatic Checks + +- `linter all` +- `cargo clippy --workspace --all-targets --all-features` +- `cargo test --doc --workspace` +- `cargo test --tests --benches --examples --workspace --all-targets --all-features` +- Pre-push hook (full gate) + +### Manual Verification Scenarios + +Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`. + +| ID | Scenario | Command/Steps | Expected Result | Status | Evidence | +| --- | ------------------------------------------------------------- | ------------------------------------------------------------ | ------------------------------------------- | ------ | -------- | +| M1 | Direct `cargo clippy` enforces workspace lints without linter | `cargo clippy --workspace --all-targets --all-features` | Exits 0; pedantic/nursery lints applied | TODO | | +| M2 | `cargo build` no longer picks up redundant lint RUSTFLAGS | `cargo build --workspace` (inspect output for lint warnings) | No spurious warnings from removed RUSTFLAGS | TODO | | +| M3 | `linter all` still passes with the new configuration | `linter all` | Exits 0 | TODO | | + +### Acceptance Verification + +| AC ID | Status (`TODO`/`DONE`) | Evidence | +| ----- | ---------------------- | -------- | +| AC1 | TODO | | +| AC2 | TODO | | +| AC3 | TODO | | +| AC4 | TODO | | +| AC5 | TODO | | +| AC6 | TODO | | +| AC7 | TODO | | +| AC8 | TODO | | +| AC9 | TODO | | +| AC10 | TODO | | +| AC11 | TODO | | + +## Risks and Trade-offs + +- **`nursery = "warn"` may surface many warnings**: nursery lints are experimental and + can be noisy. Fixing them is not mandatory for CI to pass (warn, not deny), but a + large warning count degrades signal quality. Monitor after enabling. +- **`torrust-linting` coordination**: if the redundant `-D` flags are left in the linter + after workspace lints are added, they remain harmless (idempotent) but add confusion. + Cleaning them up requires a separate PR to `torrust-linting`. + +## References + +- Related PRs: #1784 +- Suggested by: @da2ce7 in PR #1784 review +- Reference config: `torrust-index` workspace `Cargo.toml` +- Related issue: #1787 (evaluate MSRV bump) diff --git a/docs/issues/open/1787-evaluate-msrv-bump.md b/docs/issues/open/1787-evaluate-msrv-bump.md new file mode 100644 index 000000000..9dacd69ad --- /dev/null +++ b/docs/issues/open/1787-evaluate-msrv-bump.md @@ -0,0 +1,190 @@ +--- +doc-type: issue +issue-type: task +status: blocked +priority: p2 +github-issue: 1787 +spec-path: docs/issues/open/1787-evaluate-msrv-bump.md +branch: "1787-evaluate-msrv-bump" +related-pr: 1784 +last-updated-utc: 2026-05-15 08:00 +blocked-by: "#1669 (package restructuring)" +semantic-links: + skill-links: + - create-issue + related-artifacts: + - Cargo.toml + - AGENTS.md + - .github/skills/dev/maintenance/setup-dev-environment/SKILL.md +--- + + + +# Issue #1787 - Evaluate and update workspace MSRV above 1.85 + +## Goal + +Decide on the appropriate Minimum Supported Rust Version (MSRV) for the workspace +given the project's trajectory (planned extraction of `bittorrent-*` crates as +independent libraries) and update `rust-version` in `Cargo.toml` accordingly. + +## Background + +PR #1784 set `rust-version = "1.85"` — the strict minimum required to compile +Rust edition 2024. This was correct as the conservative baseline for the migration, +but 1.85 is now several releases behind the current stable toolchain. + +Two classes of crate coexist in this workspace: + +1. **Application layer** (`torrust-tracker-*` crates and the main binary) — not + consumed as a library by external projects; MSRV currently has no downstream + impact. All workspace packages carry `publish.workspace = true` but none have + been published to crates.io yet. Which packages will actually be released, + under what names, and whether some will move to their own repositories is + being decided in #1669. + +2. **Protocol/domain layer** (`bittorrent-*` crates: `bittorrent-peer-id`, + `bittorrent-http-tracker-protocol`, `bittorrent-udp-tracker-protocol`, + `bittorrent-tracker-core`, `bittorrent-http-tracker-core`, + `bittorrent-udp-tracker-core`, `bittorrent-tracker-client`) — planned for + extraction into independent repositories and publication to crates.io, where + they will be consumed by other BitTorrent projects. + +This dual nature creates a tension: + +- **For the application layer**: there is no reason to stay on an old MSRV; tracking + a recent stable is better (access to new APIs, better diagnostics). +- **For the future libraries**: a conservative MSRV (e.g. latest stable minus two + releases, or a deliberate policy) is appropriate once they are published. + +Until the `bittorrent-*` crates are extracted, a single workspace MSRV applies to +both classes, so the decision must be made with the extraction timeline in mind. + +**This issue is currently blocked on #1669** (ongoing package restructuring). +Several decisions that directly affect the MSRV strategy have not yet been made: + +- Which packages will be extracted as independent crates.io libraries. +- Final names for those packages. +- Which packages will share a versioning lifecycle with the main tracker and which + will evolve independently. +- Publication targets and minimum toolchain expectations for downstream consumers. + +The MSRV evaluation should be re-opened only after #1669 has settled these questions. +Opening it sooner risks choosing a policy that becomes invalid once extraction scope +is defined. + +## Scope + +### In Scope + +- Evaluate the appropriate MSRV policy for this workspace given the two crate classes. +- Define a policy: track latest stable, pin to a specific recent release, or maintain + a conservative floor. +- Update `rust-version` in `Cargo.toml` to the agreed value. +- Update all documentation that references the MSRV: + - `AGENTS.md` (line referencing `MSRV 1.85`) + - `.github/skills/dev/maintenance/setup-dev-environment/SKILL.md` +- Verify CI passes with the new MSRV value. + +### Out of Scope + +- Extracting `bittorrent-*` crates to independent repositories (separate epic). +- Setting per-crate MSRV values (only the workspace `rust-version` is in scope here). +- Adding a MSRV CI job (may be proposed as a follow-up if a conservative MSRV is chosen). + +## Blockers + +- **#1669 — Package restructuring**: names, extraction scope, versioning lifecycle, and + publication targets for the `bittorrent-*` crates must be decided before a rational + MSRV policy can be set. Track that issue and re-evaluate when it closes. + +## Implementation Plan + +Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. + +| ID | Status | Task | Notes / Expected Output | +| --- | ------ | ------------------------------------------------------------------- | ------------------------------------------------------ | +| T1 | TODO | Decide MSRV policy (track latest stable vs. pin conservative floor) | Document the rationale in this spec before proceeding | +| T2 | TODO | Update `rust-version` in root `Cargo.toml` | Change from `"1.85"` to the agreed value | +| T3 | TODO | Update `AGENTS.md` MSRV reference | Keep in sync with `Cargo.toml` | +| T4 | TODO | Update setup-dev-environment SKILL.md MSRV reference | Keep in sync with `Cargo.toml` | +| T5 | TODO | Verify CI passes | Full quality gate (`linter all`, tests, pre-push hook) | + +## Progress Tracking + +### Workflow Checkpoints + +- [ ] Spec drafted in `docs/issues/drafts/` +- [x] Spec reviewed and approved by user/maintainer +- [x] GitHub issue created and issue number added to this spec +- [ ] Implementation completed +- [ ] Automatic verification completed (`linter all`, relevant tests, and pre-push checks) +- [ ] Manual verification scenarios executed and recorded (status + evidence) +- [ ] Acceptance criteria reviewed after implementation and updated with evidence +- [ ] Reviewer validated acceptance criteria and updated checkboxes +- [ ] Committer verified spec progress is up to date before commit +- [ ] Issue closed and spec moved from `docs/issues/open/` to `docs/issues/closed/` + +### Progress Log + +- 2026-05-15 07:00 UTC - Agent - Spec drafted, follow-up from PR #1784 (Rust edition 2024 migration, MSRV set to 1.85) +- 2026-05-15 07:30 UTC - Jose Celano - Marked blocked on #1669 (package restructuring); MSRV policy requires knowing extraction scope, names, and versioning lifecycle +- 2026-05-15 08:00 UTC - Agent - GitHub issue #1787 created; spec moved to docs/issues/open/ + +## Acceptance Criteria + +- [ ] AC1: A MSRV policy decision is recorded in this spec with rationale +- [ ] AC2: `rust-version` in `Cargo.toml` reflects the agreed value +- [ ] AC3: `AGENTS.md` MSRV reference is in sync with `Cargo.toml` +- [ ] AC4: `setup-dev-environment` SKILL.md MSRV reference is in sync with `Cargo.toml` +- [ ] AC5: `linter all` exits `0` +- [ ] AC6: All tests pass +- [ ] AC7: Pre-push hook passes + +## Verification Plan + +### Automatic Checks + +- `linter all` +- `cargo check --workspace --all-targets --all-features` +- `cargo test --doc --workspace` +- Pre-push hook (full gate) + +### Manual Verification Scenarios + +Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`. + +| ID | Scenario | Command/Steps | Expected Result | Status | Evidence | +| --- | -------------------------------------------------- | ------------------------------------------------ | ---------------------------------------- | ------ | -------- | +| M1 | `rust-version` in Cargo.toml matches documentation | Compare `Cargo.toml`, `AGENTS.md`, SKILL.md | All three reference the same MSRV string | TODO | | +| M2 | Workspace builds cleanly on the new MSRV toolchain | `rustup install ; cargo + check ...` | Exit 0 with no errors | TODO | | + +### Acceptance Verification + +| AC ID | Status (`TODO`/`DONE`) | Evidence | +| ----- | ---------------------- | -------- | +| AC1 | TODO | | +| AC2 | TODO | | +| AC3 | TODO | | +| AC4 | TODO | | +| AC5 | TODO | | +| AC6 | TODO | | +| AC7 | TODO | | + +## Risks and Trade-offs + +- **Too high a MSRV before crate extraction**: if `bittorrent-*` crates are extracted + carrying a high MSRV, downstream BitTorrent projects may be forced to upgrade their + toolchain. Setting a modest floor now (e.g. current stable minus two releases) gives + the extracted crates a clean, defensible starting point. +- **Too low a MSRV after extraction**: the application layer has no reason to stay + conservative; a low MSRV denies developers access to new stable APIs and better + compiler diagnostics. +- **Drift without a MSRV CI job**: a stated MSRV is only trustworthy if CI verifies it. + If a conservative MSRV is chosen, a MSRV CI job should be added. + +## References + +- Related PRs: #1784 +- Related issue: #1786 (tighten lint config) +- Blocked by: https://github.com/torrust/torrust-tracker/issues/1669 (package restructuring) diff --git a/project-words.txt b/project-words.txt index e88296595..5a234b4e7 100644 --- a/project-words.txt +++ b/project-words.txt @@ -39,6 +39,7 @@ Buildx byteorder callgrind CALLSITE +callsites camino canonicalize canonicalized