WIP: Implementation for multiple build scripts #15704
Conversation
rustbot
added
A-build-execution
Bad copy/paste? |
| pub fn find_build_script_metadata(&self, unit: &Unit) -> Option<UnitHash> { | ||
| let script_unit = self.find_build_script_unit(unit)?; | ||
| Some(self.get_run_build_script_metadata(&script_unit)) | ||
| pub fn find_build_script_metadata(&self, unit: &Unit) -> Option<Vec<UnitHash>> { |
There was a problem hiding this comment.
find_build_script_metadata -> find_build_script_metadatas to express through the name that this is plural
| @@ -45,7 +45,7 @@ pub struct Doctest { | |||
| /// The script metadata, if this unit's package has a build script. | |||
| /// | |||
| /// This is used for indexing [`Compilation::extra_env`]. | |||
| pub script_meta: Option<UnitHash>, | |||
| pub scripts_meta: Option<Vec<UnitHash>>, | |||
There was a problem hiding this comment.
Always a weird question on whether to pluralize the first part or second. I lean towards the second
| @@ -61,7 +61,7 @@ pub struct UnitOutput { | |||
| /// The script metadata, if this unit's package has a build script. | |||
| /// | |||
| /// This is used for indexing [`Compilation::extra_env`]. | |||
| pub script_meta: Option<UnitHash>, | |||
| pub script_meta: Option<Vec<UnitHash>>, | |||
There was a problem hiding this comment.
Should we also pluralize this?
| } | ||
|
|
||
| /// Returns a [`ProcessBuilder`] for running `rustdoc`. | ||
| pub fn rustdoc_process( | ||
| &self, | ||
| unit: &Unit, | ||
| script_meta: Option<UnitHash>, | ||
| script_meta: &Option<Vec<UnitHash>>, |
There was a problem hiding this comment.
This should be switched to Option<&Vec<UnitHash>>, (accessed via Option::as_ref) or Option<&[UnitHash]>, (accessed via Option::as_deref)
| pub fn find_build_script_units(&self, unit: &Unit) -> Option<Vec<Unit>> { | ||
| if unit.mode.is_run_custom_build() { | ||
| return Some(unit.clone()); | ||
| return Some(vec![unit.clone()]); | ||
| } | ||
| self.bcx.unit_graph[unit] | ||
|
|
||
| let build_script_units: Vec<Unit> = self.bcx.unit_graph[unit] | ||
| .iter() | ||
| .find(|unit_dep| { | ||
| .filter(|unit_dep| { | ||
| unit_dep.unit.mode.is_run_custom_build() | ||
| && unit_dep.unit.pkg.package_id() == unit.pkg.package_id() | ||
| }) | ||
| .map(|unit_dep| unit_dep.unit.clone()) | ||
| .collect(); | ||
| if build_script_units.is_empty() { | ||
| None | ||
| } else { | ||
| Some(build_script_units) | ||
| } | ||
| } |
There was a problem hiding this comment.
What order is this returning units? We should do it in a deterministic order, preferably one controlled by the user.
There was a problem hiding this comment.
If this commit is just about changing types, we can always add this in a later commit
namanlp
force-pushed
the
multiple-build-scripts-2
branch
from
66a7106 to
2042b1c
Compare
namanlp
force-pushed
the
multiple-build-scripts-2
branch
from
2042b1c to
96ca257
Compare
Hi Everyone!
This is PR for the implementation of the first milestone of GSoC Project : Build Script Delegation
This will provide actual implementation for #15630
What does this PR try to resolve?
Now, multiple build scripts are parsed, this PR aims to implement the functioning the feature. This PR will allow users to use multiple build scripts, and is backward compatible with single script as well as boolean values.
Motivation : This will help users to maintain separate smaller and cleaner build scripts instead of one large build script. This is also necessary for build script delegation.
How to test and review this PR?
There is a feature gate
multiple-build-scriptsthat can be passed viacargo-featuresinCargo.toml. So, you have to addPreferably on the top of the
Cargo.tomland use nightly toolchain to use the featureThis PR is WIP and not ready for review yet!