From b3ed7d535266267b04b9ae1b67e4d922f3f50f4e Mon Sep 17 00:00:00 2001 From: Martin Kuntz Jacobsen Date: Mon, 9 Nov 2026 17:23:06 +0100 Subject: [PATCH 1/2] modules: test that every non-dev bazel_dep is loaded by a public .bzl (part of #10067) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Xut2hwESun1HL2eSVd1d5B --- modules/BUILD.bazel | 16 ++++ modules/loaders_test.rs | 162 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+) create mode 100644 modules/loaders_test.rs diff --git a/modules/BUILD.bazel b/modules/BUILD.bazel index 7228fbf96..2fa908856 100644 --- a/modules/BUILD.bazel +++ b/modules/BUILD.bazel @@ -37,6 +37,21 @@ rust_test( ], ) +# keep +rust_test( + name = "loaders_test", + srcs = ["loaders_test.rs"], + data = ["//:MODULE.bazel"], + tags = [ + "external", + "no-remote-exec", + ], + deps = [ + "//libs/module_bazel:dx_module_bazel", + "//tools/testing:dx_testing", + ], +) + # keep rust_test( name = "oci_pins_test", @@ -137,6 +152,7 @@ test_suite( tests = [ ":consumer_deps_test", ":families_test", + ":loaders_test", ":python_budget_test", ":versions_contract", ], diff --git a/modules/loaders_test.rs b/modules/loaders_test.rs new file mode 100644 index 000000000..009430cd4 --- /dev/null +++ b/modules/loaders_test.rs @@ -0,0 +1,162 @@ +//! Every non-dev bazel_dep of the root module is loaded by a public .bzl or listed as root-only. + +use std::collections::BTreeMap; +use std::path::Path; + +use dx_module_bazel::{ModuleFile, Value}; + +/// Non-dev deps that no public .bzl loads; each is used by a root extension or a BUILD file. +const ROOT_ONLY: &[&str] = &[ + "bazel_features", + "bazel_linux_packages", + "elfutils", + "hermetic_launcher", + "libxml2", + "lua", + "rules_android", + "rules_apple", + "rules_batch", + "rules_jvm_external", + "rules_rust_prost", + "tar.bzl", + "windows_support", + "yaml.bzl", + "zlib", +]; + +const SKIPPED_DIRS: &[&str] = &[ + "tests", + "test", + "testdata", + "fixtures", + "examples", + "e2e", + "testing", + "node_modules", +]; + +const SKIPPED_ROOTS: &[&str] = &["tools", "docs", "bcr_test_module"]; + +fn public_bzl(root: &Path, dir: &Path, found: &mut Vec) { + let entries = + std::fs::read_dir(dir).unwrap_or_else(|error| panic!("read_dir {dir:?}: {error}")); + for entry in entries.filter_map(Result::ok) { + let path = entry.path(); + let name = entry.file_name().to_string_lossy().into_owned(); + if name.starts_with('.') || name.starts_with("bazel-") { + continue; + } + if path.is_dir() { + let top = dir == root; + if SKIPPED_DIRS.contains(&name.as_str()) + || (top && SKIPPED_ROOTS.contains(&name.as_str())) + || path.join("MODULE.bazel").exists() + { + continue; + } + public_bzl(root, &path, found); + } else if name.ends_with(".bzl") { + if let Ok(text) = std::fs::read_to_string(&path) { + found.push(text); + } + } + } +} + +fn mentions(text: &str, dep: &str) -> bool { + let needle = format!("@{dep}"); + text.match_indices(&needle).any(|(start, found)| { + matches!( + text[start + found.len()..].chars().next(), + Some('/' | '"' | ':' | '+') + ) + }) +} + +fn non_dev_deps(source: &str) -> Vec { + let module = ModuleFile::parse(source).unwrap_or_else(|error| panic!("{error}")); + module + .calls_named("bazel_dep") + .filter(|call| !matches!(call.arg("dev_dependency"), Some(Value::Ident(value)) if value == "True")) + .filter_map(|call| call.str("name").map(str::to_owned)) + .collect() +} + +/// Every way the loaders of `deps` disagree with `root_only`; empty when they agree. +fn violations(deps: &[String], bzl: &[String], root_only: &[&str]) -> Vec { + let loaders: BTreeMap<&str, usize> = deps + .iter() + .map(|dep| { + let count = bzl.iter().filter(|text| mentions(text, dep)).count(); + (dep.as_str(), count) + }) + .collect(); + let mut problems = Vec::new(); + for (dep, count) in &loaders { + let listed = root_only.contains(dep); + if *count == 0 && !listed { + problems.push(format!( + "bazel_dep {dep:?} is loaded by no public .bzl; make it dev_dependency or move it" + )); + } + if *count > 0 && listed { + problems.push(format!( + "bazel_dep {dep:?} is loaded by {count} public .bzl files; drop it from ROOT_ONLY" + )); + } + } + for dep in root_only { + if !loaders.contains_key(dep) { + problems.push(format!("ROOT_ONLY {dep:?} has no non-dev bazel_dep")); + } + } + problems +} + +#[test] +fn every_non_dev_bazel_dep_is_loaded_by_a_public_bzl() { + let root = dx_testing::rules_dx_source(); + let mut bzl = Vec::new(); + public_bzl(&root, &root, &mut bzl); + assert!( + bzl.len() > 100, + "found only {} public .bzl files", + bzl.len() + ); + let deps = non_dev_deps(&dx_testing::read_runfiles("MODULE.bazel")); + let problems = violations(&deps, &bzl, ROOT_ONLY); + assert!(problems.is_empty(), "{}", problems.join("\n")); +} + +#[test] +fn an_unloaded_dep_fails() { + let deps = vec!["demo".to_owned(), "used".to_owned()]; + let bzl = vec!["load(\"@used//:defs.bzl\", \"x\")".to_owned()]; + let problems = violations(&deps, &bzl, &[]); + assert_eq!(problems.len(), 1, "{problems:?}"); + assert!(problems[0].contains("\"demo\""), "{problems:?}"); +} + +#[test] +fn a_listed_dep_that_is_loaded_fails() { + let deps = vec!["used".to_owned()]; + let bzl = vec!["Label(\"@@used+//:x\")".to_owned()]; + let problems = violations(&deps, &bzl, &["used"]); + assert_eq!(problems.len(), 1, "{problems:?}"); +} + +#[test] +fn a_prefix_of_another_repo_name_is_not_a_load() { + assert!(!mentions( + "load(\"@rules_python_extra//:x.bzl\")", + "rules_python" + )); + assert!(mentions("load(\"@rules_python//:x.bzl\")", "rules_python")); + assert!(mentions("\"@rules_python\"", "rules_python")); +} + +#[test] +fn a_stale_root_only_entry_fails() { + let problems = violations(&[], &[], &["gone"]); + assert_eq!(problems.len(), 1, "{problems:?}"); +} From ed119029fe96d8a3907fcc85b59a4b7037c5bb4f Mon Sep 17 00:00:00 2001 From: Martin Kuntz Jacobsen Date: Mon, 9 Nov 2026 17:26:14 +0100 Subject: [PATCH 2/2] modules: pins.bzl loads every pinned root dep, ROOT_ONLY removed (part of #10067) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Xut2hwESun1HL2eSVd1d5B --- modules/BUILD.bazel | 2 ++ modules/loaders_test.rs | 73 +++++------------------------------------ modules/pins.bzl | 19 +++++++++++ 3 files changed, 30 insertions(+), 64 deletions(-) create mode 100644 modules/pins.bzl diff --git a/modules/BUILD.bazel b/modules/BUILD.bazel index 2fa908856..19d372a91 100644 --- a/modules/BUILD.bazel +++ b/modules/BUILD.bazel @@ -165,6 +165,7 @@ exports_files([ "rust.bzl", "python.bzl", "families.json", + "pins.bzl", "python_budget.json", "versions.bzl", ]) @@ -175,6 +176,7 @@ real_source_target( "BUILD.bazel", "consumer_deps.bzl", "cuda.bzl", + "pins.bzl", "python.bzl", "rust.bzl", "ruby.bzl", diff --git a/modules/loaders_test.rs b/modules/loaders_test.rs index 009430cd4..5f1a93538 100644 --- a/modules/loaders_test.rs +++ b/modules/loaders_test.rs @@ -1,29 +1,9 @@ -//! Every non-dev bazel_dep of the root module is loaded by a public .bzl or listed as root-only. +//! Every non-dev bazel_dep of the root module is loaded by a public .bzl. -use std::collections::BTreeMap; use std::path::Path; use dx_module_bazel::{ModuleFile, Value}; -/// Non-dev deps that no public .bzl loads; each is used by a root extension or a BUILD file. -const ROOT_ONLY: &[&str] = &[ - "bazel_features", - "bazel_linux_packages", - "elfutils", - "hermetic_launcher", - "libxml2", - "lua", - "rules_android", - "rules_apple", - "rules_batch", - "rules_jvm_external", - "rules_rust_prost", - "tar.bzl", - "windows_support", - "yaml.bzl", - "zlib", -]; - const SKIPPED_DIRS: &[&str] = &[ "tests", "test", @@ -82,35 +62,14 @@ fn non_dev_deps(source: &str) -> Vec { .collect() } -/// Every way the loaders of `deps` disagree with `root_only`; empty when they agree. -fn violations(deps: &[String], bzl: &[String], root_only: &[&str]) -> Vec { - let loaders: BTreeMap<&str, usize> = deps - .iter() +/// The deps in `deps` that no text in `bzl` mentions. +fn unloaded(deps: &[String], bzl: &[String]) -> Vec { + deps.iter() + .filter(|dep| !bzl.iter().any(|text| mentions(text, dep))) .map(|dep| { - let count = bzl.iter().filter(|text| mentions(text, dep)).count(); - (dep.as_str(), count) + format!("bazel_dep {dep:?} is loaded by no public .bzl; make it dev_dependency or move it") }) - .collect(); - let mut problems = Vec::new(); - for (dep, count) in &loaders { - let listed = root_only.contains(dep); - if *count == 0 && !listed { - problems.push(format!( - "bazel_dep {dep:?} is loaded by no public .bzl; make it dev_dependency or move it" - )); - } - if *count > 0 && listed { - problems.push(format!( - "bazel_dep {dep:?} is loaded by {count} public .bzl files; drop it from ROOT_ONLY" - )); - } - } - for dep in root_only { - if !loaders.contains_key(dep) { - problems.push(format!("ROOT_ONLY {dep:?} has no non-dev bazel_dep")); - } - } - problems + .collect() } #[test] @@ -124,7 +83,7 @@ fn every_non_dev_bazel_dep_is_loaded_by_a_public_bzl() { bzl.len() ); let deps = non_dev_deps(&dx_testing::read_runfiles("MODULE.bazel")); - let problems = violations(&deps, &bzl, ROOT_ONLY); + let problems = unloaded(&deps, &bzl); assert!(problems.is_empty(), "{}", problems.join("\n")); } @@ -132,19 +91,11 @@ fn every_non_dev_bazel_dep_is_loaded_by_a_public_bzl() { fn an_unloaded_dep_fails() { let deps = vec!["demo".to_owned(), "used".to_owned()]; let bzl = vec!["load(\"@used//:defs.bzl\", \"x\")".to_owned()]; - let problems = violations(&deps, &bzl, &[]); + let problems = unloaded(&deps, &bzl); assert_eq!(problems.len(), 1, "{problems:?}"); assert!(problems[0].contains("\"demo\""), "{problems:?}"); } -#[test] -fn a_listed_dep_that_is_loaded_fails() { - let deps = vec!["used".to_owned()]; - let bzl = vec!["Label(\"@@used+//:x\")".to_owned()]; - let problems = violations(&deps, &bzl, &["used"]); - assert_eq!(problems.len(), 1, "{problems:?}"); -} - #[test] fn a_prefix_of_another_repo_name_is_not_a_load() { assert!(!mentions( @@ -154,9 +105,3 @@ fn a_prefix_of_another_repo_name_is_not_a_load() { assert!(mentions("load(\"@rules_python//:x.bzl\")", "rules_python")); assert!(mentions("\"@rules_python\"", "rules_python")); } - -#[test] -fn a_stale_root_only_entry_fails() { - let problems = violations(&[], &[], &["gone"]); - assert_eq!(problems.len(), 1, "{problems:?}"); -} diff --git a/modules/pins.bzl b/modules/pins.bzl new file mode 100644 index 000000000..ed7e935bb --- /dev/null +++ b/modules/pins.bzl @@ -0,0 +1,19 @@ +"""Root module deps that rules_dx pins for consumers, as labels of each repo.""" + +PINNED_REPOS = { + "bazel_features": Label("@bazel_features//:all"), + "bazel_linux_packages": Label("@bazel_linux_packages//:all"), + "elfutils": Label("@elfutils//:all"), + "hermetic_launcher": Label("@hermetic_launcher//:all"), + "libxml2": Label("@libxml2//:all"), + "lua": Label("@lua//:all"), + "rules_android": Label("@rules_android//:all"), + "rules_apple": Label("@rules_apple//:all"), + "rules_batch": Label("@rules_batch//:all"), + "rules_jvm_external": Label("@rules_jvm_external//:all"), + "rules_rust_prost": Label("@rules_rust_prost//:all"), + "tar.bzl": Label("@tar.bzl//:all"), + "windows_support": Label("@windows_support//:all"), + "yaml.bzl": Label("@yaml.bzl//:all"), + "zlib": Label("@zlib//:all"), +}