Skip to content

Commit

Permalink
Plumb preferred_linkage and undefined_symbols for cxx library fixups
Browse files Browse the repository at this point in the history
Summary:
`preferred_linkage = "static"` causes ODR violations when building rust code with advanced_unstable_linking under mode/dev. In order to properly handle this, we need to stop unconditionally passing static preferred_linkage to cxx_library targets in third-party/rust and start passing `undefined_symbols = True` instead.

In order to do this, I need to land this in a few parts:
1. This diff to add the new flag plumbing to reindeer.
2. The msdk bump build + regenerate third-party/rust/BUCK with preferred_linkage = static in all required fixups files.
3. Migrate from `preferred_linkage` to `undefined_symbols`.
4. Cleanup (probably can remove `preferred_linkage` if there are no usages after (3)).

Reviewed By: dtolnay

Differential Revision:
D59829306

Privacy Context Container: L1122763

fbshipit-source-id: 161dbbc59c143367350c6791aaaec3277258fb15
  • Loading branch information
capickett authored and facebook-github-bot committed Jul 16, 2024
1 parent 08a8dc6 commit 245f60f
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
9 changes: 8 additions & 1 deletion src/buck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,7 @@ pub struct CxxLibrary {
pub include_directories: Vec<SubtargetOrPath>,
pub deps: BTreeSet<RuleRef>,
pub preferred_linkage: Option<String>,
pub undefined_symbols: bool,
}

impl Serialize for CxxLibrary {
Expand All @@ -857,6 +858,7 @@ impl Serialize for CxxLibrary {
include_directories,
deps,
preferred_linkage,
undefined_symbols,
} = self;
let mut map = ser.serialize_map(None)?;
map.serialize_entry("name", name)?;
Expand Down Expand Up @@ -885,7 +887,9 @@ impl Serialize for CxxLibrary {
if !licenses.is_empty() {
map.serialize_entry("licenses", licenses)?;
}
map.serialize_entry("preferred_linkage", preferred_linkage)?;
if let Some(preferred_linkage) = preferred_linkage {
map.serialize_entry("preferred_linkage", preferred_linkage)?;
}
if !preprocessor_flags.is_empty()
|| include_directories
.iter()
Expand All @@ -899,6 +903,9 @@ impl Serialize for CxxLibrary {
},
)?;
}
if *undefined_symbols {
map.serialize_entry("undefined_symbols", undefined_symbols)?;
}
map.serialize_entry("visibility", visibility)?;
if !deps.is_empty() {
map.serialize_entry("deps", deps)?;
Expand Down
5 changes: 4 additions & 1 deletion src/fixups.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,8 @@ impl<'meta> Fixups<'meta> {
header_namespace,
deps,
compatible_with,
preferred_linkage,
undefined_symbols,
..
}) => {
let actual = Name(format!(
Expand Down Expand Up @@ -469,7 +471,8 @@ impl<'meta> Fixups<'meta> {
preprocessor_flags: preprocessor_flags.clone(),
header_namespace: header_namespace.clone(),
deps: deps.iter().cloned().map(RuleRef::new).collect(),
preferred_linkage: Some("static".to_string()),
preferred_linkage: preferred_linkage.clone(),
undefined_symbols: undefined_symbols.clone(),
};

res.push(Rule::CxxLibrary(rule));
Expand Down
6 changes: 6 additions & 0 deletions src/fixups/buildscript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ pub struct CxxLibraryFixup {
pub deps: Vec<String>,
#[serde(default)]
pub compatible_with: Vec<String>,
/// Cxx library preferred linkage (how dependents should link you)
pub preferred_linkage: Option<String>,
/// Whether to allow undefined symbols during compilation (e.g. when a rust library
/// and cxx library depend on each other for symbols)
#[serde(default)]
pub undefined_symbols: bool,
}

#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
Expand Down

0 comments on commit 245f60f

Please sign in to comment.