Skip to content

Commit

Permalink
[crate_universe] add an annotation to disable pipelining (#1733)
Browse files Browse the repository at this point in the history
The recent support for pipelined compilation (#1275) is great. There are some situations, however, where we need to disable pipelining for specific crates, e.g. #1584. This PR adds a crate annotation option to disable pipelining for rust_library targets generated by cargo_bazel. The alternatives would be patching the crate with the annotations system or disabling pipelining globally.
  • Loading branch information
Calsign authored Jan 26, 2023
1 parent f651cd1 commit ad01d1b
Show file tree
Hide file tree
Showing 16 changed files with 684 additions and 581 deletions.
3 changes: 3 additions & 0 deletions crate_universe/private/crate.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ def _annotation(
data_glob = None,
deps = None,
gen_binaries = [],
disable_pipelining = False,
gen_build_script = None,
patch_args = None,
patch_tool = None,
Expand Down Expand Up @@ -118,6 +119,7 @@ def _annotation(
deps (list, optional): A list of labels to add to a crate's `rust_library::deps` attribute.
gen_binaries (list or bool, optional): As a list, the subset of the crate's bins that should get `rust_binary`
targets produced. Or `True` to generate all, `False` to generate none.
disable_pipelining (bool, optional): If True, disables pipelining for library targets for this crate.
gen_build_script (bool, optional): An authorative flag to determine whether or not to produce
`cargo_build_script` targets for the current crate.
patch_args (list, optional): The `patch_args` attribute of a Bazel repository rule. See
Expand Down Expand Up @@ -164,6 +166,7 @@ def _annotation(
data_glob = data_glob,
deps = deps,
gen_binaries = gen_binaries,
disable_pipelining = disable_pipelining,
gen_build_script = gen_build_script,
patch_args = patch_args,
patch_tool = patch_tool,
Expand Down
4 changes: 4 additions & 0 deletions crate_universe/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ pub struct CrateAnnotations {
/// [compile_data](https://bazelbuild.github.io/rules_rust/defs.html#rust_library-compile_data) attribute.
pub compile_data_glob: Option<BTreeSet<String>>,

/// If true, disables pipelining for library targets generated for this crate.
pub disable_pipelining: bool,

/// Additional data to pass to the target's
/// [rustc_env](https://bazelbuild.github.io/rules_rust/defs.html#rust_library-rustc_env) attribute.
pub rustc_env: Option<BTreeMap<String, String>>,
Expand Down Expand Up @@ -278,6 +281,7 @@ impl Add for CrateAnnotations {
crate_features: joined_extra_member!(self.crate_features, rhs.crate_features, BTreeSet::new, BTreeSet::extend),
data: joined_extra_member!(self.data, rhs.data, BTreeSet::new, BTreeSet::extend),
data_glob: joined_extra_member!(self.data_glob, rhs.data_glob, BTreeSet::new, BTreeSet::extend),
disable_pipelining: self.disable_pipelining || rhs.disable_pipelining,
compile_data: joined_extra_member!(self.compile_data, rhs.compile_data, BTreeSet::new, BTreeSet::extend),
compile_data_glob: joined_extra_member!(self.compile_data_glob, rhs.compile_data_glob, BTreeSet::new, BTreeSet::extend),
rustc_env: joined_extra_member!(self.rustc_env, rhs.rustc_env, BTreeMap::new, BTreeMap::extend),
Expand Down
10 changes: 10 additions & 0 deletions crate_universe/src/context/crate_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,10 @@ pub struct CrateContext {
/// Additional text to add to the generated BUILD file.
#[serde(skip_serializing_if = "Option::is_none")]
pub additive_build_file_content: Option<String>,

/// If true, disables pipelining for library targets generated for this crate
#[serde(skip_serializing_if = "std::ops::Not::not")]
pub disable_pipelining: bool,
}

impl CrateContext {
Expand Down Expand Up @@ -394,6 +398,7 @@ impl CrateContext {
build_script_attrs,
license,
additive_build_file_content: None,
disable_pipelining: false,
}
.with_overrides(extras)
}
Expand Down Expand Up @@ -446,6 +451,11 @@ impl CrateContext {
self.common_attrs.data_glob.extend(extra.clone());
}

// Disable pipelining
if crate_extra.disable_pipelining {
self.disable_pipelining = true;
}

// Rustc flags
if let Some(extra) = &crate_extra.rustc_flags {
self.common_attrs.rustc_flags.append(&mut extra.clone());
Expand Down
2 changes: 1 addition & 1 deletion crate_universe/src/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ mod test {

assert_eq!(
digest,
Digest("712442b3d89756257cf2739fa4ab58c2154d1bda3fb2c4c3647c613403351694".to_owned())
Digest("0485e1ac3d5a868679b2ee4b59443eec3f8b54e1f4824f7c251b20031542f64c".to_owned())
);
}

Expand Down
26 changes: 26 additions & 0 deletions crate_universe/src/rendering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ impl Renderer {
.make_aliases(krate, false, false)
.remap_configurations(platforms),
common: self.make_common_attrs(platforms, krate, target)?,
disable_pipelining: krate.disable_pipelining,
})
}

Expand Down Expand Up @@ -771,6 +772,31 @@ mod test {
assert!(build_file_content.contains("\"crate-name=mock_crate\""));
}

#[test]
fn test_disable_pipelining() {
let mut context = Context::default();
let crate_id = CrateId::new("mock_crate".to_owned(), "0.1.0".to_owned());
context.crates.insert(
crate_id.clone(),
CrateContext {
name: crate_id.name,
version: crate_id.version,
targets: BTreeSet::from([Rule::Library(mock_target_attributes())]),
disable_pipelining: true,
..CrateContext::default()
},
);

let renderer = Renderer::new(mock_render_config());
let output = renderer.render(&context).unwrap();

let build_file_content = output
.get(&PathBuf::from("BUILD.mock_crate-0.1.0.bazel"))
.unwrap();

assert!(build_file_content.contains("disable_pipelining = True"));
}

#[test]
fn render_cargo_build_script() {
let mut context = Context::default();
Expand Down
2 changes: 2 additions & 0 deletions crate_universe/src/utils/starlark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ pub struct RustLibrary {
pub aliases: SelectDict<WithOriginalConfigurations<String>>,
#[serde(flatten)]
pub common: CommonAttrs,
#[serde(skip_serializing_if = "std::ops::Not::not")]
pub disable_pipelining: bool,
}

#[derive(Serialize)]
Expand Down
5 changes: 3 additions & 2 deletions docs/crate_universe.md
Original file line number Diff line number Diff line change
Expand Up @@ -602,8 +602,8 @@ crate.annotation(<a href="#crate.annotation-version">version</a>, <a href="#crat
<a href="#crate.annotation-build_script_tools">build_script_tools</a>, <a href="#crate.annotation-build_script_data_glob">build_script_data_glob</a>, <a href="#crate.annotation-build_script_deps">build_script_deps</a>, <a href="#crate.annotation-build_script_env">build_script_env</a>,
<a href="#crate.annotation-build_script_proc_macro_deps">build_script_proc_macro_deps</a>, <a href="#crate.annotation-build_script_rustc_env">build_script_rustc_env</a>, <a href="#crate.annotation-build_script_toolchains">build_script_toolchains</a>,
<a href="#crate.annotation-compile_data">compile_data</a>, <a href="#crate.annotation-compile_data_glob">compile_data_glob</a>, <a href="#crate.annotation-crate_features">crate_features</a>, <a href="#crate.annotation-data">data</a>, <a href="#crate.annotation-data_glob">data_glob</a>, <a href="#crate.annotation-deps">deps</a>, <a href="#crate.annotation-gen_binaries">gen_binaries</a>,
<a href="#crate.annotation-gen_build_script">gen_build_script</a>, <a href="#crate.annotation-patch_args">patch_args</a>, <a href="#crate.annotation-patch_tool">patch_tool</a>, <a href="#crate.annotation-patches">patches</a>, <a href="#crate.annotation-proc_macro_deps">proc_macro_deps</a>, <a href="#crate.annotation-rustc_env">rustc_env</a>,
<a href="#crate.annotation-rustc_env_files">rustc_env_files</a>, <a href="#crate.annotation-rustc_flags">rustc_flags</a>, <a href="#crate.annotation-shallow_since">shallow_since</a>)
<a href="#crate.annotation-disable_pipelining">disable_pipelining</a>, <a href="#crate.annotation-gen_build_script">gen_build_script</a>, <a href="#crate.annotation-patch_args">patch_args</a>, <a href="#crate.annotation-patch_tool">patch_tool</a>, <a href="#crate.annotation-patches">patches</a>,
<a href="#crate.annotation-proc_macro_deps">proc_macro_deps</a>, <a href="#crate.annotation-rustc_env">rustc_env</a>, <a href="#crate.annotation-rustc_env_files">rustc_env_files</a>, <a href="#crate.annotation-rustc_flags">rustc_flags</a>, <a href="#crate.annotation-shallow_since">shallow_since</a>)
</pre>

A collection of extra attributes and settings for a particular crate
Expand Down Expand Up @@ -631,6 +631,7 @@ A collection of extra attributes and settings for a particular crate
| <a id="crate.annotation-data_glob"></a>data_glob | A list of glob patterns to add to a crate's <code>rust_library::data</code> attribute. | `None` |
| <a id="crate.annotation-deps"></a>deps | A list of labels to add to a crate's <code>rust_library::deps</code> attribute. | `None` |
| <a id="crate.annotation-gen_binaries"></a>gen_binaries | As a list, the subset of the crate's bins that should get <code>rust_binary</code> targets produced. Or <code>True</code> to generate all, <code>False</code> to generate none. | `[]` |
| <a id="crate.annotation-disable_pipelining"></a>disable_pipelining | If True, disables pipelining for library targets for this crate. | `False` |
| <a id="crate.annotation-gen_build_script"></a>gen_build_script | An authorative flag to determine whether or not to produce <code>cargo_build_script</code> targets for the current crate. | `None` |
| <a id="crate.annotation-patch_args"></a>patch_args | The <code>patch_args</code> attribute of a Bazel repository rule. See [http_archive.patch_args](https://docs.bazel.build/versions/main/repo/http.html#http_archive-patch_args) | `None` |
| <a id="crate.annotation-patch_tool"></a>patch_tool | The <code>patch_tool</code> attribute of a Bazel repository rule. See [http_archive.patch_tool](https://docs.bazel.build/versions/main/repo/http.html#http_archive-patch_tool) | `None` |
Expand Down
24 changes: 12 additions & 12 deletions examples/crate_universe/cargo_aliases/Cargo.Bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit ad01d1b

Please sign in to comment.