diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index 2636213af9..9fd27cacf3 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -81,6 +81,7 @@ jobs: --use-node-test-env \ -- \ test \ + --test_tag_filters=-argument-comment-lint \ --test_verbose_timeout_warnings \ --build_metadata=COMMIT_SHA=${GITHUB_SHA} \ -- \ diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index f993bccbe7..689def00ea 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -138,6 +138,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 @@ -148,21 +152,22 @@ 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 - # Linux still uses the default-targets-only form for now, but PRs run the - # released linter on all three platforms so wrapper regressions surface pre-merge. - - 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: | + if [[ "${RUNNER_OS}" == "Windows" ]]; then + unset BAZEL_OUTPUT_USER_ROOT + fi + + ./.github/scripts/run-bazel-ci.sh \ + --print-failed-test-logs \ + -- \ + test \ + --build_tests_only \ + --test_tag_filters=argument-comment-lint \ + --build_metadata=COMMIT_SHA=${GITHUB_SHA} \ + -- \ + //codex-rs/... # --- Gatherer job that you mark as the ONLY required status ----------------- results: diff --git a/codex-rs/BUILD.bazel b/codex-rs/BUILD.bazel index b6711d9b81..66c7ebcb61 100644 --- a/codex-rs/BUILD.bazel +++ b/codex-rs/BUILD.bazel @@ -2,3 +2,17 @@ exports_files([ "clippy.toml", "node-version.txt", ]) + +filegroup( + name = "workspace-files", + srcs = glob( + [ + "*", + ".cargo/**", + ], + exclude = [ + "BUILD.bazel", + ], + ), + visibility = ["//visibility:public"], +) diff --git a/defs.bzl b/defs.bzl index 02cbbc6295..d31b51aa2d 100644 --- a/defs.bzl +++ b/defs.bzl @@ -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) @@ -278,3 +345,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", + ) diff --git a/justfile b/justfile index 39a65df46d..5c3d2f9481 100644 --- a/justfile +++ b/justfile @@ -67,13 +67,17 @@ bazel-lock-check: ./scripts/check-module-bazel-lock.sh bazel-test: - bazel test //... --keep_going + bazel test --test_tag_filters=-argument-comment-lint //... --keep_going 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 + bazel test --test_tag_filters=-argument-comment-lint //... --config=remote --platforms=//:rbe --keep_going build-for-release: bazel build //codex-rs/cli:release_binaries --config=remote diff --git a/tools/argument-comment-lint/BUILD.bazel b/tools/argument-comment-lint/BUILD.bazel new file mode 100644 index 0000000000..8bc0475dd5 --- /dev/null +++ b/tools/argument-comment-lint/BUILD.bazel @@ -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"], +) diff --git a/tools/argument-comment-lint/README.md b/tools/argument-comment-lint/README.md index 92b0739601..bb453681a1 100644 --- a/tools/argument-comment-lint/README.md +++ b/tools/argument-comment-lint/README.md @@ -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 diff --git a/tools/argument-comment-lint/bazel_runner.rs b/tools/argument-comment-lint/bazel_runner.rs new file mode 100644 index 0000000000..0b77e483ce --- /dev/null +++ b/tools/argument-comment-lint/bazel_runner.rs @@ -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 { + 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 { + 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 +} diff --git a/tools/argument-comment-lint/test_wrapper_common.py b/tools/argument-comment-lint/test_wrapper_common.py index a3a57dfe34..3da6da604b 100644 --- a/tools/argument-comment-lint/test_wrapper_common.py +++ b/tools/argument-comment-lint/test_wrapper_common.py @@ -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() diff --git a/tools/argument-comment-lint/wrapper_common.py b/tools/argument-comment-lint/wrapper_common.py index 3202c22381..d898f5d93d 100644 --- a/tools/argument-comment-lint/wrapper_common.py +++ b/tools/argument-comment-lint/wrapper_common.py @@ -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")