Skip to content

Commit 39d3a39

Browse files
David Tolnayfacebook-github-bot
authored andcommitted
Prevent most fixups (such as rustc_flags) from applying to buildscript
Summary: In the fixups we use for `coreaudio-sys` and `rustix`, it is very surprising that the `rustc_flags` they specify were getting applied against **both** the library and the library's build script. https://www.internalfb.com/code/fbsource/[3bec93b51a600700d81b31f6cc00c31af209902f]/third-party/rust/fixups/coreaudio-sys/fixups.toml?lines=3%2C11-17 https://www.internalfb.com/code/fbsource/[3bec93b51a600700d81b31f6cc00c31af209902f]/third-party/rust/fixups/rustix/fixups.toml?lines=3-4 Flags like `-lframework=AudioUnit` and `-Zdefault-visibility=hidden` are definitely intended for the library only. This diff rearranges Reindeer's processing of fixups for libraries and build scripts to make build scripts only receive fixups pertaining to `srcs` (such as `extra_srcs`), `mapped_srcs` (such as `overlay`), and `features`. Other fixups such as `rustc_flags` and `env` and `link_style` should *not* get applied to the build script. For `link_style` (since D74046797) and for `env` (since D73977930), it's now possible to control those for the build script build independently of the library. ```lang=toml # before: applied to both # now: not build script rustc_flags = [...] link_style = "..." env = {...} [buildscript.build] # for build script build link_style = "..." env = {...} # if ever needed, add support for rustc_flags here ``` Reviewed By: JakobDegen Differential Revision: D74046795 fbshipit-source-id: a6592b7827424c00e5b2683bbc2ef0ed74dda6b3
1 parent c9ac474 commit 39d3a39

File tree

1 file changed

+84
-84
lines changed

1 file changed

+84
-84
lines changed

src/buckify.rs

Lines changed: 84 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ use crate::buck::RuleRef;
4545
use crate::buck::RustBinary;
4646
use crate::buck::RustCommon;
4747
use crate::buck::RustLibrary;
48-
use crate::buck::Selectable;
4948
use crate::buck::StringOrPath;
5049
use crate::buck::SubtargetOrPath;
5150
use crate::buck::Visibility;
@@ -539,31 +538,6 @@ fn generate_target_rules<'scope>(
539538
// Per platform rule bits
540539
let mut perplat: BTreeMap<PlatformName, PlatformRustCommon> = BTreeMap::new();
541540

542-
unzip_platform(
543-
config,
544-
&mut base,
545-
&mut perplat,
546-
|rule, (flags, flags_select)| {
547-
log::debug!(
548-
"pkg {} target {}: adding flags {:?} and select {:?}",
549-
pkg,
550-
tgt.name,
551-
flags,
552-
flags_select
553-
);
554-
rule.rustc_flags.common.extend(flags);
555-
flags_select.into_iter().for_each(|(k, v)| {
556-
rule.rustc_flags
557-
.selects
558-
.entry(k)
559-
.or_insert(BTreeSet::new())
560-
.extend(v);
561-
});
562-
},
563-
fixups.compute_cmdline(),
564-
)
565-
.context("rustc_flags")?;
566-
567541
if matches!(config.vendor, VendorConfig::Source(_)) || matches!(pkg.source, Source::Local) {
568542
unzip_platform(
569543
config,
@@ -590,28 +564,6 @@ fn generate_target_rules<'scope>(
590564
.insert(BuckPath(PathBuf::from(http_archive_target)));
591565
}
592566

593-
unzip_platform(
594-
config,
595-
&mut base,
596-
&mut perplat,
597-
|rule, ()| {
598-
log::debug!(
599-
"pkg {} target {}: adding OUT_DIR for gen_srcs",
600-
pkg,
601-
tgt.name,
602-
);
603-
rule.env.unwrap_mut().insert(
604-
"OUT_DIR".to_owned(),
605-
StringOrPath::String(format!(
606-
"$(location :{}[out_dir])",
607-
fixups.buildscript_genrule_name(),
608-
)),
609-
);
610-
},
611-
fixups.compute_gen_srcs(),
612-
)
613-
.context("OUT_DIR for gen_srcs")?;
614-
615567
unzip_platform(
616568
config,
617569
&mut base,
@@ -646,18 +598,6 @@ fn generate_target_rules<'scope>(
646598
)
647599
.context("features")?;
648600

649-
unzip_platform(
650-
config,
651-
&mut base,
652-
&mut perplat,
653-
|rule, env| {
654-
log::debug!("pkg {} target {}: adding env {:?}", pkg, tgt.name, env);
655-
rule.env.unwrap_mut().extend(env);
656-
},
657-
fixups.compute_env()?,
658-
)
659-
.context("env")?;
660-
661601
// Compute set of dependencies any rule we generate here will need. They will only
662602
// be emitted if we actually emit some rules below.
663603
let mut dep_pkgs = Vec::new();
@@ -746,6 +686,90 @@ fn generate_target_rules<'scope>(
746686
}
747687
}
748688

689+
// If this is a build script, we only apply fixups pertaining to srcs
690+
// (extra_srcs), mapped_srcs (overlay), and features. Return early before
691+
// applying any other fixups (such as rustc_flags, env, or link_style).
692+
// Every other fixup handled after this point should only apply to the
693+
// library or binary, not a build script.
694+
if tgt.crate_bin() && tgt.kind_custom_build() {
695+
let buildscript = RustBinary {
696+
common: RustCommon {
697+
common: Common {
698+
name: Name(format!("{}-{}", pkg, tgt.name)),
699+
visibility: Visibility::Private,
700+
licenses: Default::default(),
701+
compatible_with: vec![],
702+
},
703+
krate: tgt.name.replace('-', "_"),
704+
crate_root: BuckPath(crate_root),
705+
edition,
706+
base,
707+
platform: perplat,
708+
},
709+
};
710+
let rules = fixups.emit_buildscript_rules(buildscript, config, manifest_dir_subtarget)?;
711+
return Ok((rules, dep_pkgs));
712+
}
713+
714+
unzip_platform(
715+
config,
716+
&mut base,
717+
&mut perplat,
718+
|rule, (flags, flags_select)| {
719+
log::debug!(
720+
"pkg {} target {}: adding flags {:?} and select {:?}",
721+
pkg,
722+
tgt.name,
723+
flags,
724+
flags_select
725+
);
726+
rule.rustc_flags.common.extend(flags);
727+
flags_select.into_iter().for_each(|(k, v)| {
728+
rule.rustc_flags
729+
.selects
730+
.entry(k)
731+
.or_insert(BTreeSet::new())
732+
.extend(v);
733+
});
734+
},
735+
fixups.compute_cmdline(),
736+
)
737+
.context("rustc_flags")?;
738+
739+
unzip_platform(
740+
config,
741+
&mut base,
742+
&mut perplat,
743+
|rule, ()| {
744+
log::debug!(
745+
"pkg {} target {}: adding OUT_DIR for gen_srcs",
746+
pkg,
747+
tgt.name,
748+
);
749+
rule.env.unwrap_mut().insert(
750+
"OUT_DIR".to_owned(),
751+
StringOrPath::String(format!(
752+
"$(location :{}[out_dir])",
753+
fixups.buildscript_genrule_name(),
754+
)),
755+
);
756+
},
757+
fixups.compute_gen_srcs(),
758+
)
759+
.context("OUT_DIR for gen_srcs")?;
760+
761+
unzip_platform(
762+
config,
763+
&mut base,
764+
&mut perplat,
765+
|rule, env| {
766+
log::debug!("pkg {} target {}: adding env {:?}", pkg, tgt.name, env);
767+
rule.env.unwrap_mut().extend(env);
768+
},
769+
fixups.compute_env()?,
770+
)
771+
.context("env")?;
772+
749773
// "link_style" only really applies to binaries, so maintain separate binary base & perplat
750774
let mut bin_base = base.clone();
751775
let mut bin_perplat = perplat.clone();
@@ -877,30 +901,6 @@ fn generate_target_rules<'scope>(
877901
dep_pkgs.push((pkg, TargetReq::BuildScript));
878902

879903
rules
880-
} else if tgt.crate_bin() && tgt.kind_custom_build() {
881-
// Build script
882-
let buildscript = RustBinary {
883-
common: RustCommon {
884-
common: Common {
885-
name: Name(format!("{}-{}", pkg, tgt.name)),
886-
visibility: Visibility::Private,
887-
licenses: Default::default(),
888-
compatible_with: vec![],
889-
},
890-
krate: tgt.name.replace('-', "_"),
891-
crate_root: BuckPath(crate_root),
892-
edition,
893-
base: PlatformRustCommon {
894-
// don't use fixed ones because it will be a cyclic dependency
895-
rustc_flags: Default::default(),
896-
env: Selectable::default(),
897-
link_style: None,
898-
..base
899-
},
900-
platform: bin_perplat,
901-
},
902-
};
903-
fixups.emit_buildscript_rules(buildscript, config, manifest_dir_subtarget)?
904904
} else if tgt.kind_bin() && tgt.crate_bin() {
905905
let mut rules = vec![];
906906
let actual = Name(format!("{}-{}", index.private_rule_name(pkg), tgt.name));

0 commit comments

Comments
 (0)