From b87ba0a3cc1ee3cb1f558233a8d4e3b994217795 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 19 Mar 2026 11:59:02 -0700 Subject: [PATCH] Publish runnable DotSlash package for argument-comment lint (#15198) ## Why To date, the argument-comment linter introduced in https://github.com/openai/codex/pull/14651 had to be built from source to run, which can be a bit slow (both for local dev and when it is run in CI). Because of the potential slowness, I did not wire it up to run as part of `just clippy` or anything like that. As a result, I have seen a number of occasions where folks put up PRs that violate the lint, see it fail in CI, and then have to put up their PR again. The goal of this PR is to pre-build a runnable version of the linter and then make it available via a DotSlash file. Once it is available, I will update `just clippy` and other touchpoints to make it a natural part of the dev cycle so lint violations should get flagged _before_ putting up a PR for review. To get things started, we will build the DotSlash file as part of an alpha release. Though I don't expect the linter to change often, so I'll probably change this to only build as part of mainline releases once we have a working DotSlash file. (Ultimately, we should probably move the linter into its own repo so it can have its own release cycle.) ## What Changed - add a reusable `rust-release-argument-comment-lint.yml` workflow that builds host-specific archives for macOS arm64, Linux arm64/x64, and Windows x64 - wire `rust-release.yml` to publish the `argument-comment-lint` DotSlash manifest on all releases for now, including alpha tags - package a runnable layout instead of a bare library The Unix archive layout is: ```text argument-comment-lint/ bin/ argument-comment-lint cargo-dylint lib/ libargument_comment_lint@nightly-2025-09-18-.dylib|so ``` On Windows the same layout is published as a `.zip`, with `.exe` and `.dll` filenames instead. DotSlash resolves the package entrypoint to `argument-comment-lint/bin/argument-comment-lint`. That runner finds the sibling bundled `cargo-dylint` binary plus the single packaged Dylint library under `lib/`, then invokes `cargo-dylint dylint --lib-path ` with the repo's default lint settings. --- ...dotslash-argument-comment-lint-config.json | 24 +++ .../rust-release-argument-comment-lint.yml | 103 +++++++++++ .github/workflows/rust-release.yml | 15 ++ tools/argument-comment-lint/README.md | 5 + .../src/bin/argument-comment-lint.rs | 164 ++++++++++++++++++ 5 files changed, 311 insertions(+) create mode 100644 .github/dotslash-argument-comment-lint-config.json create mode 100644 .github/workflows/rust-release-argument-comment-lint.yml create mode 100644 tools/argument-comment-lint/src/bin/argument-comment-lint.rs diff --git a/.github/dotslash-argument-comment-lint-config.json b/.github/dotslash-argument-comment-lint-config.json new file mode 100644 index 000000000..19a2a4803 --- /dev/null +++ b/.github/dotslash-argument-comment-lint-config.json @@ -0,0 +1,24 @@ +{ + "outputs": { + "argument-comment-lint": { + "platforms": { + "macos-aarch64": { + "regex": "^argument-comment-lint-aarch64-apple-darwin\\.tar\\.gz$", + "path": "argument-comment-lint/bin/argument-comment-lint" + }, + "linux-x86_64": { + "regex": "^argument-comment-lint-x86_64-unknown-linux-gnu\\.tar\\.gz$", + "path": "argument-comment-lint/bin/argument-comment-lint" + }, + "linux-aarch64": { + "regex": "^argument-comment-lint-aarch64-unknown-linux-gnu\\.tar\\.gz$", + "path": "argument-comment-lint/bin/argument-comment-lint" + }, + "windows-x86_64": { + "regex": "^argument-comment-lint-x86_64-pc-windows-msvc\\.zip$", + "path": "argument-comment-lint/bin/argument-comment-lint.exe" + } + } + } + } +} diff --git a/.github/workflows/rust-release-argument-comment-lint.yml b/.github/workflows/rust-release-argument-comment-lint.yml new file mode 100644 index 000000000..a0d12d6db --- /dev/null +++ b/.github/workflows/rust-release-argument-comment-lint.yml @@ -0,0 +1,103 @@ +name: rust-release-argument-comment-lint + +on: + workflow_call: + inputs: + publish: + required: true + type: boolean + +jobs: + skip: + if: ${{ !inputs.publish }} + runs-on: ubuntu-latest + steps: + - run: echo "Skipping argument-comment-lint release assets for prerelease tag" + + build: + if: ${{ inputs.publish }} + name: Build - ${{ matrix.runner }} - ${{ matrix.target }} + runs-on: ${{ matrix.runs_on || matrix.runner }} + timeout-minutes: 60 + + strategy: + fail-fast: false + matrix: + include: + - runner: macos-15-xlarge + target: aarch64-apple-darwin + archive_name: argument-comment-lint-aarch64-apple-darwin.tar.gz + lib_name: libargument_comment_lint@nightly-2025-09-18-aarch64-apple-darwin.dylib + runner_binary: argument-comment-lint + cargo_dylint_binary: cargo-dylint + - runner: ubuntu-24.04 + target: x86_64-unknown-linux-gnu + archive_name: argument-comment-lint-x86_64-unknown-linux-gnu.tar.gz + lib_name: libargument_comment_lint@nightly-2025-09-18-x86_64-unknown-linux-gnu.so + runner_binary: argument-comment-lint + cargo_dylint_binary: cargo-dylint + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-gnu + archive_name: argument-comment-lint-aarch64-unknown-linux-gnu.tar.gz + lib_name: libargument_comment_lint@nightly-2025-09-18-aarch64-unknown-linux-gnu.so + runner_binary: argument-comment-lint + cargo_dylint_binary: cargo-dylint + - runner: windows-x64 + target: x86_64-pc-windows-msvc + archive_name: argument-comment-lint-x86_64-pc-windows-msvc.zip + lib_name: argument_comment_lint@nightly-2025-09-18-x86_64-pc-windows-msvc.dll + runner_binary: argument-comment-lint.exe + cargo_dylint_binary: cargo-dylint.exe + runs_on: + group: codex-runners + labels: codex-windows-x64 + + steps: + - uses: actions/checkout@v6 + + - uses: dtolnay/rust-toolchain@1.93.0 + with: + toolchain: nightly-2025-09-18 + targets: ${{ matrix.target }} + components: llvm-tools-preview, rustc-dev, rust-src + + - name: Install tooling + shell: bash + run: | + install_root="${RUNNER_TEMP}/argument-comment-lint-tools" + cargo install --locked cargo-dylint --root "$install_root" + cargo install --locked dylint-link + echo "INSTALL_ROOT=$install_root" >> "$GITHUB_ENV" + + - name: Cargo build + working-directory: tools/argument-comment-lint + shell: bash + run: cargo build --release --target ${{ matrix.target }} + + - name: Stage artifact + shell: bash + run: | + dest="dist/argument-comment-lint/${{ matrix.target }}" + mkdir -p "$dest" + package_root="${RUNNER_TEMP}/argument-comment-lint" + rm -rf "$package_root" + mkdir -p "$package_root/bin" "$package_root/lib" + + cp "tools/argument-comment-lint/target/${{ matrix.target }}/release/${{ matrix.runner_binary }}" \ + "$package_root/bin/${{ matrix.runner_binary }}" + cp "${INSTALL_ROOT}/bin/${{ matrix.cargo_dylint_binary }}" \ + "$package_root/bin/${{ matrix.cargo_dylint_binary }}" + cp "tools/argument-comment-lint/target/${{ matrix.target }}/release/${{ matrix.lib_name }}" \ + "$package_root/lib/${{ matrix.lib_name }}" + + archive_path="$dest/${{ matrix.archive_name }}" + if [[ "${{ runner.os }}" == "Windows" ]]; then + (cd "${RUNNER_TEMP}" && 7z a "$GITHUB_WORKSPACE/$archive_path" argument-comment-lint >/dev/null) + else + (cd "${RUNNER_TEMP}" && tar -czf "$GITHUB_WORKSPACE/$archive_path" argument-comment-lint) + fi + + - uses: actions/upload-artifact@v7 + with: + name: argument-comment-lint-${{ matrix.target }} + path: dist/argument-comment-lint/${{ matrix.target }}/* diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 12f2fc0d8..35078cf33 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -380,11 +380,19 @@ jobs: publish: true secrets: inherit + argument-comment-lint-release-assets: + name: argument-comment-lint release assets + needs: tag-check + uses: ./.github/workflows/rust-release-argument-comment-lint.yml + with: + publish: true + release: needs: - build - build-windows - shell-tool-mcp + - argument-comment-lint-release-assets name: release runs-on: ubuntu-latest permissions: @@ -521,6 +529,13 @@ jobs: tag: ${{ github.ref_name }} config: .github/dotslash-config.json + - uses: facebook/dotslash-publish-release@v2 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + tag: ${{ github.ref_name }} + config: .github/dotslash-argument-comment-lint-config.json + - name: Trigger developers.openai.com deploy # Only trigger the deploy if the release is not a pre-release. # The deploy is used to update the developers.openai.com website with the new config schema json file. diff --git a/tools/argument-comment-lint/README.md b/tools/argument-comment-lint/README.md index 82e5605e3..91c1fdecc 100644 --- a/tools/argument-comment-lint/README.md +++ b/tools/argument-comment-lint/README.md @@ -68,6 +68,11 @@ cd tools/argument-comment-lint cargo test ``` +GitHub releases also publish a DotSlash file named +`argument-comment-lint` for macOS arm64, Linux arm64, Linux x64, and Windows +x64. The published package contains a small runner executable, a bundled +`cargo-dylint`, and the prebuilt lint library. + Run the lint against `codex-rs` from the repo root: ```bash diff --git a/tools/argument-comment-lint/src/bin/argument-comment-lint.rs b/tools/argument-comment-lint/src/bin/argument-comment-lint.rs new file mode 100644 index 000000000..e3175efe5 --- /dev/null +++ b/tools/argument-comment-lint/src/bin/argument-comment-lint.rs @@ -0,0 +1,164 @@ +use std::env; +use std::ffi::OsString; +use std::fs; +use std::path::Path; +use std::path::PathBuf; +use std::process::Command; +use std::process::ExitCode; + +fn main() -> ExitCode { + match run() { + Ok(code) => code, + Err(err) => { + eprintln!("{err}"); + ExitCode::from(1) + } + } +} + +fn run() -> Result { + let exe_path = + env::current_exe().map_err(|err| format!("failed to locate current executable: {err}"))?; + let bin_dir = exe_path.parent().ok_or_else(|| { + format!( + "failed to locate parent directory for executable {}", + exe_path.display() + ) + })?; + let package_root = bin_dir.parent().ok_or_else(|| { + format!( + "failed to locate package root for executable {}", + exe_path.display() + ) + })?; + let cargo_dylint = bin_dir.join(cargo_dylint_binary_name()); + let library_dir = package_root.join("lib"); + let library_path = find_bundled_library(&library_dir)?; + + ensure_exists(&cargo_dylint, "bundled cargo-dylint executable")?; + ensure_exists( + &library_dir, + "bundled argument-comment lint library directory", + )?; + + let args: Vec = env::args_os().skip(1).collect(); + let mut command = Command::new(&cargo_dylint); + command.arg("dylint"); + command.arg("--lib-path").arg(&library_path); + if !has_library_selection(&args) { + command.arg("--all"); + } + command.args(&args); + set_default_env(&mut command); + + let status = command + .status() + .map_err(|err| format!("failed to execute {}: {err}", cargo_dylint.display()))?; + Ok(exit_code_from_status(status.code())) +} + +fn has_library_selection(args: &[OsString]) -> bool { + let mut expect_value = false; + for arg in args { + if expect_value { + return true; + } + + match arg.to_string_lossy().as_ref() { + "--" => break, + "--lib" | "--lib-path" => { + expect_value = true; + } + "--lib=" | "--lib-path=" => return true, + value if value.starts_with("--lib=") || value.starts_with("--lib-path=") => { + return true; + } + _ => {} + } + } + + false +} + +fn set_default_env(command: &mut Command) { + if let Some(flags) = env::var_os("DYLINT_RUSTFLAGS") { + let mut flags = flags.to_string_lossy().to_string(); + append_flag_if_missing(&mut flags, "-D uncommented-anonymous-literal-argument"); + append_flag_if_missing(&mut flags, "-A unknown_lints"); + command.env("DYLINT_RUSTFLAGS", flags); + } else { + command.env( + "DYLINT_RUSTFLAGS", + "-D uncommented-anonymous-literal-argument -A unknown_lints", + ); + } + + if env::var_os("CARGO_INCREMENTAL").is_none() { + command.env("CARGO_INCREMENTAL", "0"); + } +} + +fn append_flag_if_missing(flags: &mut String, flag: &str) { + if flags.contains(flag) { + return; + } + + if !flags.is_empty() { + flags.push(' '); + } + flags.push_str(flag); +} + +fn cargo_dylint_binary_name() -> &'static str { + if cfg!(windows) { + "cargo-dylint.exe" + } else { + "cargo-dylint" + } +} + +fn ensure_exists(path: &Path, label: &str) -> Result<(), String> { + if path.exists() { + Ok(()) + } else { + Err(format!("{label} not found at {}", path.display())) + } +} + +fn find_bundled_library(library_dir: &Path) -> Result { + let entries = fs::read_dir(library_dir).map_err(|err| { + format!( + "failed to read bundled library directory {}: {err}", + library_dir.display() + ) + })?; + let mut candidates = entries + .filter_map(Result::ok) + .map(|entry| entry.path()) + .filter(|path| path.is_file()) + .filter(|path| { + path.file_name() + .map(|name| name.to_string_lossy().contains('@')) + .unwrap_or(false) + }); + + let Some(first) = candidates.next() else { + return Err(format!( + "no packaged Dylint library found in {}", + library_dir.display() + )); + }; + if candidates.next().is_some() { + return Err(format!( + "expected exactly one packaged Dylint library in {}", + library_dir.display() + )); + } + + Ok(first) +} + +fn exit_code_from_status(code: Option) -> ExitCode { + code.and_then(|value| u8::try_from(value).ok()) + .map_or_else(|| ExitCode::from(1), ExitCode::from) +}