mirror of
https://github.com/openai/codex.git
synced 2026-11-08 20:40:33 +00:00
build: migrate argument-comment-lint to a native Bazel aspect
This commit is contained in:
@@ -1,19 +1,32 @@
|
||||
load("@rules_rust//rust:defs.bzl", "rust_binary")
|
||||
load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library")
|
||||
|
||||
filegroup(
|
||||
name = "runtime-files",
|
||||
exports_files(["lint_aspect.bzl"])
|
||||
|
||||
rust_library(
|
||||
name = "argument-comment-lint-lib",
|
||||
crate_name = "argument_comment_lint",
|
||||
crate_features = ["bazel_native"],
|
||||
crate_root = "src/lib.rs",
|
||||
srcs = [
|
||||
"argument-comment-lint",
|
||||
"run-prebuilt-linter.py",
|
||||
"wrapper_common.py",
|
||||
"src/comment_parser.rs",
|
||||
"src/lib.rs",
|
||||
],
|
||||
edition = "2024",
|
||||
deps = ["@argument_comment_lint_crates//:clippy_utils"],
|
||||
tags = ["manual"],
|
||||
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"],
|
||||
name = "argument-comment-lint-driver",
|
||||
crate_name = "argument_comment_lint_driver",
|
||||
crate_root = "driver.rs",
|
||||
srcs = ["driver.rs"],
|
||||
edition = "2024",
|
||||
deps = [
|
||||
":argument-comment-lint-lib",
|
||||
"@zlib//:z",
|
||||
],
|
||||
tags = ["manual"],
|
||||
visibility = ["//visibility:public"],
|
||||
)
|
||||
|
||||
@@ -8,6 +8,9 @@ publish = false
|
||||
[lib]
|
||||
crate-type = ["cdylib"]
|
||||
|
||||
[features]
|
||||
bazel_native = []
|
||||
|
||||
[dependencies]
|
||||
clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "20ce69b9a63bcd2756cd906fe0964d1e901e042a" }
|
||||
dylint_linting = "5.0.0"
|
||||
|
||||
@@ -85,11 +85,11 @@ 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` 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.
|
||||
targeted package runs such as `just argument-comment-lint -p codex-core`.
|
||||
Repo-wide runs now go through a native Bazel aspect that invokes a custom
|
||||
`rustc_driver` and reuses Bazel-managed Rust dependency metadata instead of
|
||||
spawning `cargo dylint` once per crate. The source-build path remains available
|
||||
in `run.py` for people iterating on the lint crate itself.
|
||||
|
||||
The Unix archive layout is:
|
||||
|
||||
@@ -128,16 +128,17 @@ If you are changing the lint crate itself, use the source-build wrapper:
|
||||
Run the lint against `codex-rs` from the repo root:
|
||||
|
||||
```bash
|
||||
just argument-comment-lint
|
||||
bazel build --config=argument-comment-lint -- //codex-rs/...
|
||||
./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
|
||||
`codex-rs` workspace with `--workspace --no-deps`.
|
||||
For non-`--fix` runs, both wrappers also default the underlying Cargo
|
||||
invocation to `--all-targets` unless you explicitly narrow the target set, so
|
||||
workspace and package lint runs both cover test-only call sites by default.
|
||||
If no package selection is provided, `just argument-comment-lint` now defaults
|
||||
to the Bazel aspect path over `//codex-rs/...`. The Python wrappers remain the
|
||||
package-scoped escape hatch and still default the underlying Cargo invocation
|
||||
to `--all-targets` unless you explicitly narrow the target set, so targeted
|
||||
wrapper runs cover test-only call sites by default.
|
||||
|
||||
Repo runs also promote `uncommented_anonymous_literal_argument` to an error by
|
||||
default:
|
||||
|
||||
@@ -1,148 +0,0 @@
|
||||
use std::env;
|
||||
use std::ffi::OsString;
|
||||
use std::hash::DefaultHasher;
|
||||
use std::hash::Hash;
|
||||
use std::hash::Hasher;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
use std::process::ExitCode;
|
||||
|
||||
const TOOLCHAIN_CHANNEL: &str = "nightly-2025-09-18";
|
||||
#[cfg(target_os = "linux")]
|
||||
const TOOLCHAIN_TRIPLE: &str = "x86_64-unknown-linux-gnu";
|
||||
#[cfg(all(target_os = "macos", target_arch = "aarch64"))]
|
||||
const TOOLCHAIN_TRIPLE: &str = "aarch64-apple-darwin";
|
||||
#[cfg(all(target_os = "macos", target_arch = "x86_64"))]
|
||||
const TOOLCHAIN_TRIPLE: &str = "x86_64-apple-darwin";
|
||||
#[cfg(target_os = "windows")]
|
||||
const TOOLCHAIN_TRIPLE: &str = "x86_64-pc-windows-msvc";
|
||||
|
||||
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() {
|
||||
command.env("CARGO_TARGET_DIR", shared_target_dir(&workspace_dir));
|
||||
}
|
||||
|
||||
command.env("RUSTUP_TOOLCHAIN", TOOLCHAIN_CHANNEL);
|
||||
command.env("RUSTUP_AUTO_INSTALL", "0");
|
||||
if env::var_os("RUSTUP_HOME").is_none()
|
||||
&& let Some(rustup_home) = infer_rustup_home()
|
||||
{
|
||||
command.env("RUSTUP_HOME", rustup_home);
|
||||
}
|
||||
|
||||
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 shared_target_dir(workspace_dir: &Path) -> PathBuf {
|
||||
let namespace = if let Some(run_id) = env::var_os("GITHUB_RUN_ID") {
|
||||
let mut namespace = run_id;
|
||||
if let Some(run_attempt) = env::var_os("GITHUB_RUN_ATTEMPT") {
|
||||
namespace.push("-");
|
||||
namespace.push(run_attempt);
|
||||
}
|
||||
namespace
|
||||
} else {
|
||||
let mut hasher = DefaultHasher::new();
|
||||
workspace_dir.hash(&mut hasher);
|
||||
OsString::from(format!("{:016x}", hasher.finish()))
|
||||
};
|
||||
|
||||
env::temp_dir()
|
||||
.join("argument-comment-lint")
|
||||
.join(namespace)
|
||||
.join(format!("{TOOLCHAIN_CHANNEL}-{TOOLCHAIN_TRIPLE}"))
|
||||
}
|
||||
|
||||
fn infer_rustup_home() -> Option<OsString> {
|
||||
if let Some(rustup_home) = env::var_os("RUSTUP_HOME") {
|
||||
return Some(rustup_home);
|
||||
}
|
||||
|
||||
if let Some(cargo_home) = env::var_os("CARGO_HOME")
|
||||
&& let Some(home_dir) = Path::new(&cargo_home).parent()
|
||||
{
|
||||
let rustup_home = home_dir.join(".rustup");
|
||||
if rustup_home.is_dir() {
|
||||
return Some(rustup_home.into_os_string());
|
||||
}
|
||||
}
|
||||
|
||||
for var in ["HOME", "USERPROFILE"] {
|
||||
if let Some(home) = env::var_os(var) {
|
||||
let rustup_home = Path::new(&home).join(".rustup");
|
||||
if rustup_home.is_dir() {
|
||||
return Some(rustup_home.into_os_string());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
@@ -0,0 +1,43 @@
|
||||
#![feature(rustc_private)]
|
||||
|
||||
extern crate rustc_driver;
|
||||
extern crate rustc_interface;
|
||||
|
||||
use std::env;
|
||||
use std::ffi::OsString;
|
||||
use std::path::Path;
|
||||
|
||||
fn main() {
|
||||
let mut callbacks = Callbacks;
|
||||
let args = rustc_args(env::args_os().skip(1).collect());
|
||||
rustc_driver::run_compiler(&args, &mut callbacks);
|
||||
}
|
||||
|
||||
struct Callbacks;
|
||||
|
||||
impl rustc_driver::Callbacks for Callbacks {
|
||||
fn config(&mut self, config: &mut rustc_interface::Config) {
|
||||
let previous = config.register_lints.take();
|
||||
config.register_lints = Some(Box::new(move |sess, lint_store| {
|
||||
if let Some(previous) = &previous {
|
||||
previous(sess, lint_store);
|
||||
}
|
||||
argument_comment_lint::register_lints(sess, lint_store);
|
||||
}));
|
||||
}
|
||||
}
|
||||
|
||||
fn rustc_args(args: Vec<OsString>) -> Vec<String> {
|
||||
let mut rustc_args: Vec<String> = args
|
||||
.into_iter()
|
||||
.map(|arg| arg.to_string_lossy().into_owned())
|
||||
.collect();
|
||||
if rustc_args.first().is_none_or(|arg| !is_rustc(arg)) {
|
||||
rustc_args.insert(0, "rustc".to_string());
|
||||
}
|
||||
rustc_args
|
||||
}
|
||||
|
||||
fn is_rustc(arg: &str) -> bool {
|
||||
Path::new(arg).file_stem().and_then(|stem| stem.to_str()) == Some("rustc")
|
||||
}
|
||||
@@ -0,0 +1,183 @@
|
||||
"""Bazel aspect for running argument-comment-lint on Rust targets."""
|
||||
|
||||
load("@rules_rust//rust:defs.bzl", "rust_common")
|
||||
load("@rules_rust//rust/private:rust.bzl", "RUSTC_ATTRS")
|
||||
load(
|
||||
"@rules_rust//rust/private:rustc.bzl",
|
||||
"collect_deps",
|
||||
"collect_inputs",
|
||||
"construct_arguments",
|
||||
)
|
||||
load(
|
||||
"@rules_rust//rust/private:utils.bzl",
|
||||
"determine_output_hash",
|
||||
"find_cc_toolchain",
|
||||
"find_toolchain",
|
||||
)
|
||||
|
||||
_STRICT_LINT_FLAGS = [
|
||||
"-Duncommented-anonymous-literal-argument",
|
||||
"-Aunknown-lints",
|
||||
]
|
||||
|
||||
def _find_rustc_driver_library_dir(toolchain):
|
||||
for file in toolchain.rustc_lib.to_list():
|
||||
if file.basename.startswith("librustc_driver-") or file.basename.startswith("rustc_driver-"):
|
||||
return file.dirname
|
||||
return None
|
||||
|
||||
def _set_driver_runtime_env(env, toolchain):
|
||||
library_dir = _find_rustc_driver_library_dir(toolchain)
|
||||
if not library_dir:
|
||||
return
|
||||
|
||||
path = library_dir
|
||||
if toolchain.target_os == "windows":
|
||||
previous = env.get("PATH")
|
||||
env["PATH"] = "{};{}".format(path, previous) if previous else path
|
||||
elif toolchain.target_os == "macos":
|
||||
previous = env.get("DYLD_LIBRARY_PATH")
|
||||
env["DYLD_LIBRARY_PATH"] = "{}:{}".format(path, previous) if previous else path
|
||||
else:
|
||||
previous = env.get("LD_LIBRARY_PATH")
|
||||
env["LD_LIBRARY_PATH"] = "{}:{}".format(path, previous) if previous else path
|
||||
|
||||
def _get_argument_comment_lint_ready_crate_info(target, aspect_ctx):
|
||||
if target.label.workspace_root.startswith("external"):
|
||||
return None
|
||||
|
||||
if aspect_ctx:
|
||||
ignore_tags = [
|
||||
"no_argument_comment_lint",
|
||||
"no-lint",
|
||||
"no_lint",
|
||||
"nolint",
|
||||
]
|
||||
for tag in aspect_ctx.rule.attr.tags:
|
||||
if tag.replace("-", "_").lower() in ignore_tags:
|
||||
return None
|
||||
|
||||
if rust_common.crate_info in target:
|
||||
return target[rust_common.crate_info]
|
||||
if rust_common.test_crate_info in target:
|
||||
return target[rust_common.test_crate_info].crate
|
||||
return None
|
||||
|
||||
def _rust_argument_comment_lint_aspect_impl(target, ctx):
|
||||
if OutputGroupInfo in target and hasattr(target[OutputGroupInfo], "argument_comment_lint_checks"):
|
||||
return []
|
||||
|
||||
crate_info = _get_argument_comment_lint_ready_crate_info(target, ctx)
|
||||
if not crate_info:
|
||||
return []
|
||||
|
||||
toolchain = find_toolchain(ctx)
|
||||
cc_toolchain, feature_configuration = find_cc_toolchain(ctx)
|
||||
|
||||
dep_info, build_info, _ = collect_deps(
|
||||
deps = crate_info.deps.to_list(),
|
||||
proc_macro_deps = crate_info.proc_macro_deps.to_list(),
|
||||
aliases = crate_info.aliases,
|
||||
)
|
||||
|
||||
compile_inputs, out_dir, build_env_files, build_flags_files, linkstamp_outs, ambiguous_libs = collect_inputs(
|
||||
ctx,
|
||||
ctx.rule.file,
|
||||
ctx.rule.files,
|
||||
depset([]),
|
||||
toolchain,
|
||||
cc_toolchain,
|
||||
feature_configuration,
|
||||
crate_info,
|
||||
dep_info,
|
||||
build_info,
|
||||
[],
|
||||
)
|
||||
|
||||
success_marker = ctx.actions.declare_file(
|
||||
ctx.label.name + ".argument_comment_lint.ok",
|
||||
sibling = crate_info.output,
|
||||
)
|
||||
|
||||
args, env = construct_arguments(
|
||||
ctx = ctx,
|
||||
attr = ctx.rule.attr,
|
||||
file = ctx.file,
|
||||
toolchain = toolchain,
|
||||
tool_path = ctx.executable._driver.path,
|
||||
cc_toolchain = cc_toolchain,
|
||||
feature_configuration = feature_configuration,
|
||||
crate_info = crate_info,
|
||||
dep_info = dep_info,
|
||||
linkstamp_outs = linkstamp_outs,
|
||||
ambiguous_libs = ambiguous_libs,
|
||||
output_hash = determine_output_hash(crate_info.root, ctx.label),
|
||||
rust_flags = [],
|
||||
out_dir = out_dir,
|
||||
build_env_files = build_env_files,
|
||||
build_flags_files = build_flags_files,
|
||||
emit = ["dep-info", "metadata"],
|
||||
skip_expanding_rustc_env = True,
|
||||
)
|
||||
|
||||
if crate_info.is_test:
|
||||
args.rustc_flags.add("--test")
|
||||
|
||||
args.process_wrapper_flags.add("--touch-file", success_marker)
|
||||
args.rustc_flags.add_all(_STRICT_LINT_FLAGS)
|
||||
_set_driver_runtime_env(env, toolchain)
|
||||
|
||||
driver_runfiles = ctx.attr._driver[DefaultInfo].default_runfiles.files
|
||||
action_inputs = depset(
|
||||
transitive = [
|
||||
compile_inputs,
|
||||
driver_runfiles,
|
||||
toolchain.rustc_lib,
|
||||
],
|
||||
)
|
||||
|
||||
ctx.actions.run(
|
||||
executable = ctx.executable._process_wrapper,
|
||||
inputs = action_inputs,
|
||||
outputs = [success_marker],
|
||||
env = env,
|
||||
tools = [ctx.executable._driver],
|
||||
execution_requirements = {
|
||||
"no-sandbox": "1",
|
||||
},
|
||||
arguments = args.all,
|
||||
mnemonic = "ArgumentCommentLint",
|
||||
progress_message = "ArgumentCommentLint %{label}",
|
||||
toolchain = "@rules_rust//rust:toolchain_type",
|
||||
)
|
||||
|
||||
return [OutputGroupInfo(argument_comment_lint_checks = depset([success_marker]))]
|
||||
|
||||
rust_argument_comment_lint_aspect = aspect(
|
||||
implementation = _rust_argument_comment_lint_aspect_impl,
|
||||
fragments = ["cpp"],
|
||||
attrs = {
|
||||
"_driver": attr.label(
|
||||
default = Label("//tools/argument-comment-lint:argument-comment-lint-driver"),
|
||||
executable = True,
|
||||
cfg = "exec",
|
||||
),
|
||||
} | RUSTC_ATTRS,
|
||||
toolchains = [
|
||||
str(Label("@rules_rust//rust:toolchain_type")),
|
||||
config_common.toolchain_type("@bazel_tools//tools/cpp:toolchain_type", mandatory = False),
|
||||
],
|
||||
required_providers = [
|
||||
[rust_common.crate_info],
|
||||
[rust_common.test_crate_info],
|
||||
],
|
||||
doc = """\
|
||||
Runs argument-comment-lint on Rust targets using Bazel's Rust dependency graph.
|
||||
|
||||
Example:
|
||||
|
||||
```output
|
||||
$ bazel build --config=argument-comment-lint //codex-rs/...
|
||||
```
|
||||
""",
|
||||
)
|
||||
@@ -31,6 +31,7 @@ use rustc_span::Span;
|
||||
use crate::comment_parser::parse_argument_comment;
|
||||
use crate::comment_parser::parse_argument_comment_prefix;
|
||||
|
||||
#[cfg(not(feature = "bazel_native"))]
|
||||
dylint_linting::dylint_library!();
|
||||
|
||||
#[unsafe(no_mangle)]
|
||||
|
||||
Reference in New Issue
Block a user