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

Added global setting for cargo_build_script.use_default_shell_env #3065

Merged
merged 4 commits into from
Dec 13, 2024
Merged
Changes from 1 commit
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
Next Next commit
Added global setting for cargo_build_script.use_default_shell_env
UebelAndre committed Dec 9, 2024
commit ffd294383bcdc3230ee8f30d92dadb7b9ec76434
21 changes: 15 additions & 6 deletions cargo/private/cargo_build_script.bzl
Original file line number Diff line number Diff line change
@@ -348,9 +348,16 @@ def _cargo_build_script_impl(ctx):

env = dict({})

if ctx.attr.use_default_shell_env == -1:
use_default_shell_env = ctx.attr._default_use_default_shell_env[BuildSettingInfo].value
elif ctx.attr.use_default_shell_env == 0:
use_default_shell_env = False
else:
use_default_shell_env = True

# If enabled, start with the default shell env, which contains any --action_env
# settings passed in on the command line and defaults like $PATH.
if ctx.attr.use_default_shell_env:
if use_default_shell_env:
env.update(ctx.configuration.default_shell_env)

env.update({
@@ -537,9 +544,7 @@ def _cargo_build_script_impl(ctx):
progress_message = "Running Cargo build script {}".format(pkg_name),
env = env,
toolchain = None,
# If enabled, sets the $PATH environment variable so tools like `cmake`
# can probe $PATH for helper tools. Defaults to `True`.
use_default_shell_env = ctx.attr.use_default_shell_env,
use_default_shell_env = use_default_shell_env,
)

return [
@@ -628,13 +633,14 @@ cargo_build_script = rule(
allow_files = True,
cfg = "exec",
),
"use_default_shell_env": attr.bool(
"use_default_shell_env": attr.int(
doc = dedent("""\
Whether or not to include the default shell environment for the build
script action. By default Bazel's `default_shell_env` is set for build
script actions so crates like `cmake` can probe $PATH to find tools.
"""),
default = True,
default = -1,
values = [-1, 0, 1],
),
"version": attr.string(
doc = "The semantic version (semver) of the crate",
@@ -654,6 +660,9 @@ cargo_build_script = rule(
"_debug_std_streams_output_group": attr.label(
default = Label("//cargo/settings:debug_std_streams_output_group"),
),
"_default_use_default_shell_env": attr.label(
default = Label("//cargo/settings:use_default_shell_env"),
),
"_experimental_symlink_execroot": attr.label(
default = Label("//cargo/settings:experimental_symlink_execroot"),
),
12 changes: 12 additions & 0 deletions cargo/private/cargo_build_script_wrapper.bzl
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@ load(
load("//rust:defs.bzl", "rust_binary")

def cargo_build_script(
*,
name,
edition = None,
crate_name = None,
@@ -21,6 +22,7 @@ def cargo_build_script(
link_deps = [],
proc_macro_deps = [],
build_script_env = {},
use_default_shell_env = None,
data = [],
compile_data = [],
tools = [],
@@ -107,6 +109,8 @@ def cargo_build_script(
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.
build_script_env (dict, optional): Environment variables for build scripts.
use_default_shell_env (bool, optional): Whether or not to include the default shell environment for the build script action. If unset the global
setting `@rules_rust//cargo/settings:use_default_shell_env` will be used to determine this value.
data (list, optional): Files needed by the build script.
compile_data (list, optional): Files needed for the compilation of the build script.
tools (list, optional): Tools (executables) needed by the build script.
@@ -191,13 +195,21 @@ def cargo_build_script(
**wrapper_kwargs
)

if use_default_shell_env == None:
sanitized_use_default_shell_env = -1
elif type(use_default_shell_env) == "bool":
sanitized_use_default_shell_env = 1 if use_default_shell_env else 0
else:
sanitized_use_default_shell_env = use_default_shell_env

# This target executes the build script.
_build_script_run(
name = name,
script = ":{}-".format(name),
crate_features = crate_features,
version = version,
build_script_env = build_script_env,
use_default_shell_env = sanitized_use_default_shell_env,
links = links,
deps = deps,
link_deps = link_deps,
7 changes: 7 additions & 0 deletions cargo/settings/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -34,3 +34,10 @@ bool_flag(
name = "debug_std_streams_output_group",
build_setting_default = False,
)

# A flag which controls the global default of `ctx.actions.run.use_default_shell_env`
# for `cargo_build_script` targets.
bool_flag(
name = "use_default_shell_env",
build_setting_default = True,
)
4 changes: 4 additions & 0 deletions crate_universe/private/crate.bzl
Original file line number Diff line number Diff line change
@@ -97,6 +97,7 @@ def _annotation(
build_script_rundir = None,
build_script_rustc_env = None,
build_script_toolchains = None,
build_script_use_default_shell_env = None,
compile_data = None,
compile_data_glob = None,
crate_features = None,
@@ -140,6 +141,8 @@ def _annotation(
build_script_rustc_env (dict, optional): Additional environment variables to set on a crate's
`cargo_build_script::env` attribute.
build_script_toolchains (list, optional): A list of labels to set on a crates's `cargo_build_script::toolchains` attribute.
build_script_use_default_shell_env (int, optional): Whether or not to include the default shell environment for the build
script action.
compile_data (list, optional): A list of labels to add to a crate's `rust_library::compile_data` attribute.
compile_data_glob (list, optional): A list of glob patterns to add to a crate's `rust_library::compile_data`
attribute.
@@ -197,6 +200,7 @@ def _annotation(
build_script_rundir = build_script_rundir,
build_script_rustc_env = build_script_rustc_env,
build_script_toolchains = _stringify_list(build_script_toolchains),
build_script_use_default_shell_env = build_script_use_default_shell_env,
compile_data = _stringify_list(compile_data),
compile_data_glob = compile_data_glob,
crate_features = crate_features,
5 changes: 5 additions & 0 deletions crate_universe/src/config.rs
Original file line number Diff line number Diff line change
@@ -303,6 +303,10 @@ pub(crate) struct CrateAnnotations {
/// [toolchains](https://bazel.build/reference/be/common-definitions#common-attributes) attribute.
pub(crate) build_script_toolchains: Option<BTreeSet<Label>>,

/// Additional rustc_env flags to pass to a build script's
/// [use_default_shell_env](https://bazelbuild.github.io/rules_rust/cargo.html#cargo_build_script-use_default_shell_env) attribute.
pub(crate) build_script_use_default_shell_env: Option<i32>,

/// Directory to run the crate's build script in. If not set, will run in the manifest directory, otherwise a directory relative to the exec root.
pub(crate) build_script_rundir: Option<Select<String>>,

@@ -398,6 +402,7 @@ impl Add for CrateAnnotations {
build_script_env: select_merge(self.build_script_env, rhs.build_script_env),
build_script_rustc_env: select_merge(self.build_script_rustc_env, rhs.build_script_rustc_env),
build_script_toolchains: joined_extra_member!(self.build_script_toolchains, rhs.build_script_toolchains, BTreeSet::new, BTreeSet::extend),
build_script_use_default_shell_env: self.build_script_use_default_shell_env.or(rhs.build_script_use_default_shell_env),
build_script_rundir: self.build_script_rundir.or(rhs.build_script_rundir),
additive_build_file_content: joined_extra_member!(self.additive_build_file_content, rhs.additive_build_file_content, String::new, concat_string),
shallow_since: self.shallow_since.or(rhs.shallow_since),
9 changes: 9 additions & 0 deletions crate_universe/src/context/crate_context.rs
Original file line number Diff line number Diff line change
@@ -242,6 +242,9 @@ pub(crate) struct BuildScriptAttributes {

#[serde(skip_serializing_if = "BTreeSet::is_empty")]
pub(crate) toolchains: BTreeSet<Label>,

#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) use_default_shell_env: Option<i32>,
}

impl Default for BuildScriptAttributes {
@@ -268,6 +271,7 @@ impl Default for BuildScriptAttributes {
tools: Default::default(),
links: Default::default(),
toolchains: Default::default(),
use_default_shell_env: None,
}
}
}
@@ -656,6 +660,11 @@ impl CrateContext {
Select::merge(attrs.build_script_env.clone(), extra.clone());
}

// Default Shell Env
if let Some(extra) = &crate_extra.build_script_use_default_shell_env {
attrs.use_default_shell_env = Some(*extra);
}

if let Some(rundir) = &crate_extra.build_script_rundir {
attrs.rundir = Select::merge(attrs.rundir.clone(), rundir.clone());
}
2 changes: 1 addition & 1 deletion crate_universe/src/lockfile.rs
Original file line number Diff line number Diff line change
@@ -290,7 +290,7 @@ mod test {
);

assert_eq!(
Digest("da25032dd6941980501632d9d63bd3daffc6261443ccc3c4dcdbb45a99088cce".to_owned()),
Digest("9e578eae3fe8a8a8211433d7c1d40bc26a1d921fba1451235b8d6fe0939ba810".to_owned()),
digest,
);
}
108 changes: 104 additions & 4 deletions crate_universe/src/rendering.rs
Original file line number Diff line number Diff line change
@@ -486,6 +486,10 @@ impl Renderer {
.unwrap_or_default(),
platforms,
),
use_default_shell_env: krate
.build_script_attrs
.as_ref()
.and_then(|a| a.use_default_shell_env),
compile_data: make_data_with_exclude(
platforms,
attrs
@@ -1086,6 +1090,7 @@ mod test {
fn render_cargo_build_script() {
let mut context = Context::default();
let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO);

context.crates.insert(
crate_id.clone(),
CrateContext {
@@ -1120,10 +1125,105 @@ mod test {
.get(&PathBuf::from("BUILD.mock_crate-0.1.0.bazel"))
.unwrap();

assert!(build_file_content.contains("cargo_build_script("));
assert!(build_file_content.contains("name = \"build_script_build\""));
assert!(build_file_content.contains("\"crate-name=mock_crate\""));
assert!(build_file_content.contains("compile_data = glob("));
assert!(
build_file_content.contains("cargo_build_script("),
"```\n{}```\n",
build_file_content
);
assert!(
build_file_content.contains("name = \"build_script_build\""),
"```\n{}```\n",
build_file_content
);
assert!(
build_file_content.contains("\"crate-name=mock_crate\""),
"```\n{}```\n",
build_file_content
);
assert!(
build_file_content.contains("compile_data = glob("),
"```\n{}```\n",
build_file_content
);
assert!(
!build_file_content.contains("use_default_shell_env ="),
"```\n{}```\n",
build_file_content
);

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

#[test]
fn render_cargo_build_script_complex() {
let mut context = Context::default();
let crate_id = CrateId::new("mock_crate".to_owned(), VERSION_ZERO_ONE_ZERO);

let attrs = BuildScriptAttributes {
use_default_shell_env: Some(1),
..BuildScriptAttributes::default()
};

context.crates.insert(
crate_id.clone(),
CrateContext {
name: crate_id.name,
version: crate_id.version,
package_url: None,
repository: None,
targets: BTreeSet::from([Rule::BuildScript(TargetAttributes {
crate_name: "build_script_build".to_owned(),
crate_root: Some("build.rs".to_owned()),
..TargetAttributes::default()
})]),
// Build script attributes are required.
library_target_name: None,
common_attrs: CommonAttributes::default(),
build_script_attrs: Some(attrs),
license: None,
license_ids: BTreeSet::default(),
license_file: None,
additive_build_file_content: None,
disable_pipelining: false,
extra_aliased_targets: BTreeMap::default(),
alias_rule: None,
override_targets: BTreeMap::default(),
},
);

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

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

assert!(
build_file_content.contains("cargo_build_script("),
"```\n{}```\n",
build_file_content
);
assert!(
build_file_content.contains("name = \"build_script_build\""),
"```\n{}```\n",
build_file_content
);
assert!(
build_file_content.contains("\"crate-name=mock_crate\""),
"```\n{}```\n",
build_file_content
);
assert!(
build_file_content.contains("compile_data = glob("),
"```\n{}```\n",
build_file_content
);
assert!(
build_file_content.contains("use_default_shell_env = 1"),
"```\n{}```\n",
build_file_content
);

// Ensure `cargo_build_script` requirements are met
assert!(build_file_content.contains("name = \"_bs\""));
2 changes: 2 additions & 0 deletions crate_universe/src/utils/starlark.rs
Original file line number Diff line number Diff line change
@@ -133,6 +133,8 @@ pub(crate) struct CargoBuildScript {
pub(crate) tools: SelectSet<Label>,
#[serde(skip_serializing_if = "Set::is_empty")]
pub(crate) toolchains: Set<Label>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) use_default_shell_env: Option<i32>,
pub(crate) version: String,
pub(crate) visibility: Set<String>,
}