ci: run argument-comment-lint through bazel package tests

This commit is contained in:
Michael Bolin
2026-03-27 22:37:26 -07:00
parent e02fd6e1d3
commit ffae2cc58d
9 changed files with 273 additions and 14 deletions

View File

@@ -144,6 +144,10 @@ jobs:
labels: codex-windows-x64
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
- uses: ./.github/actions/setup-bazel-ci
with:
target: ${{ runner.os }}
install-test-prereqs: true
- name: Install Linux sandbox build dependencies
if: ${{ runner.os == 'Linux' }}
shell: bash
@@ -154,19 +158,9 @@ jobs:
with:
toolchain: nightly-2025-09-18
components: llvm-tools-preview, rustc-dev, rust-src
- uses: facebook/install-dotslash@1e4e7b3e07eaca387acb98f1d4720e0bee8dbb6a # v2
- name: Run argument comment lint on codex-rs
if: ${{ runner.os == 'macOS' }}
- name: Run argument comment lint on codex-rs via Bazel
shell: bash
run: python3 ./tools/argument-comment-lint/run-prebuilt-linter.py
- name: Run argument comment lint on codex-rs (default targets only)
if: ${{ runner.os == 'Linux' }}
shell: bash
run: python3 ./tools/argument-comment-lint/run-prebuilt-linter.py -- --lib --bins
- name: Run argument comment lint on codex-rs
if: ${{ runner.os == 'Windows' }}
shell: bash
run: python ./tools/argument-comment-lint/run-prebuilt-linter.py
run: bazel test --build_tests_only --test_tag_filters=argument-comment-lint //codex-rs/...
# --- CI to validate on different os/targets --------------------------------
lint_build:

View File

@@ -2,3 +2,17 @@ exports_files([
"clippy.toml",
"node-version.txt",
])
filegroup(
name = "workspace-files",
srcs = glob(
[
"*",
".cargo/**",
],
exclude = [
"BUILD.bazel",
],
),
visibility = ["//visibility:public"],
)

View File

@@ -57,6 +57,9 @@ def _workspace_root_test_impl(ctx):
)
runfiles = ctx.runfiles(files = [test_bin, workspace_root_marker]).merge(ctx.attr.test_bin[DefaultInfo].default_runfiles)
for data_dep in ctx.attr.data:
runfiles = runfiles.merge(ctx.runfiles(files = data_dep[DefaultInfo].files.to_list()))
runfiles = runfiles.merge(data_dep[DefaultInfo].default_runfiles)
return [
DefaultInfo(
@@ -73,6 +76,9 @@ workspace_root_test = rule(
implementation = _workspace_root_test_impl,
test = True,
attrs = {
"data": attr.label_list(
allow_files = True,
),
"env": attr.string_dict(),
"test_bin": attr.label(
cfg = "target",
@@ -98,6 +104,48 @@ workspace_root_test = rule(
},
)
def _workspace_dep_packages(package_name):
package_data = DEP_DATA.get(package_name, {})
dependency_labels = []
for key in ["deps", "build_deps", "dev_deps"]:
dependency_labels += package_data.get(key, [])
for key in ["deps_by_platform", "build_deps_by_platform", "dev_deps_by_platform"]:
for labels in package_data.get(key, {}).values():
dependency_labels += labels
workspace_packages = []
for dependency_label in dependency_labels:
if dependency_label.startswith("//codex-rs/"):
workspace_packages.append(Label(dependency_label).package)
return workspace_packages
def _argument_comment_lint_data(package_name):
closure = {package_name: True}
frontier = [package_name]
for _ in range(len(DEP_DATA)):
next_frontier = []
for current_package in frontier:
for dependency_package in _workspace_dep_packages(current_package):
if dependency_package not in closure:
closure[dependency_package] = True
next_frontier.append(dependency_package)
if not next_frontier:
break
frontier = next_frontier
return [
"//codex-rs:workspace-files",
"//tools/argument-comment-lint:runtime-files",
] + [
"//{}:package-files".format(closure_package)
for closure_package in sorted(closure.keys())
]
def codex_rust_crate(
name,
crate_name,
@@ -157,10 +205,29 @@ def codex_rust_crate(
"INSTA_SNAPSHOT_PATH": "src",
}
native.filegroup(
name = "package-files",
srcs = native.glob(
["**"],
exclude = [
"**/BUILD.bazel",
"BUILD.bazel",
"target/**",
],
allow_empty = True,
),
visibility = ["//visibility:public"],
)
rustc_env = {
"BAZEL_PACKAGE": native.package_name(),
} | rustc_env
manifest_relpath = native.package_name()
if manifest_relpath.startswith("codex-rs/"):
manifest_relpath = manifest_relpath[len("codex-rs/"):]
manifest_path = manifest_relpath + "/Cargo.toml"
binaries = DEP_DATA.get(native.package_name())["binaries"]
lib_srcs = crate_srcs or native.glob(["src/**/*.rs"], exclude = binaries.values(), allow_empty = True)
@@ -275,3 +342,17 @@ def codex_rust_crate(
env = cargo_env,
tags = test_tags,
)
workspace_root_test(
name = name + "-argument-comment-lint",
data = _argument_comment_lint_data(native.package_name()),
env = {
"ARGUMENT_COMMENT_LINT_MANIFEST": manifest_path,
},
tags = [
"argument-comment-lint",
"no-sandbox",
],
test_bin = "//tools/argument-comment-lint:argument-comment-lint-bazel-runner",
workspace_root_marker = "//codex-rs/utils/cargo-bin:repo_root.marker",
)

View File

@@ -72,6 +72,10 @@ bazel-test:
bazel-clippy:
bazel build --config=clippy -- //codex-rs/... -//codex-rs/v8-poc:all
[no-cd]
bazel-argument-comment-lint:
bazel test --build_tests_only --test_tag_filters=argument-comment-lint //codex-rs/...
bazel-remote-test:
bazel test //... --config=remote --platforms=//:rbe --keep_going

View File

@@ -0,0 +1,19 @@
load("@rules_rust//rust:defs.bzl", "rust_binary")
filegroup(
name = "runtime-files",
srcs = [
"argument-comment-lint",
"run-prebuilt-linter.py",
"wrapper_common.py",
],
visibility = ["//visibility:public"],
)
rust_binary(
name = "argument-comment-lint-bazel-runner",
crate_name = "argument_comment_lint_bazel_runner",
crate_root = "bazel_runner.rs",
srcs = ["bazel_runner.rs"],
visibility = ["//visibility:public"],
)

View File

@@ -85,7 +85,9 @@ rustup toolchain install nightly-2025-09-18 \
The checked-in DotSlash file lives at `tools/argument-comment-lint/argument-comment-lint`.
`run-prebuilt-linter.py` resolves that file via `dotslash` and is the path used by
`just clippy`, `just argument-comment-lint`, and the Rust CI job. The
`just clippy` and `just argument-comment-lint`. Bazel-backed CI now drives the
same wrapper through package-owned test targets tagged `argument-comment-lint`.
The
source-build path remains available in `run.py` for people
iterating on the lint crate itself.
@@ -128,6 +130,7 @@ Run the lint against `codex-rs` from the repo root:
```bash
./tools/argument-comment-lint/run-prebuilt-linter.py -p codex-core
just argument-comment-lint -p codex-core
bazel test --build_tests_only --test_tag_filters=argument-comment-lint //codex-rs/...
```
If no package selection is provided, `run-prebuilt-linter.py` defaults to checking the

View File

@@ -0,0 +1,124 @@
use std::env;
use std::path::Path;
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<ExitCode, String> {
let manifest = env::var("ARGUMENT_COMMENT_LINT_MANIFEST")
.map_err(|_| "ARGUMENT_COMMENT_LINT_MANIFEST must be set".to_string())?;
let workspace_dir = env::current_dir().map_err(|err| format!("failed to get cwd: {err}"))?;
let wrapper = find_repo_root(&workspace_dir)?
.join("tools")
.join("argument-comment-lint")
.join("run-prebuilt-linter.py");
let python = if cfg!(windows) { "python" } else { "python3" };
let mut command = Command::new(python);
command.arg(&wrapper);
command.arg("--manifest-path");
command.arg(&manifest);
// Keep Linux on the narrower target set for now to match the current CI
// rollout, while macOS and Windows continue to exercise all targets.
if cfg!(target_os = "linux") {
command.args(["--", "--lib", "--bins"]);
}
if env::var_os("CARGO_TARGET_DIR").is_none()
&& let Some(test_tmpdir) = env::var_os("TEST_TMPDIR")
{
let sanitized_manifest = manifest.replace('/', "_").replace('\\', "_");
let target_dir =
Path::new(&test_tmpdir).join(format!("argument-comment-lint-{sanitized_manifest}"));
command.env("CARGO_TARGET_DIR", target_dir);
}
if let Some(cargo) = cargo_binary() {
let cargo_dir = cargo
.parent()
.ok_or_else(|| format!("failed to resolve cargo directory from {}", cargo.display()))?;
let existing_path = env::var_os("PATH").unwrap_or_default();
let mut paths = vec![cargo_dir.to_path_buf()];
paths.extend(env::split_paths(&existing_path));
let joined_paths =
env::join_paths(paths).map_err(|err| format!("failed to build PATH: {err}"))?;
command.env("PATH", joined_paths);
command.env("CARGO", cargo);
}
let status = command
.status()
.map_err(|err| format!("failed to execute {python}: {err}"))?;
Ok(status
.code()
.and_then(|code| u8::try_from(code).ok())
.map_or_else(|| ExitCode::from(1), ExitCode::from))
}
fn find_repo_root(cwd: &Path) -> Result<&Path, String> {
if cwd
.join("tools")
.join("argument-comment-lint")
.join("run-prebuilt-linter.py")
.is_file()
{
return Ok(cwd);
}
let Some(parent) = cwd.parent() else {
return Err(format!(
"argument-comment wrapper not found relative to {}",
cwd.display()
));
};
if parent
.join("tools")
.join("argument-comment-lint")
.join("run-prebuilt-linter.py")
.is_file()
{
return Ok(parent);
}
Err(format!(
"argument-comment wrapper not found relative to {}",
cwd.display()
))
}
fn cargo_binary() -> Option<std::path::PathBuf> {
for var in ["HOME", "USERPROFILE"] {
if let Some(home) = env::var_os(var) {
let cargo_bin = Path::new(&home).join(".cargo").join("bin");
let candidate = cargo_bin.join(if cfg!(windows) { "cargo.exe" } else { "cargo" });
if candidate.is_file() {
return Some(candidate);
}
}
}
if let Ok(cwd) = env::current_dir() {
for ancestor in cwd.ancestors() {
let candidate = ancestor.join(".cargo").join("bin").join(if cfg!(windows) {
"cargo.exe"
} else {
"cargo"
});
if candidate.is_file() {
return Some(candidate);
}
}
}
None
}

View File

@@ -83,6 +83,26 @@ class WrapperCommonTest(unittest.TestCase):
],
)
def test_explicit_package_manifest_does_not_force_workspace(self) -> None:
parsed = wrapper_common.parse_wrapper_args(
[
"--manifest-path",
"/tmp/custom/Cargo.toml",
]
)
final_args = wrapper_common.build_final_args(parsed, Path("/repo/codex-rs/Cargo.toml"))
self.assertEqual(
final_args,
[
"--no-deps",
"--manifest-path",
"/tmp/custom/Cargo.toml",
"--",
"--all-targets",
],
)
if __name__ == "__main__":
unittest.main()

View File

@@ -107,7 +107,7 @@ def build_final_args(parsed: ParsedWrapperArgs, manifest_path: Path) -> list[str
if not parsed.has_manifest_path:
final_args.extend(["--manifest-path", str(manifest_path)])
if not parsed.has_package_selection:
if not parsed.has_package_selection and not parsed.has_manifest_path:
final_args.append("--workspace")
if not parsed.has_no_deps:
final_args.append("--no-deps")