Skip to content

Use released DotSlash package for argument-comment lint#15199

Merged
bolinfest merged 1 commit intomainfrom
pr15199
Mar 20, 2026
Merged

Use released DotSlash package for argument-comment lint#15199
bolinfest merged 1 commit intomainfrom
pr15199

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Mar 19, 2026

Why

The argument-comment lint now has a packaged DotSlash artifact from #15198, so the normal repo lint path should use that released payload instead of rebuilding the lint from source every time.

That keeps just clippy and CI aligned with the shipped artifact while preserving a separate source-build path for people actively hacking on the lint crate.

The current alpha package also exposed two integration wrinkles that the repo-side prebuilt wrapper needs to smooth over:

  • the bundled Dylint library filename includes the host triple, for example @nightly-2025-09-18-aarch64-apple-darwin, and Dylint derives RUSTUP_TOOLCHAIN from that filename
  • on Windows, Dylint's driver path also expects RUSTUP_HOME to be present in the environment

Without those adjustments, the prebuilt CI jobs fail during cargo metadata or driver setup. This change makes the checked-in prebuilt wrapper normalize the packaged library name to the plain nightly-2025-09-18 channel before invoking cargo-dylint, and it teaches both the wrapper and the packaged runner source to infer RUSTUP_HOME from rustup show home when the environment does not already provide it.

After the prebuilt Windows lint job started running successfully, it also surfaced a handful of existing anonymous literal callsites in windows-sandbox-rs. This PR now annotates those callsites so the new cross-platform lint job is green on the current tree.

What Changed

  • checked in the current tools/argument-comment-lint/argument-comment-lint DotSlash manifest
  • kept tools/argument-comment-lint/run.sh as the source-build wrapper for lint development
  • added tools/argument-comment-lint/run-prebuilt-linter.sh as the normal enforcement path, using the checked-in DotSlash package and bundled cargo-dylint
  • updated just clippy and just argument-comment-lint to use the prebuilt wrapper
  • split .github/workflows/rust-ci.yml so source-package checks live in a dedicated argument_comment_lint_package job, while the released lint runs in an argument_comment_lint_prebuilt matrix on Linux, macOS, and Windows
  • kept the pinned nightly-2025-09-18 toolchain install in the prebuilt CI matrix, since the prebuilt package still relies on rustup-provided toolchain components
  • updated tools/argument-comment-lint/run-prebuilt-linter.sh to normalize host-qualified nightly library filenames, keep the rustup shim directory ahead of direct toolchain cargo binaries, and export RUSTUP_HOME when needed for Windows Dylint driver setup
  • updated tools/argument-comment-lint/src/bin/argument-comment-lint.rs so future published DotSlash artifacts apply the same nightly-filename normalization and RUSTUP_HOME inference internally
  • fixed the remaining Windows lint violations in codex-rs/windows-sandbox-rs by adding the required /*param*/ comments at the reported callsites
  • documented the checked-in DotSlash file, wrapper split, archive layout, nightly prerequisite, and Windows RUSTUP_HOME requirement in tools/argument-comment-lint/README.md

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 08cf074f2e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
lint_path="$repo_root/tools/argument-comment-lint"
manifest_path="$repo_root/codex-rs/Cargo.toml"
release_manifest_url="https://github.com/openai/codex/releases/latest/download/argument-comment-lint"
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.

P2 Badge Pin the packaged lint runner to the checked-out revision

When dotslash is installed, run.sh now pulls https://github.com/openai/codex/releases/latest/... and executes that package instead of the tools/argument-comment-lint code in the current checkout. That makes lint results non-reproducible across branches/tags and, more importantly, it means local validation of changes under tools/argument-comment-lint will silently ignore the branch's own lint/runner edits until a release is cut. Please tie the packaged runner to the checked-out version (or disable it when this directory differs) so ./tools/argument-comment-lint/run.sh actually tests the code being reviewed.

Useful? React with 👍 / 👎.

Base automatically changed from pr15198 to main March 19, 2026 18:59
@bolinfest bolinfest force-pushed the pr15199 branch 11 times, most recently from 6d9c0c6 to eef5b65 Compare March 20, 2026 01:27
@bolinfest bolinfest requested a review from aibrahim-oai March 20, 2026 01:31
@bolinfest bolinfest force-pushed the pr15199 branch 4 times, most recently from a1db96f to eec3a8e Compare March 20, 2026 01:59
@bolinfest bolinfest force-pushed the pr15199 branch 2 times, most recently from cbd0ecf to bfe26bd Compare March 20, 2026 02:27
@bolinfest bolinfest enabled auto-merge (squash) March 20, 2026 02:30
@bolinfest bolinfest force-pushed the pr15199 branch 2 times, most recently from 69544dc to 0a9e8c3 Compare March 20, 2026 02:43
@bolinfest bolinfest merged commit fa2a2f0 into main Mar 20, 2026
38 of 40 checks passed
@bolinfest bolinfest deleted the pr15199 branch March 20, 2026 03:19
@github-actions github-actions bot locked and limited conversation to collaborators Mar 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants