Skip to content

Commit 4ea4b07

Browse files
committed
fix: resolve overlinking false positives for staging outputs
When a package output inherits from a staging cache, the staging cache's host dependencies aren't installed in the prefix during overlinking checks. This means libraries like libz.so.1 can't be attributed to zlib, even though zlib is a run dependency via inherited run_exports. Fix: at staging build time, capture a LibraryNameMap (filename to package mapping) from PrefixInfo while conda-meta still exists. Store it in the staging cache metadata, thread it to BuildOutput, and use it as a fallback in perform_linking_checks when resolve_libraries can't find the library on disk. Fixes prefix-dev#2186 Supersedes prefix-dev#2210
1 parent 809e112 commit 4ea4b07

File tree

7 files changed

+283
-8
lines changed

7 files changed

+283
-8
lines changed

crates/rattler_build_core/src/build.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,12 @@ pub async fn run_build(
136136
// This will build or restore staging caches and return their dependencies/sources if inherited
137137
let staging_result = output.process_staging_caches(tool_configuration).await?;
138138

139-
// If we inherit from a staging cache, store its dependencies and sources
140-
if let Some((deps, sources)) = staging_result {
139+
// If we inherit from a staging cache, store its dependencies, sources, and
140+
// library name map for overlinking checks
141+
if let Some((deps, sources, library_name_map)) = staging_result {
141142
output.finalized_cache_dependencies = Some(deps);
142143
output.finalized_cache_sources = Some(sources);
144+
output.staging_library_name_map = Some(library_name_map);
143145
}
144146

145147
// Fetch sources for this output

crates/rattler_build_core/src/post_process/checks.rs

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,7 @@ pub fn perform_linking_checks(
528528
let system_libs = find_system_libs(output)?;
529529

530530
let prefix_info = PrefixInfo::from_prefix(output.prefix())?;
531+
let staging_lib_map = output.staging_library_name_map.as_ref();
531532

532533
let host_dso_packages = host_run_export_dso_packages(output, &prefix_info.package_to_nature);
533534
tracing::trace!("Host run_export DSO packages: {host_dso_packages:#?}",);
@@ -644,6 +645,26 @@ pub fn perform_linking_checks(
644645
);
645646
}
646647

648+
// Fallback: if the library couldn't be resolved on disk (e.g. from
649+
// a staging cache whose host deps are not installed), try to match
650+
// it by filename against the cached library name map.
651+
if let Some(lib_map) = staging_lib_map
652+
&& let Some(providing_package) = lib_map.find_package(lib)
653+
&& run_dependency_names.contains(&providing_package)
654+
{
655+
tracing::debug!(
656+
"Library {lib:?} matched to '{}' via staging library name map",
657+
providing_package.as_normalized()
658+
);
659+
link_info.linked_packages.push(LinkedPackage {
660+
name: lib.to_path_buf(),
661+
link_origin: LinkOrigin::ForeignPackage(
662+
providing_package.as_normalized().to_string(),
663+
),
664+
});
665+
continue;
666+
}
667+
647668
// Check if the library is one of the system libraries (i.e. comes from sysroot).
648669
if system_libs.allow.is_match(lib) && !system_libs.deny.is_match(lib) {
649670
link_info.linked_packages.push(LinkedPackage {
@@ -730,7 +751,21 @@ pub fn perform_linking_checks(
730751

731752
#[cfg(test)]
732753
mod tests {
754+
use std::collections::BTreeMap;
755+
use std::sync::{Arc, Mutex};
756+
757+
use rattler_conda_types::{MatchSpec, package::CondaArchiveType};
758+
use rattler_solve::{ChannelPriority, SolveStrategy};
759+
733760
use super::*;
761+
use crate::render::resolved_dependencies::{
762+
DependencyInfo, FinalizedDependencies, FinalizedRunDependencies, SourceDependency,
763+
};
764+
use crate::system_tools::SystemTools;
765+
use crate::types::{
766+
BuildConfiguration, BuildSummary, Directories, PackagingSettings,
767+
PlatformWithVirtualPackages,
768+
};
734769
use fs_err;
735770

736771
#[test]
@@ -1088,4 +1123,138 @@ mod tests {
10881123
assert!(result.is_err());
10891124
assert!(result.unwrap_err().to_string().contains("Failed to parse"));
10901125
}
1126+
1127+
/// Creates a minimal `Output` for testing `perform_linking_checks`.
1128+
/// Creates a minimal `Output` for testing `perform_linking_checks`.
1129+
///
1130+
/// The recipe is parsed from YAML. The remaining fields (`BuildConfiguration`,
1131+
/// `FinalizedDependencies`, etc.) are not part of the recipe format and must
1132+
/// be constructed manually.
1133+
fn create_test_output(
1134+
target_platform: Platform,
1135+
host_prefix: PathBuf,
1136+
build_prefix: PathBuf,
1137+
run_deps: Vec<&str>,
1138+
recipe_yaml: &str,
1139+
) -> crate::metadata::Output {
1140+
let recipe: rattler_build_recipe::Stage1Recipe =
1141+
serde_yaml::from_str(recipe_yaml).expect("failed to parse recipe YAML");
1142+
1143+
let pvp = PlatformWithVirtualPackages {
1144+
platform: target_platform,
1145+
virtual_packages: vec![],
1146+
};
1147+
1148+
let depends = run_deps
1149+
.into_iter()
1150+
.map(|name| {
1151+
DependencyInfo::Source(SourceDependency {
1152+
spec: MatchSpec::from_str(name, rattler_conda_types::ParseStrictness::Lenient)
1153+
.unwrap(),
1154+
})
1155+
})
1156+
.collect();
1157+
1158+
crate::metadata::Output {
1159+
recipe,
1160+
build_configuration: BuildConfiguration {
1161+
target_platform,
1162+
host_platform: pvp.clone(),
1163+
build_platform: pvp,
1164+
variant: BTreeMap::new(),
1165+
hash: rattler_build_recipe::stage1::HashInfo {
1166+
hash: "test".into(),
1167+
prefix: String::new(),
1168+
},
1169+
directories: Directories {
1170+
host_prefix,
1171+
build_prefix,
1172+
..Default::default()
1173+
},
1174+
channels: vec![],
1175+
channel_priority: ChannelPriority::Strict,
1176+
solve_strategy: SolveStrategy::Highest,
1177+
timestamp: chrono::Utc::now(),
1178+
subpackages: BTreeMap::new(),
1179+
packaging_settings: PackagingSettings {
1180+
archive_type: CondaArchiveType::Conda,
1181+
compression_level: 1,
1182+
},
1183+
store_recipe: false,
1184+
force_colors: false,
1185+
sandbox_config: None,
1186+
exclude_newer: None,
1187+
},
1188+
finalized_dependencies: Some(FinalizedDependencies {
1189+
build: None,
1190+
host: None,
1191+
run: FinalizedRunDependencies {
1192+
depends,
1193+
constraints: vec![],
1194+
run_exports: Default::default(),
1195+
},
1196+
}),
1197+
finalized_sources: None,
1198+
finalized_cache_dependencies: None,
1199+
finalized_cache_sources: None,
1200+
staging_library_name_map: None,
1201+
build_summary: Arc::new(Mutex::new(BuildSummary::default())),
1202+
system_tools: SystemTools::new("test", "0.0.0"),
1203+
extra_meta: None,
1204+
}
1205+
}
1206+
1207+
/// Simulates a staging output scenario: the binary (zlink) links against
1208+
/// libz.so.1, zlib IS in the run dependencies (via inherited run_exports),
1209+
/// but zlib is NOT installed in the host prefix (no conda-meta, no library
1210+
/// files). With the staging library name map providing the fallback
1211+
/// mapping, the overlinking check should pass.
1212+
#[test]
1213+
fn test_staging_overlinking() {
1214+
let test_data =
1215+
Path::new(env!("CARGO_MANIFEST_DIR")).join("../../test-data/binary_files");
1216+
1217+
let tmp = tempfile::tempdir().unwrap();
1218+
let tmp_prefix = tmp.path().join("tmp_prefix");
1219+
let host_prefix = tmp.path().join("host_prefix");
1220+
let build_prefix = tmp.path().join("build_prefix");
1221+
fs_err::create_dir_all(&tmp_prefix).unwrap();
1222+
fs_err::create_dir_all(&host_prefix).unwrap();
1223+
fs_err::create_dir_all(&build_prefix).unwrap();
1224+
1225+
let binary_dest = tmp_prefix.join("zlink");
1226+
fs_err::copy(test_data.join("zlink"), &binary_dest).unwrap();
1227+
1228+
let mut output = create_test_output(
1229+
Platform::Linux64,
1230+
host_prefix,
1231+
build_prefix,
1232+
vec!["zlib"],
1233+
r#"
1234+
package:
1235+
name: test-pkg
1236+
version: "1.0.0"
1237+
build:
1238+
dynamic_linking:
1239+
overlinking_behavior: error
1240+
missing_dso_allowlist:
1241+
- "libc*"
1242+
"#,
1243+
);
1244+
output.staging_library_name_map =
1245+
Some(crate::post_process::package_nature::LibraryNameMap {
1246+
library_to_package: [("libz.so.1".to_string(), "zlib".to_string())]
1247+
.into_iter()
1248+
.collect(),
1249+
});
1250+
1251+
let new_files: HashSet<PathBuf> = [binary_dest].into_iter().collect();
1252+
1253+
let result = perform_linking_checks(&output, &new_files, &tmp_prefix);
1254+
assert!(
1255+
result.is_ok(),
1256+
"Expected overlinking check to pass since zlib is a run dependency \
1257+
with a staging library name map, but got: {result:?}"
1258+
);
1259+
}
10911260
}

crates/rattler_build_core/src/post_process/package_nature.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,76 @@ impl PrefixInfo {
215215
}
216216
}
217217

218+
/// A mapping from shared library filenames to the package that provides them.
219+
///
220+
/// This is used as a fallback during overlinking checks when the staging
221+
/// cache's host dependencies are not physically installed in the prefix.
222+
/// Instead of requiring files on disk, this allows name-based attribution
223+
/// of libraries to packages.
224+
#[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)]
225+
pub struct LibraryNameMap {
226+
/// Maps library filenames (e.g. "libz.so.1", "libz.1.dylib") to the
227+
/// package name that provides them.
228+
pub library_to_package: HashMap<String, String>,
229+
}
230+
231+
impl LibraryNameMap {
232+
/// Build a `LibraryNameMap` from a `PrefixInfo` by extracting the
233+
/// filenames of all files that look like shared objects.
234+
pub(crate) fn from_prefix_info(prefix_info: &PrefixInfo) -> Self {
235+
let mut library_to_package = HashMap::new();
236+
237+
for (path, package_name) in &prefix_info.path_to_package {
238+
if is_dso(&path.path) {
239+
if let Some(file_name) = path.path.file_name() {
240+
library_to_package.insert(
241+
file_name.to_string_lossy().to_string(),
242+
package_name.as_normalized().to_string(),
243+
);
244+
}
245+
}
246+
}
247+
248+
Self { library_to_package }
249+
}
250+
251+
/// Look up a library path by extracting its filename and checking the map.
252+
/// Returns the package name if found.
253+
///
254+
/// Handles various library path forms:
255+
/// - Plain filenames: `libz.so.1`
256+
/// - macOS @rpath references: `@rpath/libz.1.dylib`
257+
/// - Full or relative paths: `lib/libz.so.1`
258+
pub fn find_package(&self, library: &Path) -> Option<PackageName> {
259+
let path_str = library.to_string_lossy();
260+
261+
// Strip @rpath/ or @loader_path/ prefixes (macOS)
262+
let stripped = path_str
263+
.strip_prefix("@rpath/")
264+
.or_else(|| path_str.strip_prefix("@loader_path/"))
265+
.unwrap_or(&path_str);
266+
267+
// Try the stripped path directly (handles plain filenames)
268+
if let Some(pkg) = self.library_to_package.get(stripped) {
269+
return PackageName::try_from(pkg.as_str()).ok();
270+
}
271+
272+
// Try just the filename component
273+
let file_name = Path::new(stripped)
274+
.file_name()?
275+
.to_string_lossy();
276+
277+
self.library_to_package
278+
.get(file_name.as_ref())
279+
.and_then(|n| PackageName::try_from(n.as_str()).ok())
280+
}
281+
282+
/// Returns true if the map is empty.
283+
pub fn is_empty(&self) -> bool {
284+
self.library_to_package.is_empty()
285+
}
286+
}
287+
218288
#[cfg(test)]
219289
mod tests {
220290
use super::*;

0 commit comments

Comments
 (0)