-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Closed
Description
The test build_script_with_dynamic_native_dependency does not seem to be working as it was originally intended. I was going to fix it myself, but there are too many changes that I don't understand. There are two primary issues:
- This line in the test points to a non-existing path:
cargo/tests/testsuite/build_script.rs
Line 1440 in b3d0b2e
println!("cargo:rustc-link-search=native={}/target/debug/deps", - These lines do not have any test coverage (removing
add_plugin_depsdoes not cause any errors):cargo/src/cargo/core/compiler/mod.rs
Lines 517 to 542 in b3d0b2e
// For all plugin dependencies, add their -L paths (now calculated and // present in `state`) to the dynamic library load path for the command to // execute. fn add_plugin_deps( rustc: &mut ProcessBuilder, build_state: &BuildMap, build_scripts: &BuildScripts, root_output: &PathBuf, ) -> CargoResult<()> { let var = util::dylib_path_envvar(); let search_path = rustc.get_env(var).unwrap_or_default(); let mut search_path = env::split_paths(&search_path).collect::<Vec<_>>(); for id in build_scripts.plugins.iter() { let key = (id.clone(), Kind::Host); let output = build_state .get(&key) .ok_or_else(|| internal(format!("couldn't find libs for plugin dep {}", id)))?; search_path.append(&mut filter_dynamic_search_path( output.library_paths.iter(), root_output, )); } let search_path = join_paths(&search_path, var)?; rustc.env(var, &search_path); Ok(()) }
The history for this test:
- The test and
add_plugin_depsadded in Remove the deprecated build command infrastructure #1195. - The test dylib builder was changed to add
plugin = truein Don't link build scripts dynamically #1960. ← This one I particularly don't understand. - Test was changed to use a workspace in fix dynamic search path for build scripts #3974 which resulted in the build script emitting a path that does not exist. That PR also changed it so that linker paths must be prefixes of the
target/debugdirectory, so you can't just switch it back to a non-workspace.
It's not clear to me how #3974 was supposed to work. It's also not clear if add_plugin_deps is even needed anymore. I would think all plugin dylibs should already live in the deps dir which is already included in the LD_LIBRARY_PATH, so I don't know why add_plugin_deps exists.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels