Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: crates_universe Intra workspace Dependency Resolution #3308

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
43 changes: 43 additions & 0 deletions crate_universe/extensions.bzl
Original file line number Diff line number Diff line change
@@ -492,6 +492,11 @@ def _collect_render_config(module, repository):
if default_alias_rule_bzl:
config_kwargs["default_alias_rule"] = str(default_alias_rule_bzl)

if "intra_workspace_dependencies_workspace_toml" in config_kwargs:
default_intra_workspace_dependencies_workspace_toml = config_kwargs.pop("intra_workspace_dependencies_workspace_toml")
if default_intra_workspace_dependencies_workspace_toml:
config_kwargs["intra_workspace_dependencies_workspace_toml"] = str(default_intra_workspace_dependencies_workspace_toml)

if "default_alias_rule_name" in config_kwargs:
config_kwargs["default_alias_rule"] = "{}:{}".format(
config_kwargs.get("default_alias_rule", ""),
@@ -1338,6 +1343,44 @@ can be found below where the supported keys for each template can be found in th
doc = "Whether to generate `target_compatible_with` annotations on the generated BUILD files. This catches a `target_triple` being targeted that isn't declared in `supported_platform_triples`.",
default = True,
),
"intra_workspace_dependencies_workspace_toml": attr.label(
doc = """
Whether to generate intra (cargo) workspace dependency aliases.
If None (the default), you are responsible for maintaining all of the dependencies
between your crates. If Some, the `crates_repository` will attempt to generate
intra-workspace dependencies. The value should be the label of the workspace `Cargo.toml` file.
This relies on you to have created the library targets for each intra-workspace dependency.
These targets should have the following properties:
- They should be in the same bazel module as the `cargo_lockfile`
- They should have the same name as the library name in your cargo toml.
- They should have the same relative bazel path as their cargo path. For example
if your workspace has the following layout
```text
[workspace]/
WORKSPACE.bazel
BUILD.bazel
Cargo.toml
Cargo.Bazel.lock
foo/
Cargo.toml
BUILD
src/
main.rs
bar/
Cargo.toml
BUILD
src/
lib.rs
```
Where `foo/Cargo.toml` contains
```toml
[dependencies]
bar.path = "../bar"
```
Then `bar/BUILD` must define a `rust_library` target names `bar`
""",
default = None,
),
"platforms_template": attr.string(
doc = "The base template to use for platform names. See [platforms documentation](https://docs.bazel.build/versions/main/platforms.html). The available format keys are [`{triple}`].",
default = "@rules_rust//rust/platform:{triple}",
36 changes: 35 additions & 1 deletion crate_universe/private/generate_utils.bzl
Original file line number Diff line number Diff line change
@@ -100,7 +100,8 @@ def render_config(
platforms_template = "@rules_rust//rust/platform:{triple}",
regen_command = None,
vendor_mode = None,
generate_rules_license_metadata = False):
generate_rules_license_metadata = False,
intra_workspace_dependencies_workspace_toml = None):
"""Various settings used to configure rendered outputs

The template parameters each support a select number of format keys. A description of each key
@@ -143,6 +144,38 @@ def render_config(
regen_command (str, optional): An optional command to demonstrate how generated files should be regenerated.
vendor_mode (str, optional): An optional configuration for rendirng content to be rendered into repositories.
generate_rules_license_metadata (bool, optional): Whether to generate rules license metedata
intra_workspace_dependencies_workspace_toml (Label, optional): Whether to generate intra (cargo) workspace dependency aliases.
If None (the default), you are responsible for maintaining all of the dependencies
between your crates. If Some, the `crates_repository` will generate intra-workspace dependencies. The value
should be the label of the workspace `Cargo.toml` file. This relies on you to have created the library targets
for each intra-workspace dependency. These targets should have the following properties:
- They should be in the same bazel module as the `cargo_lockfile`
- They should have the same name as the library name in your cargo toml.
- They should have the same relative bazel path as their cargo path. For example
if your workspace has the following layout
```text
[workspace]/
WORKSPACE.bazel
BUILD.bazel
Cargo.toml
Cargo.Bazel.lock
foo/
Cargo.toml
BUILD
src/
main.rs
bar/
Cargo.toml
BUILD
src/
lib.rs
```
Where `foo/Cargo.toml` contains
```toml
[dependencies]
bar.path = "../bar"
```
Then `bar/BUILD` must define a `rust_library` target names `bar`

Returns:
string: A json encoded struct to match the Rust `config::RenderConfig` struct
@@ -161,6 +194,7 @@ def render_config(
platforms_template = platforms_template,
regen_command = regen_command,
vendor_mode = vendor_mode,
intra_workspace_dependencies_workspace_toml = intra_workspace_dependencies_workspace_toml,
))

def _crate_id(name, version):
5 changes: 5 additions & 0 deletions crate_universe/src/config.rs
Original file line number Diff line number Diff line change
@@ -107,6 +107,10 @@ pub(crate) struct RenderConfig {
/// Whether to generate cargo_toml_env_vars targets.
/// This is expected to always be true except for bootstrapping.
pub(crate) generate_cargo_toml_env_vars: bool,

/// The label of the cargo lockfile, used to create build aliases for intra-workspace dependencies.
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) intra_workspace_dependencies_workspace_toml: Option<Label>,
}

// Default is manually implemented so that the default values match the default
@@ -129,6 +133,7 @@ impl Default for RenderConfig {
regen_command: String::default(),
vendor_mode: Option::default(),
generate_rules_license_metadata: default_generate_rules_license_metadata(),
intra_workspace_dependencies_workspace_toml: Option::default(),
}
}
}
114 changes: 91 additions & 23 deletions crate_universe/src/metadata/dependency.rs
Original file line number Diff line number Diff line change
@@ -45,6 +45,7 @@ impl DependencySet {
node: &Node,
metadata: &CargoMetadata,
resolver_data: &TreeResolverMetadata,
track_intra_workspace_dependencies: bool,
) -> Self {
// Build a dep tree mapping that's easily indexable via `cargo_metadata::PackageId`
let dep_tree: BTreeMap<CrateId, Select<BTreeSet<CrateId>>> = resolver_data
@@ -69,8 +70,9 @@ impl DependencySet {
let (dev, normal) = node
.deps
.iter()
// Do not track workspace members as dependencies. Users are expected to maintain those connections
.filter(|dep| !is_workspace_member(dep, metadata))
.filter(|dep| {
filter_workspace_members(track_intra_workspace_dependencies, dep, metadata)
})
.filter(|dep| is_lib_package(&metadata[&dep.pkg]))
.filter(|dep| is_normal_dependency(dep) || is_dev_dependency(dep))
.partition(|dep| is_dev_dependency(dep));
@@ -97,8 +99,9 @@ impl DependencySet {
let (dev, normal) = node
.deps
.iter()
// Do not track workspace members as dependencies. Users are expected to maintain those connections
.filter(|dep| !is_workspace_member(dep, metadata))
.filter(|dep| {
filter_workspace_members(track_intra_workspace_dependencies, dep, metadata)
})
.filter(|dep| is_proc_macro_package(&metadata[&dep.pkg]))
.filter(|dep| is_normal_dependency(dep) || is_dev_dependency(dep))
.partition(|dep| is_dev_dependency(dep));
@@ -128,7 +131,9 @@ impl DependencySet {
.deps
.iter()
// Do not track workspace members as dependencies. Users are expected to maintain those connections
.filter(|dep| !is_workspace_member(dep, metadata))
.filter(|dep| {
filter_workspace_members(track_intra_workspace_dependencies, dep, metadata)
})
.filter(|dep| is_build_dependency(dep))
.filter(|dep| !is_dev_dependency(dep))
.partition(|dep| is_proc_macro_package(&metadata[&dep.pkg]));
@@ -323,6 +328,14 @@ fn is_normal_dependency(node_dep: &NodeDep) -> bool {
.any(|k| matches!(k.kind, cargo_metadata::DependencyKind::Normal))
}

fn filter_workspace_members(
track_intra_workspace_dependencies: bool,
node_dep: &NodeDep,
metadata: &CargoMetadata,
) -> bool {
track_intra_workspace_dependencies || !is_workspace_member(node_dep, metadata)
}

fn is_workspace_member(node_dep: &NodeDep, metadata: &CargoMetadata) -> bool {
metadata
.workspace_members
@@ -540,7 +553,7 @@ mod test {

let node = find_metadata_node("example-proc-macro-dep", &metadata);
let dependencies =
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default());
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default(), false);

let normal_deps: Vec<_> = dependencies
.normal_deps
@@ -565,7 +578,7 @@ mod test {

let node = find_metadata_node("surrealdb-core", &metadata);
let dependencies =
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default());
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default(), false);

let bindings = dependencies.normal_deps.items();

@@ -588,8 +601,12 @@ mod test {

let openssl_node = find_metadata_node("openssl", &metadata);

let dependencies =
DependencySet::new_for_node(openssl_node, &metadata, &TreeResolverMetadata::default());
let dependencies = DependencySet::new_for_node(
openssl_node,
&metadata,
&TreeResolverMetadata::default(),
false,
);

let normal_sys_crate =
dependencies
@@ -622,8 +639,12 @@ mod test {
let metadata = metadata::build_scripts();

let libssh2 = find_metadata_node("libssh2-sys", &metadata);
let libssh2_depset =
DependencySet::new_for_node(libssh2, &metadata, &TreeResolverMetadata::default());
let libssh2_depset = DependencySet::new_for_node(
libssh2,
&metadata,
&TreeResolverMetadata::default(),
false,
);

// Collect build dependencies into a set
let build_deps: BTreeSet<String> = libssh2_depset
@@ -672,8 +693,12 @@ mod test {
let metadata = metadata::alias();

let aliases_node = find_metadata_node("aliases", &metadata);
let dependencies =
DependencySet::new_for_node(aliases_node, &metadata, &TreeResolverMetadata::default());
let dependencies = DependencySet::new_for_node(
aliases_node,
&metadata,
&TreeResolverMetadata::default(),
false,
);

let aliases: Vec<Dependency> = dependencies
.normal_deps
@@ -700,7 +725,7 @@ mod test {

let node = find_metadata_node("crate-types", &metadata);
let dependencies =
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default());
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default(), false);

let rlib_deps: Vec<Dependency> = dependencies
.normal_deps
@@ -731,7 +756,7 @@ mod test {

let node = find_metadata_node("cpufeatures", &metadata);
let dependencies =
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default());
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default(), false);

let libc_cfgs: BTreeSet<Option<String>> = dependencies
.normal_deps
@@ -757,7 +782,7 @@ mod test {

let node = find_metadata_node("multi-kind-proc-macro-dep", &metadata);
let dependencies =
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default());
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default(), false);

let lib_deps: Vec<_> = dependencies
.proc_macro_deps
@@ -782,7 +807,7 @@ mod test {

let node = find_metadata_node("clap", &metadata);
let dependencies =
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default());
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default(), false);

assert!(!dependencies
.normal_deps
@@ -798,7 +823,7 @@ mod test {

let serde_with = find_metadata_node("serde_with", &metadata);
let serde_with_depset =
DependencySet::new_for_node(serde_with, &metadata, &TreeResolverMetadata::new());
DependencySet::new_for_node(serde_with, &metadata, &TreeResolverMetadata::new(), false);
assert!(!serde_with_depset
.normal_deps
.items()
@@ -826,7 +851,7 @@ mod test {
)]);

let clap = find_metadata_node("clap", &metadata);
let clap_depset = DependencySet::new_for_node(clap, &metadata, &resolver_data);
let clap_depset = DependencySet::new_for_node(clap, &metadata, &resolver_data, false);
assert_eq!(
clap_depset
.normal_deps
@@ -840,7 +865,7 @@ mod test {

let notify = find_metadata_node("notify", &metadata);
let notify_depset =
DependencySet::new_for_node(notify, &metadata, &TreeResolverMetadata::default());
DependencySet::new_for_node(notify, &metadata, &TreeResolverMetadata::default(), false);

// mio is not present in the common list of dependencies
assert!(!notify_depset
@@ -874,7 +899,7 @@ mod test {

let node = find_metadata_node("gherkin", &metadata);
let dependencies =
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default());
DependencySet::new_for_node(node, &metadata, &TreeResolverMetadata::default(), false);

assert!(!dependencies
.normal_deps
@@ -907,7 +932,7 @@ mod test {
)]);

let p256 = find_metadata_node("p256", &metadata);
let p256_depset = DependencySet::new_for_node(p256, &metadata, &resolver_data);
let p256_depset = DependencySet::new_for_node(p256, &metadata, &resolver_data, false);
assert_eq!(
p256_depset
.normal_deps
@@ -965,7 +990,8 @@ mod test {
)]);

let tokio_node = find_metadata_node("tokio", &metadata);
let tokio_depset = DependencySet::new_for_node(tokio_node, &metadata, &tree_metadata);
let tokio_depset =
DependencySet::new_for_node(tokio_node, &metadata, &tree_metadata, false);
assert_eq!(
tokio_depset
.normal_deps
@@ -983,4 +1009,46 @@ mod test {
"`mio` is a platform specific dependency and therefore should not be identified under the common configuration."
);
}

#[test]
fn intra_workspace_deps() {
let metadata = metadata::workspace_path();

let child_b_node = find_metadata_node("child_b", &metadata);
let child_b_depset_with_intra_workspace = DependencySet::new_for_node(
child_b_node,
&metadata,
&TreeResolverMetadata::default(),
true,
);
assert_eq!(
child_b_depset_with_intra_workspace
.normal_deps
.items()
.iter()
.filter(|(configuration, dep)| {
configuration.is_none() && dep.target_name == "child_a"
})
.count(),
1,
);

let child_b_depset_without_intra_workspace = DependencySet::new_for_node(
child_b_node,
&metadata,
&TreeResolverMetadata::default(),
false,
);
assert_eq!(
child_b_depset_without_intra_workspace
.normal_deps
.items()
.iter()
.filter(|(configuration, dep)| {
configuration.is_none() && dep.target_name == "child_a"
})
.count(),
0,
)
}
}
Loading