Skip to content

Commit 0111b9d

Browse files
committed
apply nit notes
Signed-off-by: onur-ozkan <work@onurozkan.dev>
1 parent ad2e4a4 commit 0111b9d

File tree

2 files changed

+70
-25
lines changed

2 files changed

+70
-25
lines changed

bootstrap.example.toml

+3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
# Inherits configuration values from different configuration files (a.k.a. config extensions).
2323
# Supports absolute paths, and uses the current directory (where the bootstrap was invoked)
2424
# as the base if the given path is not absolute.
25+
#
26+
# The overriding logic follows a right-to-left order. For example, in `include = ["a.toml", "b.toml"]`,
27+
# extension `b.toml` overrides `a.toml`. Also, parent extensions always overrides the inner ones.
2528
#include = []
2629

2730
# Keeps track of major changes made to this configuration.

src/bootstrap/src/core/config/config.rs

+67-25
Original file line numberDiff line numberDiff line change
@@ -748,6 +748,7 @@ enum ReplaceOpt {
748748
trait Merge {
749749
fn merge(
750750
&mut self,
751+
parent_config_path: Option<PathBuf>,
751752
included_extensions: &mut HashSet<PathBuf>,
752753
other: Self,
753754
replace: ReplaceOpt,
@@ -757,26 +758,35 @@ trait Merge {
757758
impl Merge for TomlConfig {
758759
fn merge(
759760
&mut self,
761+
parent_config_path: Option<PathBuf>,
760762
included_extensions: &mut HashSet<PathBuf>,
761763
TomlConfig { build, install, llvm, gcc, rust, dist, target, profile, change_id, include }: Self,
762764
replace: ReplaceOpt,
763765
) {
764766
fn do_merge<T: Merge>(x: &mut Option<T>, y: Option<T>, replace: ReplaceOpt) {
765767
if let Some(new) = y {
766768
if let Some(original) = x {
767-
original.merge(&mut Default::default(), new, replace);
769+
original.merge(None, &mut Default::default(), new, replace);
768770
} else {
769771
*x = Some(new);
770772
}
771773
}
772774
}
773775

774-
for include_path in include.clone().unwrap_or_default() {
776+
let parent_dir = parent_config_path
777+
.as_ref()
778+
.and_then(|p| p.parent().map(ToOwned::to_owned))
779+
.unwrap_or_default();
780+
781+
for include_path in include.clone().unwrap_or_default().iter().rev() {
782+
let include_path = parent_dir.join(include_path);
783+
let include_path = include_path.canonicalize().unwrap_or_else(|e| {
784+
eprintln!("ERROR: Failed to canonicalize '{}' path: {e}", include_path.display());
785+
exit!(2);
786+
});
787+
775788
let included_toml = Config::get_toml(&include_path).unwrap_or_else(|e| {
776-
eprintln!(
777-
"ERROR: Failed to parse default config profile at '{}': {e}",
778-
include_path.display()
779-
);
789+
eprintln!("ERROR: Failed to parse '{}': {e}", include_path.display());
780790
exit!(2);
781791
});
782792

@@ -786,13 +796,20 @@ impl Merge for TomlConfig {
786796
include_path.display()
787797
);
788798

789-
self.merge(included_extensions, included_toml, ReplaceOpt::Override);
799+
self.merge(
800+
Some(include_path.clone()),
801+
included_extensions,
802+
included_toml,
803+
// Ensures that parent configuration always takes precedence
804+
// over child configurations.
805+
ReplaceOpt::IgnoreDuplicate,
806+
);
790807

791808
included_extensions.remove(&include_path);
792809
}
793810

794-
self.change_id.inner.merge(&mut Default::default(), change_id.inner, replace);
795-
self.profile.merge(&mut Default::default(), profile, replace);
811+
self.change_id.inner.merge(None, &mut Default::default(), change_id.inner, replace);
812+
self.profile.merge(None, &mut Default::default(), profile, replace);
796813

797814
do_merge(&mut self.build, build, replace);
798815
do_merge(&mut self.install, install, replace);
@@ -807,7 +824,7 @@ impl Merge for TomlConfig {
807824
(Some(original_target), Some(new_target)) => {
808825
for (triple, new) in new_target {
809826
if let Some(original) = original_target.get_mut(&triple) {
810-
original.merge(&mut Default::default(), new, replace);
827+
original.merge(None, &mut Default::default(), new, replace);
811828
} else {
812829
original_target.insert(triple, new);
813830
}
@@ -828,7 +845,13 @@ macro_rules! define_config {
828845
}
829846

830847
impl Merge for $name {
831-
fn merge(&mut self, _included_extensions: &mut HashSet<PathBuf>, other: Self, replace: ReplaceOpt) {
848+
fn merge(
849+
&mut self,
850+
_parent_config_path: Option<PathBuf>,
851+
_included_extensions: &mut HashSet<PathBuf>,
852+
other: Self,
853+
replace: ReplaceOpt
854+
) {
832855
$(
833856
match replace {
834857
ReplaceOpt::IgnoreDuplicate => {
@@ -930,6 +953,7 @@ macro_rules! define_config {
930953
impl<T> Merge for Option<T> {
931954
fn merge(
932955
&mut self,
956+
_parent_config_path: Option<PathBuf>,
933957
_included_extensions: &mut HashSet<PathBuf>,
934958
other: Self,
935959
replace: ReplaceOpt,
@@ -1608,6 +1632,24 @@ impl Config {
16081632
toml.profile = Some("dist".into());
16091633
}
16101634

1635+
// Reverse the list to ensure the last added config extension remains the most dominant.
1636+
// For example, given ["a.toml", "b.toml"], "b.toml" should take precedence over "a.toml".
1637+
//
1638+
// This must be handled before applying the `profile` since `include`s should always take
1639+
// precedence over `profile`s.
1640+
for include_path in toml.include.clone().unwrap_or_default().iter().rev() {
1641+
let included_toml = get_toml(include_path).unwrap_or_else(|e| {
1642+
eprintln!("ERROR: Failed to parse '{}': {e}", include_path.display());
1643+
exit!(2);
1644+
});
1645+
toml.merge(
1646+
Some(config.src.join(include_path)),
1647+
&mut Default::default(),
1648+
included_toml,
1649+
ReplaceOpt::IgnoreDuplicate,
1650+
);
1651+
}
1652+
16111653
if let Some(include) = &toml.profile {
16121654
// Allows creating alias for profile names, allowing
16131655
// profiles to be renamed while maintaining back compatibility
@@ -1629,18 +1671,12 @@ impl Config {
16291671
);
16301672
exit!(2);
16311673
});
1632-
toml.merge(&mut Default::default(), included_toml, ReplaceOpt::IgnoreDuplicate);
1633-
}
1634-
1635-
for include_path in toml.include.clone().unwrap_or_default() {
1636-
let included_toml = get_toml(&include_path).unwrap_or_else(|e| {
1637-
eprintln!(
1638-
"ERROR: Failed to parse default config profile at '{}': {e}",
1639-
include_path.display()
1640-
);
1641-
exit!(2);
1642-
});
1643-
toml.merge(&mut Default::default(), included_toml, ReplaceOpt::Override);
1674+
toml.merge(
1675+
Some(include_path),
1676+
&mut Default::default(),
1677+
included_toml,
1678+
ReplaceOpt::IgnoreDuplicate,
1679+
);
16441680
}
16451681

16461682
let mut override_toml = TomlConfig::default();
@@ -1651,7 +1687,12 @@ impl Config {
16511687

16521688
let mut err = match get_table(option) {
16531689
Ok(v) => {
1654-
override_toml.merge(&mut Default::default(), v, ReplaceOpt::ErrorOnDuplicate);
1690+
override_toml.merge(
1691+
None,
1692+
&mut Default::default(),
1693+
v,
1694+
ReplaceOpt::ErrorOnDuplicate,
1695+
);
16551696
continue;
16561697
}
16571698
Err(e) => e,
@@ -1663,6 +1704,7 @@ impl Config {
16631704
match get_table(&format!(r#"{key}="{value}""#)) {
16641705
Ok(v) => {
16651706
override_toml.merge(
1707+
None,
16661708
&mut Default::default(),
16671709
v,
16681710
ReplaceOpt::ErrorOnDuplicate,
@@ -1676,7 +1718,7 @@ impl Config {
16761718
eprintln!("failed to parse override `{option}`: `{err}");
16771719
exit!(2)
16781720
}
1679-
toml.merge(&mut Default::default(), override_toml, ReplaceOpt::Override);
1721+
toml.merge(None, &mut Default::default(), override_toml, ReplaceOpt::Override);
16801722

16811723
config.change_id = toml.change_id.inner;
16821724

0 commit comments

Comments
 (0)