Skip to content

Commit

Permalink
crate_universe: Don't include crate name in build script path. (#2663)
Browse files Browse the repository at this point in the history
This PR shortens the path to the build script (potentially
significantly), which helps with long-path issues on Windows.
This change is motivated by the crate `tree-sitter-embedded-template`,
which has a too long path to the compiled build script otherwise, and
which then fails to build on Windows in a `bzlmod` setting.
Fixes #2520.

The cargo build script name was used to automatically derive the cargo
package name, this now went into a separate override parameter that is
generated by the crate-universe tooling.
There's never two build scripts in a single crate, so I don't see how
having a single build script target with a single name would be a
problem.
  • Loading branch information
criemen authored Jun 3, 2024
1 parent a92de54 commit e3f6258
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 12 deletions.
9 changes: 7 additions & 2 deletions cargo/private/cargo_build_script.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ def _cargo_build_script_impl(ctx):
stderr = ctx.actions.declare_file(ctx.label.name + ".stderr.log"),
)

pkg_name = name_to_pkg_name(ctx.label.name)
pkg_name = ctx.attr.pkg_name
if pkg_name == "":
pkg_name = name_to_pkg_name(ctx.label.name)

toolchain_tools = [toolchain.all_files]

Expand Down Expand Up @@ -373,6 +375,9 @@ cargo_build_script = rule(
"links": attr.string(
doc = "The name of the native library this crate links against.",
),
"pkg_name": attr.string(
doc = "The name of package being compiled, if not derived from `name`.",
),
"rundir": attr.string(
default = "",
doc = dedent("""\
Expand All @@ -388,7 +393,7 @@ cargo_build_script = rule(
List of compiler flags passed to `rustc`.
These strings are subject to Make variable expansion for predefined
source/output path variables like `$location`, `$execpath`, and
source/output path variables like `$location`, `$execpath`, and
`$rootpath`. This expansion is useful if you wish to pass a generated
file of arguments to rustc: `@$(location //package:target)`.
"""),
Expand Down
10 changes: 8 additions & 2 deletions cargo/private/cargo_build_script_wrapper.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def cargo_build_script(
visibility = None,
tags = None,
aliases = None,
pkg_name = None,
**kwargs):
"""Compile and execute a rust build script to generate build attributes
Expand Down Expand Up @@ -92,14 +93,15 @@ def cargo_build_script(
Args:
name (str): The name for the underlying rule. This should be the name of the package
being compiled, optionally with a suffix of `_build_script`.
being compiled, optionally with a suffix of `_bs`. Otherwise, you can set the package name via `pkg_name`.
edition (str): The rust edition to use for the internal binary crate.
crate_name (str): Crate name to use for build script.
crate_root (label): The file that will be passed to rustc to be used for building this crate.
srcs (list of label): Souce files of the crate to build. Passing source files here can be used to trigger rebuilds when changes are made.
crate_features (list, optional): A list of features to enable for the build script.
version (str, optional): The semantic version (semver) of the crate.
deps (list, optional): The build-dependencies of the crate.
pkg_name (string, optional): Override the package name used for the build script. This is useful if the build target name gets too long otherwise.
link_deps (list, optional): The subset of the (normal) dependencies of the crate that have the
links attribute and therefore provide environment variables to this build script.
proc_macro_deps (list of label, optional): List of rust_proc_macro targets used to build the script.
Expand Down Expand Up @@ -131,9 +133,12 @@ def cargo_build_script(
# available both when we invoke rustc (this code) and when we run the compiled build
# script (_cargo_build_script_impl). https://github.com/bazelbuild/rules_rust/issues/661
# will hopefully remove this duplication.
if pkg_name == None:
pkg_name = name_to_pkg_name(name)

rustc_env = dict(rustc_env)
if "CARGO_PKG_NAME" not in rustc_env:
rustc_env["CARGO_PKG_NAME"] = name_to_pkg_name(name)
rustc_env["CARGO_PKG_NAME"] = pkg_name
if "CARGO_CRATE_NAME" not in rustc_env:
rustc_env["CARGO_CRATE_NAME"] = name_to_crate_name(name_to_pkg_name(name))

Expand Down Expand Up @@ -173,5 +178,6 @@ def cargo_build_script(
rustc_flags = rustc_flags,
visibility = visibility,
tags = tags,
pkg_name = pkg_name,
**kwargs
)
9 changes: 5 additions & 4 deletions crate_universe/src/rendering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ impl Renderer {
starlark.push(Starlark::Alias(Alias {
rule: AliasRule::default().rule(),
name: target.crate_name.clone(),
actual: Label::from_str(&format!(":{}_bs", krate.name)).unwrap(),
actual: Label::from_str("_bs").unwrap(),
tags: BTreeSet::from(["manual".to_owned()]),
}));
}
Expand Down Expand Up @@ -435,8 +435,8 @@ impl Renderer {
//
// Do not change this name to "cargo_build_script".
//
// This is set to a short suffix to avoid long path name issues on windows.
name: format!("{}_bs", krate.name),
// This is set to a short name to avoid long path name issues on windows.
name: "_bs".to_string(),
aliases: SelectDict::new(self.make_aliases(krate, true, false), platforms),
build_script_env: SelectDict::new(
attrs
Expand Down Expand Up @@ -484,6 +484,7 @@ impl Renderer {
edition: krate.common_attrs.edition.clone(),
linker_script: krate.common_attrs.linker_script.clone(),
links: attrs.and_then(|attrs| attrs.links.clone()),
pkg_name: Some(krate.name.clone()),
proc_macro_deps: SelectSet::new(
self.make_deps(
attrs
Expand Down Expand Up @@ -1058,7 +1059,7 @@ mod test {
assert!(build_file_content.contains("\"crate-name=mock_crate\""));

// Ensure `cargo_build_script` requirements are met
assert!(build_file_content.contains("name = \"mock_crate_bs\""));
assert!(build_file_content.contains("name = \"_bs\""));
}

#[test]
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 @@ -114,6 +114,8 @@ pub(crate) struct CargoBuildScript {
pub(crate) linker_script: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) links: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) pkg_name: Option<String>,
#[serde(skip_serializing_if = "SelectSet::is_empty")]
pub(crate) proc_macro_deps: SelectSet<Label>,
#[serde(skip_serializing_if = "SelectScalar::is_empty")]
Expand Down
6 changes: 4 additions & 2 deletions docs/cargo.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ A rule for generating variables for dependent `cargo_build_script`s without a bu
<pre>
cargo_build_script(<a href="#cargo_build_script-name">name</a>, <a href="#cargo_build_script-edition">edition</a>, <a href="#cargo_build_script-crate_name">crate_name</a>, <a href="#cargo_build_script-crate_root">crate_root</a>, <a href="#cargo_build_script-srcs">srcs</a>, <a href="#cargo_build_script-crate_features">crate_features</a>, <a href="#cargo_build_script-version">version</a>, <a href="#cargo_build_script-deps">deps</a>,
<a href="#cargo_build_script-link_deps">link_deps</a>, <a href="#cargo_build_script-proc_macro_deps">proc_macro_deps</a>, <a href="#cargo_build_script-build_script_env">build_script_env</a>, <a href="#cargo_build_script-data">data</a>, <a href="#cargo_build_script-compile_data">compile_data</a>, <a href="#cargo_build_script-tools">tools</a>, <a href="#cargo_build_script-links">links</a>,
<a href="#cargo_build_script-rundir">rundir</a>, <a href="#cargo_build_script-rustc_env">rustc_env</a>, <a href="#cargo_build_script-rustc_env_files">rustc_env_files</a>, <a href="#cargo_build_script-rustc_flags">rustc_flags</a>, <a href="#cargo_build_script-visibility">visibility</a>, <a href="#cargo_build_script-tags">tags</a>, <a href="#cargo_build_script-aliases">aliases</a>, <a href="#cargo_build_script-kwargs">kwargs</a>)
<a href="#cargo_build_script-rundir">rundir</a>, <a href="#cargo_build_script-rustc_env">rustc_env</a>, <a href="#cargo_build_script-rustc_env_files">rustc_env_files</a>, <a href="#cargo_build_script-rustc_flags">rustc_flags</a>, <a href="#cargo_build_script-visibility">visibility</a>, <a href="#cargo_build_script-tags">tags</a>, <a href="#cargo_build_script-aliases">aliases</a>,
<a href="#cargo_build_script-pkg_name">pkg_name</a>, <a href="#cargo_build_script-kwargs">kwargs</a>)
</pre>

Compile and execute a rust build script to generate build attributes
Expand Down Expand Up @@ -131,7 +132,7 @@ The `hello_lib` target will be build with the flags and the environment variable

| Name | Description | Default Value |
| :------------- | :------------- | :------------- |
| <a id="cargo_build_script-name"></a>name | The name for the underlying rule. This should be the name of the package being compiled, optionally with a suffix of <code>_build_script</code>. | none |
| <a id="cargo_build_script-name"></a>name | The name for the underlying rule. This should be the name of the package being compiled, optionally with a suffix of <code>_bs</code>. Otherwise, you can set the package name via <code>pkg_name</code>. | none |
| <a id="cargo_build_script-edition"></a>edition | The rust edition to use for the internal binary crate. | `None` |
| <a id="cargo_build_script-crate_name"></a>crate_name | Crate name to use for build script. | `None` |
| <a id="cargo_build_script-crate_root"></a>crate_root | The file that will be passed to rustc to be used for building this crate. | `None` |
Expand All @@ -153,6 +154,7 @@ The `hello_lib` target will be build with the flags and the environment variable
| <a id="cargo_build_script-visibility"></a>visibility | Visibility to apply to the generated build script output. | `None` |
| <a id="cargo_build_script-tags"></a>tags | (list of str, optional): Tags to apply to the generated build script output. | `None` |
| <a id="cargo_build_script-aliases"></a>aliases | Remap crates to a new name or moniker for linkage to this target. These are other <code>rust_library</code> targets and will be presented as the new name given. | `None` |
| <a id="cargo_build_script-pkg_name"></a>pkg_name | Override the package name used for the build script. This is useful if the build target name gets too long otherwise. | `None` |
| <a id="cargo_build_script-kwargs"></a>kwargs | Forwards to the underlying <code>rust_binary</code> rule. An exception is the <code>compatible_with</code> attribute, which shouldn't be forwarded to the <code>rust_binary</code>, as the <code>rust_binary</code> is only built and used in <code>exec</code> mode. We propagate the <code>compatible_with</code> attribute to the <code>_build_scirpt_run</code> target. | none |


Expand Down
6 changes: 4 additions & 2 deletions docs/flatten.md
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,8 @@ A collection of files either found within the `rust-stdlib` artifact or generate
<pre>
cargo_build_script(<a href="#cargo_build_script-name">name</a>, <a href="#cargo_build_script-edition">edition</a>, <a href="#cargo_build_script-crate_name">crate_name</a>, <a href="#cargo_build_script-crate_root">crate_root</a>, <a href="#cargo_build_script-srcs">srcs</a>, <a href="#cargo_build_script-crate_features">crate_features</a>, <a href="#cargo_build_script-version">version</a>, <a href="#cargo_build_script-deps">deps</a>,
<a href="#cargo_build_script-link_deps">link_deps</a>, <a href="#cargo_build_script-proc_macro_deps">proc_macro_deps</a>, <a href="#cargo_build_script-build_script_env">build_script_env</a>, <a href="#cargo_build_script-data">data</a>, <a href="#cargo_build_script-compile_data">compile_data</a>, <a href="#cargo_build_script-tools">tools</a>, <a href="#cargo_build_script-links">links</a>,
<a href="#cargo_build_script-rundir">rundir</a>, <a href="#cargo_build_script-rustc_env">rustc_env</a>, <a href="#cargo_build_script-rustc_env_files">rustc_env_files</a>, <a href="#cargo_build_script-rustc_flags">rustc_flags</a>, <a href="#cargo_build_script-visibility">visibility</a>, <a href="#cargo_build_script-tags">tags</a>, <a href="#cargo_build_script-aliases">aliases</a>, <a href="#cargo_build_script-kwargs">kwargs</a>)
<a href="#cargo_build_script-rundir">rundir</a>, <a href="#cargo_build_script-rustc_env">rustc_env</a>, <a href="#cargo_build_script-rustc_env_files">rustc_env_files</a>, <a href="#cargo_build_script-rustc_flags">rustc_flags</a>, <a href="#cargo_build_script-visibility">visibility</a>, <a href="#cargo_build_script-tags">tags</a>, <a href="#cargo_build_script-aliases">aliases</a>,
<a href="#cargo_build_script-pkg_name">pkg_name</a>, <a href="#cargo_build_script-kwargs">kwargs</a>)
</pre>

Compile and execute a rust build script to generate build attributes
Expand Down Expand Up @@ -1637,7 +1638,7 @@ The `hello_lib` target will be build with the flags and the environment variable

| Name | Description | Default Value |
| :------------- | :------------- | :------------- |
| <a id="cargo_build_script-name"></a>name | The name for the underlying rule. This should be the name of the package being compiled, optionally with a suffix of <code>_build_script</code>. | none |
| <a id="cargo_build_script-name"></a>name | The name for the underlying rule. This should be the name of the package being compiled, optionally with a suffix of <code>_bs</code>. Otherwise, you can set the package name via <code>pkg_name</code>. | none |
| <a id="cargo_build_script-edition"></a>edition | The rust edition to use for the internal binary crate. | `None` |
| <a id="cargo_build_script-crate_name"></a>crate_name | Crate name to use for build script. | `None` |
| <a id="cargo_build_script-crate_root"></a>crate_root | The file that will be passed to rustc to be used for building this crate. | `None` |
Expand All @@ -1659,6 +1660,7 @@ The `hello_lib` target will be build with the flags and the environment variable
| <a id="cargo_build_script-visibility"></a>visibility | Visibility to apply to the generated build script output. | `None` |
| <a id="cargo_build_script-tags"></a>tags | (list of str, optional): Tags to apply to the generated build script output. | `None` |
| <a id="cargo_build_script-aliases"></a>aliases | Remap crates to a new name or moniker for linkage to this target. These are other <code>rust_library</code> targets and will be presented as the new name given. | `None` |
| <a id="cargo_build_script-pkg_name"></a>pkg_name | Override the package name used for the build script. This is useful if the build target name gets too long otherwise. | `None` |
| <a id="cargo_build_script-kwargs"></a>kwargs | Forwards to the underlying <code>rust_binary</code> rule. An exception is the <code>compatible_with</code> attribute, which shouldn't be forwarded to the <code>rust_binary</code>, as the <code>rust_binary</code> is only built and used in <code>exec</code> mode. We propagate the <code>compatible_with</code> attribute to the <code>_build_scirpt_run</code> target. | none |


Expand Down

0 comments on commit e3f6258

Please sign in to comment.