Correctly record multiple native dirs per package #2763
Conversation
|
r? @wycats (rust_highfive has picked a reviewer for you, use r? to override) |
|
Added a second commit with a fix for #2765, since it depended on the other commit here. See the issue and the commit for details. |
| search_path.push(dir.clone()); | ||
|
|
||
| // Add -L arguments, after stripping off prefixes like "native=" or "framework=". | ||
| let search_path_prefix = Regex::new("^([:alpha:]+=)").unwrap(); |
There was a problem hiding this comment.
Instead of using Regex can this just use .splitn('=', 2)?
|
Thanks! Could you be sure to add a test for both bugs as well? |
This fixes a bug when a package's build script outputs multiple library search paths. Because Compilation::native_dirs is a `HashMap<PackageId, PathBuf>` it can only store one path per package. Currently if there are multiple paths, all but the last will be inserted and then overwritten. The key from this map is never used anyway, so this fixes the bug by changing it from a HashMap to a HashSet.
Done.
Done. I also made the prefix search more specific, so it exactly matches rustc's behavior. This is a bit annoying, but it's the only way to get correct results in the presence of directories with Also changed |
Correctly record multiple native dirs per package This fixes a bug when a package's build script outputs multiple library search paths. Because Compilation::native_dirs is a `HashMap<PackageId, PathBuf>` it can only store one path per package. Currently if there are multiple paths, all but the last will be inserted and then overwritten. The key from this map is never used anyway, so this fixes the bug by changing it from a HashMap to a HashSet.
|
☀️ Test successful - cargo-cross-linux, cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32, cargo-win-msvc-64 |
So this is a bit weird to do, and it's somewhat of a hack. However since rust-lang/cargo#2763, cargo will now put any `-L` arguments onto `DYLD_LIBRARY_PATH` when using `cargo test` or `cargo run`. For installations using Homebrew, this means that we will try to stick `/usr/local/lib` in `DYLD_LIBRARY_PATH`. If there's anything in there that conflicts with system libraries (such as `libJPEG`) everything blows up. It is certainly possible for this configuration issue to affect platforms other than Mac, but this is not meant to be a catch-all solution for platform configuration problems. For Mac users though, this is a very common setup that is easy to resolve. After this commit, the directory will be something like `/usr/local/Cellar/postgresql/9.5.3/lib` and not `/usr/local/lib`.
This fixes a bug when a package's build script outputs multiple library search
paths. Because Compilation::native_dirs is a
HashMap<PackageId, PathBuf>itcan only store one path per package. Currently if there are multiple paths,
all but the last will be inserted and then overwritten.
The key from this map is never used anyway, so this fixes the bug by changing
it from a HashMap to a HashSet.