Skip to content

Disallow duplicate cxx library names #94

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.

Sign up for GitHub

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

Draft
cormacrelf wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Disallow duplicate cxx library names #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f3990c
impl Display for PlatformExpr
cormacrelf Jan 5, 2026
e04135c
Add error when you name two cxx_libraries the same and one clobbers t…
cormacrelf Jan 5, 2026
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
111 changes: 81 additions & 30 deletions src/fixups.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ use crate::index::ResolvedDep;
use crate::path::normalize_path;
use crate::path::normalized_extend_path;
use crate::path::relative_path;
use crate::platform::PlatformExpr;
use crate::platform::PlatformName;
use crate::subtarget::Subtarget;

Expand Down Expand Up @@ -372,6 +373,32 @@ impl<'meta> Fixups<'meta> {
}))
}

fn check_duplicate_cxx_targets<'a>(
&self,
emitted_target_names: &mut BTreeMap<String, &'a PlatformExpr>,
target_name: &str,
platform_expr: &'a PlatformExpr,
) -> anyhow::Result<()> {
if let Some(first_platform_expr) =
emitted_target_names.insert(target_name.to_owned(), platform_expr)
{
// Assume only the base fixup is the trivially true.
// You are allowed to override the base fixup. Otherwise the user has written two
// [[cfg(xxx).cxx_library]] name = "yyy" and expected us to merge these and put
// select()s in a single cxx_library(name = "yyy"). We don't do that.
if *first_platform_expr != PlatformExpr::default() {
bail!(
"Multiple cxx_library targets named '{}' in package {}. The first was emitted for platform {}, the second for platform {}. Cxx library names must be unique per-package, except to override the base fixup with a cfg().",
target_name,
self.package,
first_platform_expr,
platform_expr,
);
}
}
Ok(())
}

/// Return buildscript-related rules
/// The rules may be platform specific, but they're emitted unconditionally. (The
/// dependencies referencing them are conditional).
Expand Down Expand Up @@ -434,8 +461,13 @@ impl<'meta> Fixups<'meta> {
for &platform_name in compatible_platforms {
for fixup in self.configs(platform_name) {
if library_fixups.insert(&raw const *fixup) {
cxx_library.extend(&fixup.cxx_library);
prebuilt_cxx_library.extend(&fixup.prebuilt_cxx_library);
cxx_library.extend(fixup.cxx_library.iter().map(|cxx| (&fixup.platform, cxx)));
prebuilt_cxx_library.extend(
fixup
.prebuilt_cxx_library
.iter()
.map(|prebuilt| (&fixup.platform, prebuilt)),
);
}
}

Expand All @@ -444,27 +476,33 @@ impl<'meta> Fixups<'meta> {
}
}

let mut emitted_target_names = BTreeMap::new();

// Emit a C++ library build rule (elsewhere - add a dependency to it)
for CxxLibraryFixup {
name,
srcs,
headers,
exported_headers,
public,
include_paths,
fixup_include_paths,
exclude,
compiler_flags,
preprocessor_flags,
header_namespace,
deps,
compatible_with,
target_compatible_with,
preferred_linkage,
undefined_symbols,
..
} in cxx_library
for (
platform_expr,
CxxLibraryFixup {
name,
srcs,
headers,
exported_headers,
public,
include_paths,
fixup_include_paths,
exclude,
compiler_flags,
preprocessor_flags,
header_namespace,
deps,
compatible_with,
target_compatible_with,
preferred_linkage,
undefined_symbols,
..
},
) in cxx_library
{
self.check_duplicate_cxx_targets(&mut emitted_target_names, name, platform_expr)?;
let cxx_library_target = if self.config.buck.split {
res.push(Rule::CxxLibrary(CxxLibrary {
owner: PackageVersion {
Expand Down Expand Up @@ -658,21 +696,29 @@ impl<'meta> Fixups<'meta> {
}

// Emit a prebuilt C++ library rule for each static library (elsewhere - add dependencies to them)
for PrebuiltCxxLibraryFixup {
name,
static_libs,
public,
compatible_with,
target_compatible_with,
preferred_linkage,
..
} in prebuilt_cxx_library
for (
platform_expr,
PrebuiltCxxLibraryFixup {
name,
static_libs,
public,
compatible_with,
target_compatible_with,
preferred_linkage,
..
},
) in prebuilt_cxx_library
{
let static_lib_globs = Globs::new(static_libs, NO_EXCLUDE);
for static_lib in static_lib_globs.walk(self.manifest_dir) {
let static_lib_file_name = static_lib.file_name().unwrap().to_string_lossy();
let prebuilt_cxx_library_target = if self.config.buck.split {
let target_name = Name(format!("{}-{}", name, static_lib_file_name));
self.check_duplicate_cxx_targets(
&mut emitted_target_names,
&target_name.0,
platform_expr,
)?;
res.push(Rule::PrebuiltCxxLibrary(PrebuiltCxxLibrary {
owner: PackageVersion {
name: self.package.name.clone(),
Expand Down Expand Up @@ -718,6 +764,11 @@ impl<'meta> Fixups<'meta> {
name,
static_lib.file_name().unwrap().to_string_lossy(),
));
self.check_duplicate_cxx_targets(
&mut emitted_target_names,
&target_name.0,
platform_expr,
)?;
res.push(Rule::PrebuiltCxxLibrary(PrebuiltCxxLibrary {
owner: PackageVersion {
name: self.package.name.clone(),
Expand Down
51 changes: 51 additions & 0 deletions src/platform.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,57 @@ impl Default for PlatformExpr {
}
}

impl Display for PlatformExpr {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
return Cfg(self).fmt(f);

struct Cfg<'a>(&'a PlatformExpr);
struct Expr<'a>(&'a PlatformExpr);
impl Display for Cfg<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "cfg({})", Expr(self.0))
}
}
impl Display for Expr<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.0 {
PlatformExpr::Any(preds) => {
write!(f, "any(")?;
for (i, pred) in preds.iter().enumerate() {
if i > 0 {
write!(f, ", ")?;
}
write!(f, "{}", Expr(pred))?;
}
write!(f, ")")
}
PlatformExpr::All(preds) => {
write!(f, "all(")?;
for (i, pred) in preds.iter().enumerate() {
if i > 0 {
write!(f, ", ")?;
}
write!(f, "{}", Expr(pred))?;
}
write!(f, ")")
}
PlatformExpr::Not(pred) => {
write!(f, "not({})", Expr(pred))
}
PlatformExpr::Value { key, value } => write!(f, "{} = {:?}", key, value),
PlatformExpr::Bool { key } => write!(f, "{}", key),
PlatformExpr::Version(version_req) => {
write!(f, "version = \"{}\"", version_req)
}
PlatformExpr::Target(target) => write!(f, "{}", *target),
PlatformExpr::Unix => write!(f, "unix"),
PlatformExpr::Windows => write!(f, "windows"),
}
}
}
}
}

#[derive(Debug, Clone)]
pub enum PredicateParseError {
TrailingJunk(String),
Expand Down