Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ Options:
--fineGrainedHashExternalReposFile <FINE_GRAINED_HASH_EXTERNAL_REPOS_FILE>

--labelOnlyModuleExtensions <LABEL_ONLY_MODULE_EXTENSIONS>
Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories use each other only through labels, never by resolving a toolchain from one another. A fine-grained target in one of their repositories then does not change whenever any other repository of the extension does
Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories are used only through labels, never by resolving a toolchain from one of them. The module that owns such an extension then does not stand in for its repositories, so a lockfile bump impacts only the targets that depend on the bumped repository
--isolatedDependencyRoot <ISOLATED_DEPENDENCY_ROOT>
Assert that consumers outside this root-isolated usage reach it through ordinary target inputs or BUILD loads. The named root import must cover every generated repository. Keep the whole usage on each such consumer
--useCquery[=<USE_CQUERY>]
Expand Down Expand Up @@ -814,7 +814,7 @@ Options:
--fineGrainedHashExternalReposFile <FINE_GRAINED_HASH_EXTERNAL_REPOS_FILE>

--labelOnlyModuleExtensions <LABEL_ONLY_MODULE_EXTENSIONS>
Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories use each other only through labels, never by resolving a toolchain from one another. A fine-grained target in one of their repositories then does not change whenever any other repository of the extension does
Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories are used only through labels, never by resolving a toolchain from one of them. The module that owns such an extension then does not stand in for its repositories, so a lockfile bump impacts only the targets that depend on the bumped repository
--isolatedDependencyRoot <ISOLATED_DEPENDENCY_ROOT>
Assert that consumers outside this root-isolated usage reach it through ordinary target inputs or BUILD loads. The named root import must cover every generated repository. Keep the whole usage on each such consumer
--useCquery[=<USE_CQUERY>]
Expand Down Expand Up @@ -914,13 +914,16 @@ A few inputs are normalized or extended on the way in:
in the workspace, a `file://%workspace%` registry URL) is hashed relative to the workspace, so
identical checkouts at different paths hash the same.
In query mode the module that owns a module extension stands in for every repository the
extension generates, because a toolchain resolved from one of them is not a query input. A
fine-grained target inside one of those repositories that uses a rule from the owning module
therefore changes whenever any repository of the extension does. Listing the extension in
`--labelOnlyModuleExtensions` (for example `@rules_rs//rs:extensions.bzl%crate`) asserts that
its repositories use each other only through labels, so such a target reaches the owning module
without them. Do not list an extension whose targets resolve a toolchain the same extension
generates: that change would be missed. An extension whose name another `.bzl` file of the same
extension generates, because a toolchain resolved from one of them is not a query input. Every
target that uses a rule of the owning module, every target of a module with a `bazel_dep` path
to it, and every fine-grained target inside one of those repositories therefore changes whenever
any repository of the extension does: a `go.mod` bump through gazelle's `go_deps` or a
`pnpm-lock.yaml` bump through rules_js's `npm` impacts most of the workspace. Listing the
extension in `--labelOnlyModuleExtensions` (for example
`@gazelle//:extensions.bzl%go_deps` or `@rules_rs//rs:extensions.bzl%crate`) asserts that its
repositories are used only through labels, which the query reports, so the owning module no
longer stands in for them. Do not list an extension that generates a toolchain, such as
`rules_go`'s `go_sdk`: a change to it would be missed. An extension whose name another `.bzl` file of the same
module also exports is reported and ignored, since Bazel tells their repositories apart only by
a numeric suffix.
`--isolatedDependencyRoot=@crates` asserts that every outside consumer of this
Expand Down
122 changes: 58 additions & 64 deletions src/bazel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,8 @@ pub struct BazelOptions {
pub fine_grained_external_repos: BTreeSet<String>,
/// Module extensions, as `bazel mod show_extension` names them
/// (`@rules_rs//rs:extensions.bzl%crate`), whose repositories use each other only through
/// labels: no target in one of them resolves a toolchain from another. A target inside such an
/// extension's repository reaches the module that owns the extension without the extension's
/// other repositories (see `extension_view_node`).
/// labels: no target resolves a toolchain from one of them. The module that owns such an
/// extension then does not stand in for its repositories (see `lower_repositories`).
pub label_only_module_extensions: BTreeSet<String>,
/// A root-isolated extension import whose external consumers are discoverable through
/// ordinary target inputs and BUILD loads.
Expand Down Expand Up @@ -723,7 +722,7 @@ impl BazelOptions {
};
let mut scope = IsolatedRepoScope::default();
let label_only = LabelOnlyRepositories {
sibling_extensions: self
extensions: self
.label_only_extension_prefixes(&canonical_to_apparent, &extension_repos.generated),
owner_exemptions: if let Some(root) = &self.isolated_dependency_root {
if let Some(extension) = failed.extensions.first() {
Expand Down Expand Up @@ -1924,7 +1923,8 @@ impl IsolatedRepoScope {

#[derive(Default)]
struct LabelOnlyRepositories {
sibling_extensions: BTreeSet<String>,
/// Usage prefixes of `label_only_module_extensions`; their owners get no stand-in edges.
extensions: BTreeSet<String>,
owner_exemptions: BTreeSet<String>,
}

Expand Down Expand Up @@ -2434,15 +2434,15 @@ fn lower_repositories(
// and each imported repository of an extension depends on the siblings nobody imports (the
// `@bd_crates` hub reaches its `bd_crates__*` spokes). Spokes themselves stay leaves, so no
// cycle forms.
// For an extension the user lists in `label_only_module_extensions`, a target inside one of
// its repositories reaches the owning module through a view without the extension's own
// repositories (`extension_view_node`): the user asserts that it uses its siblings only
// through labels, which the query reports. Otherwise every fine-grained spoke would change
// whenever any other spoke does. The view is opt-in because a toolchain resolved from a
// sibling is invisible to a query, however it is registered.
// An extension the user lists in `label_only_module_extensions` gets no owner edges: the user
// asserts that its repositories are reached only through labels, which the query reports,
// never through a resolved toolchain. Otherwise every target of the owning module, of every
// module with a `bazel_dep` path to it, and every fine-grained spoke would change whenever
// any repository of the extension does (issue #603). It is opt-in because a toolchain
// resolved from one of them is invisible to a query, however it is registered.
// An explicitly label-only imported repository needs no owner fallback. Its own node,
// the hub-to-spoke inputs and the owner's edges to other generators remain.
let mut module_extension_repos = BTreeMap::<String, BTreeMap<String, BTreeSet<String>>>::new();
let mut module_extension_repos = BTreeMap::<String, BTreeSet<String>>::new();
let mut extension_spokes = BTreeMap::<String, BTreeSet<String>>::new();
if let Some(imported) = extension_edges {
for repository in &repositories {
Expand All @@ -2453,15 +2453,15 @@ fn lower_repositories(
if let Some((module_repo, _definition_hash)) =
extension_repo_owner(extension, &module_definitions).filter(
|(module_repo, _definition_hash)| {
!module_repo.is_empty() && !label_only.owner_exemptions.contains(canonical)
!module_repo.is_empty()
&& !label_only.owner_exemptions.contains(canonical)
&& !label_only.extensions.contains(extension)
},
)
{
module_extension_repos
.entry(module_repo.clone())
.or_default()
.entry(extension.to_owned())
.or_default()
.extend(node_names(canonical));
}
if !imported.contains(canonical) {
Expand Down Expand Up @@ -2490,8 +2490,7 @@ fn lower_repositories(
.filter_map(|dependency| module_to_canonical.get(dependency))
.flat_map(|canonical| node_names(canonical))
.collect::<BTreeSet<_>>();
let extensions = module_extension_repos.get(canonical);
for repos in extensions.into_iter().flat_map(BTreeMap::values) {
if let Some(repos) = module_extension_repos.get(canonical) {
dependencies.extend(repos.iter().cloned());
}
let spokes = extension
Expand All @@ -2509,17 +2508,12 @@ fn lower_repositories(
if names.is_empty() {
continue;
}
let views = extensions
.into_iter()
.flatten()
.filter(|(extension, _)| label_only.sibling_extensions.contains(*extension))
.collect::<Vec<_>>();
let files = module_files
.remove(&index)
.unwrap_or_else(|| prepare_repository(&mut repository, &workspace));
let rule = repository_rule(repository, files, owner_definition_hash);
// All apparent names and views have the same definition. Move it into the final output.
let mut rules = std::iter::repeat_n(rule, names.len() * (1 + views.len()));
// All apparent names have the same definition. Move it into the final output.
let mut rules = std::iter::repeat_n(rule, names.len());
for apparent in names {
// Imported hubs of one extension share its ordered hidden siblings. Pass
// the same allocation to the hash traversal when no owner-specific edge
Expand All @@ -2543,26 +2537,11 @@ fn lower_repositories(
let mut target = repository_target(rules.next().unwrap(), &apparent, inputs);
transform(&mut target, shared);
targets.push(target);
for (extension, repos) in &views {
let mut view = repository_target(
rules.next().unwrap(),
&extension_view_node(&apparent, extension),
&dependencies.difference(repos).cloned().collect(),
);
transform(&mut view, None);
targets.push(view);
}
}
}
targets
}

/// The node a target in a repository of `extension` reaches its owning module `node` through:
/// the module without the extension's own repositories (see `lower_repositories`).
pub(crate) fn extension_view_node(node: &str, extension: &str) -> String {
format!("{node}%{extension}")
}

fn parse_bazel_version(text: &str) -> Option<BazelVersion> {
let version = text.lines().find_map(|line| {
line.strip_prefix("Build label: ")
Expand Down Expand Up @@ -4557,7 +4536,7 @@ mod tests {
}

#[test]
fn apparent_names_and_views_share_one_read_of_the_repository_inputs() {
fn apparent_names_share_one_read_of_the_repository_inputs() {
let workspace = tempfile::tempdir().unwrap();
let patch = workspace.path().join("dep.patch");
fs::write(&patch, "first").unwrap();
Expand Down Expand Up @@ -4585,10 +4564,7 @@ mod tests {
&parse_repo_mapping(r#"{"before":"before+","a":"dep+","b":"dep+"}"#),
&BTreeMap::new(),
Some(&BTreeSet::new()),
&LabelOnlyRepositories {
sibling_extensions: BTreeSet::from(["dep++ext".into()]),
..Default::default()
},
&LabelOnlyRepositories::default(),
workspace.path(),
&mut |target, _| {
// The module definitions have been hashed, but dep's targets have not been emitted.
Expand Down Expand Up @@ -4619,9 +4595,7 @@ mod tests {
[
("//external:before", None),
("//external:a", first),
("//external:a%dep++ext", first),
("//external:b", first),
("//external:b%dep++ext", first),
("//external:dep++ext+spoke", Some(second.as_str())),
]
);
Expand Down Expand Up @@ -5367,17 +5341,11 @@ exit 1
&mapping,
&edges,
Some(&imported),
&LabelOnlyRepositories {
sibling_extensions: BTreeSet::from([
"rules_go++go_sdk".to_owned(),
"rules_rs++crate".to_owned(),
]),
..Default::default()
},
&LabelOnlyRepositories::default(),
Path::new("."),
&mut materialize_repository_inputs,
);
assert_eq!(query.len(), 10);
assert_eq!(query.len(), 8);
// The owning module reaches everything its extensions generate, after its module deps.
assert_eq!(
inputs_of(&query, "//external:rules_go"),
Expand All @@ -5395,16 +5363,6 @@ exit 1
"//external:rules_rs++crate+bd_crates__anyhow-1.0.104",
]
);
// Its view from inside one extension leaves out that extension's repositories, and keeps
// those of its other usages, an isolated usage of the same extension included.
assert_eq!(
inputs_of(&query, "//external:rules_go%rules_go++go_sdk"),
["//external:rules_rs"]
);
assert_eq!(
inputs_of(&query, "//external:rules_rs%rules_rs++crate"),
["//external:rules_rs++_crate+++isolated_crates+crate_repo"]
);
// An imported repository reaches the siblings of its extension nobody imports.
assert_eq!(
inputs_of(&query, "//external:bd_crates"),
Expand All @@ -5423,6 +5381,42 @@ exit 1
.is_empty());
assert!(inputs_of(&query, "//external:+local+root_ext_repo").is_empty());

// A label-only extension's repositories leave its owner's node, while those of its other
// usages stay, an isolated usage of the same extension included (issue #603). The
// hub-to-spoke edges remain.
let label_only = lower_repositories(
repositories(),
&mapping,
&edges,
Some(&imported),
&LabelOnlyRepositories {
extensions: BTreeSet::from([
"rules_go++go_sdk".to_owned(),
"rules_rs++crate".to_owned(),
]),
..Default::default()
},
Path::new("."),
&mut materialize_repository_inputs,
);
assert_eq!(label_only.len(), 8);
assert_eq!(
inputs_of(&label_only, "//external:rules_go"),
["//external:rules_rs"]
);
assert_eq!(
inputs_of(&label_only, "//external:rules_rs"),
["//external:rules_rs++_crate+++isolated_crates+crate_repo"]
);
assert_eq!(
inputs_of(&label_only, "//external:bd_crates"),
["//external:rules_rs++crate+bd_crates__anyhow-1.0.104"]
);
assert_eq!(
inputs_of(&label_only, "//external:rules_go++go_sdk+go_toolchains"),
["//external:rules_go++go_sdk+go_default_sdk"]
);

let cquery = lower_repositories(
repositories(),
&mapping,
Expand Down
45 changes: 1 addition & 44 deletions src/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1072,12 +1072,6 @@ impl HashContext<'_> {
builder.direct(self.always_affected_seed);
}

// Only a listed extension has views, so a run without the flag never resolves the
// rule's repository, which can cost a `bazel mod` call.
let extension = (!self.options.bazel.use_cquery
&& !self.options.bazel.label_only_module_extensions.is_empty())
.then(|| self.rule_extension(&rule.rule.name))
.flatten();
let mut inputs: Vec<Cow<'_, str>> = if rule.input_positions.is_some() {
rule.rule
.rule_input
Expand Down Expand Up @@ -1122,29 +1116,10 @@ impl HashContext<'_> {
}
}
for (position, encoded_input) in inputs.into_iter().enumerate() {
let mut input_slot = rule
let input_slot = rule
.input_positions
.as_ref()
.and_then(|ids| ids.get(position).copied().flatten());
// A target in an extension's repository reaches its owning module through the view
// without that extension's repositories, when there is one (query mode only).
let encoded_input = match extension.as_deref().and_then(|extension| {
let node = encoded_input.strip_prefix("//external:")?;
let view = format!(
"//external:{}",
crate::bazel::extension_view_node(node, extension)
);
self.rules
.get(&view)
.is_some_and(|slot| slot.rule.is_some())
.then_some(view)
}) {
Some(view) => {
input_slot = None;
Cow::Owned(view)
}
None => encoded_input,
};
builder.direct(encoded_input.as_bytes());
let input_label = target_label(&encoded_input);
match self.resolve_input(&encoded_input, input_label, input_slot, path) {
Expand Down Expand Up @@ -1266,24 +1241,6 @@ impl HashContext<'_> {
.and_then(|slot| slot.resolving(Cow::Owned(fallback.clone())))
}

/// The usage prefix of the module extension that generated the repository `label` is in
/// (`rules_rs++crate` for `@@rules_rs++crate+crates__anyhow//:anyhow`), if any.
fn rule_extension(&self, label: &str) -> Option<String> {
let label = target_label(label);
let repo = label.strip_prefix('@')?.split_once("//")?.0;
let canonical = match repo.strip_prefix('@') {
Some("") => return None,
Some(canonical) => canonical,
None => self
.source_hasher
.external_resolver
.repo_mapping()
.get(repo)?
.as_str(),
};
crate::bazel::extension_usage_prefix(canonical).map(str::to_owned)
}

fn rule_bzl_seed(&mut self, rule: &Rule) -> Result<Option<DigestBytes>> {
if rule.instantiation_stack.is_empty() {
return Ok(None);
Expand Down
8 changes: 4 additions & 4 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,10 @@ struct HashingArgs {
#[arg(
long = "labelOnlyModuleExtensions",
value_delimiter = ',',
help = "Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories use each \
other only through labels, never by resolving a toolchain from one another. A \
fine-grained target in one of their repositories then does not change whenever any \
other repository of the extension does"
help = "Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories are \
used only through labels, never by resolving a toolchain from one of them. The \
module that owns such an extension then does not stand in for its repositories, so \
a lockfile bump impacts only the targets that depend on the bumped repository"
)]
label_only_module_extensions: Vec<String>,

Expand Down
Loading
Loading