fix dynamic search path for build scripts #3974

mcgoo · 2017-04-28T18:33:52Z

rust-highfive · 2017-04-28T18:34:05Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

alexcrichton · 2017-04-28T19:47:24Z

Thanks! Could you refactor this to share an implementation with compilation.rs as well?

mcgoo · 2017-04-28T21:07:35Z

This no longer passes tests. Do you know where to get the correct target directory path from?

let root_output = cx.layout(Kind::Target).dest().to_path_buf();

doesn't seem to be the right thing.

(I assume when you said you wanted to use the same code in both cases you meant that the build script case also should have the "don't add paths outside the target dir" logic.)

mcgoo · 2017-04-29T00:03:05Z

cargo test --test build-script gives a nice focussed example.

alexcrichton · 2017-04-29T17:37:16Z

Oh so that bug I believe is because the prefix needs to be with Kind::Host, not Kind::Target.

Could you actually avoid though making layout public and just use the path to target as the prefix to test? (that's good enough)

mcgoo · 2017-04-29T17:57:54Z

FYI, it does not work with Kind::Host either, I think because we are relative to 'unit'.

Where do I get the path to target?

alexcrichton · 2017-04-29T18:48:03Z

Hm looking back the usage of .dest() should work. To get the path to target you can just add a helper method to Context to keep layout private.

Looking at the appveyor failure the test involved is legitimately breaking after this change was applied. Not only is this picking up the native= stripping but it's also picking up the filtering of everything outside the deps dir. I think the test will need to be updated to get it to work after this is implemented.

mcgoo · 2017-04-29T21:39:21Z

How does this look?

I fixed the test by putting the project that builds the dylib and the project that uses it into a workspace.

I'm kinda shocked that the issue #3366 dylib path filtering is not breaking people's builds. It seems like it should at least give a diagnostic, so I left a debug!() in there.

mcgoo · 2017-05-01T04:24:33Z

I'm not sure what is going on, but I get test failures on cargo master that appear related to the failures in this build. I pulled the known good appveyor commit mentioned below (bors building a nightly) and tried to build it and it failed. The appveyor shows it succeeding. I deleted ~/.cargo ~/.multirust and ~/.rustup before starting. I built in a fresh clone of the cargo tree. I'm not sure if there is some other state or config file that you can think of that I need to clean up on my machine or if this is a real issue.

OS is Windows 10 1703.15063.138

Thoughts?

#!/bin/bash

# c.f.
# https://ci.appveyor.com/project/rust-lang-libs/cargo/build/1.0.1715/job/27uuclr522xnv88c
#

set -xe 

# rm -rf ~/.cargo
/c/Users/jim/Downloads/rustup-init.exe --version
/c/Users/jim/Downloads/rustup-init.exe -y --default-host x86_64-pc-windows-msvc --default-toolchain nightly-2017-03-03 
git checkout -qf 7cfeec1aaac3d1909af8476f66c547619eceb640
got log -n1
rm -rf target
git status
/c/Users/jim/.cargo/bin/rustc -V
/c/Users/jim/.cargo/bin/cargo --version
/c/Users/jim/.cargo/bin/cargo build --target x86_64-pc-windows-msvc --manifest-path /c/Users/jim/src/cargo//Cargo.toml --release
CFG_DISABLE_CROSS_TESTS=1 /c/Users/jim/.cargo/bin/cargo test --target x86_64-pc-windows-msvc --manifest-path /c/Users/jim/src/cargo//Cargo.toml --release    -- --quiet || true

# also
CFG_DISABLE_CROSS_TESTS=1 /c/Users/jim/.cargo/bin/cargo test --target x86_64-pc-windows-msvc --manifest-path /c/Users/jim/src/cargo//Cargo.toml --release --test plugins   -- --quiet

rustup-init 1.2.0 (70faf07 2017-04-08)

  nightly-2017-03-03 installed - rustc 1.17.0-nightly (c0b7112ba 2017-03-02)

HEAD detached at 7cfeec1a
commit 7cfeec1aaac3d1909af8476f66c547619eceb640
Merge: 6b6dc8e1 5d76dd23
Author: bors <bors@rust-lang.org>
Date:   Sun Apr 30 08:23:23 2017 +0000

    Auto merge of #3980 - Keruspe:master, r=alexcrichton

    update libgit2-sys for libressl compat

Untracked files:
  (use "git add <file>..." to include in what will be committed)

	test-plugins.out
	test-plugins.sh

nothing added to commit but untracked files present (use "git add" to track)
rustc 1.17.0-nightly (c0b7112ba 2017-03-02)
cargo 0.18.0-nightly (19ea423 2017-03-02)

running 37 tests
.....................................
test result: ok. 37 passed; 0 failed; 0 ignored; 0 measured


running 60 tests
............................................................
test result: ok. 60 passed; 0 failed; 0 ignored; 0 measured


running 23 tests
.......F...............
failures:

---- bench_dylib stdout ----
	running `C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\release\cargo.exe bench -v`
thread 'bench_dylib' panicked at '
Expected: execs
    but: exited with exit code: 3221225781
--- stdout

--- stderr
   Compiling bar v0.0.1 (file:///C:/Users/jim/src/cargo/target/x86_64-pc-windows-msvc/cit/t5/foo/bar)
     Running `rustc --crate-name bar bar\src\lib.rs --crate-type dylib --emit=dep-info,link -C prefer-dynamic -C opt-level=3 -C metadata=e10cd8f5ac7d88d8 --out-dir C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps -L dependency=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps`
   Compiling foo v0.0.1 (file:///C:/Users/jim/src/cargo/target/x86_64-pc-windows-msvc/cit/t5/foo)
     Running `rustc --crate-name foo src\lib.rs --crate-type dylib --emit=dep-info,link -C opt-level=3 -C metadata=70483dbfb6c289a9 --out-dir C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps -L dependency=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps --extern bar=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps\bar.dll`
     Running `rustc --crate-name foo src\lib.rs --emit=dep-info,link -C opt-level=3 --test -C metadata=296c91290cac987f -C extra-filename=-296c91290cac987f --out-dir C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps -L dependency=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps --extern bar=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps\bar.dll`
     Running `rustc --crate-name bench benches\bench.rs --emit=dep-info,link -C opt-level=3 --test -C metadata=a75f89a68520beea -C extra-filename=-a75f89a68520beea --out-dir C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps -L dependency=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps --extern bar=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps\bar.dll --extern foo=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps\foo.dll`
    Finished release [optimized] target(s) in 1.85 secs
     Running `C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t5\foo\target\release\deps\foo-296c91290cac987f.exe --bench`
error: bench failed
', C:\Users\jim\.cargo\registry\src\gh.neting.cc-1ecc6299db9ec823\hamcrest-0.1.1\src\core.rs:31
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    bench_dylib

test result: FAILED. 22 passed; 1 failed; 0 ignored; 0 measured


running 7 tests
.F..F..
failures:

---- doctest_a_plugin stdout ----
	running `C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\release\cargo.exe test -v`
thread 'doctest_a_plugin' panicked at '
Expected: execs
    but: exited with exit code: 3221225781
--- stdout

--- stderr
   Compiling bar v0.0.1 (file:///C:/Users/jim/src/cargo/target/x86_64-pc-windows-msvc/cit/t0/foo/bar)
     Running `rustc --crate-name bar bar\src\lib.rs --crate-type dylib --emit=dep-info,link -C prefer-dynamic -C debuginfo=2 -C metadata=a6b23aa348e72191 --out-dir C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t0\foo\target\debug\deps -L dependency=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t0\foo\target\debug\deps`
   Compiling foo v0.0.1 (file:///C:/Users/jim/src/cargo/target/x86_64-pc-windows-msvc/cit/t0/foo)
     Running `rustc --crate-name foo src\lib.rs --emit=dep-info,link -C debuginfo=2 --test -C metadata=87be52aa16cc53e8 -C extra-filename=-87be52aa16cc53e8 --out-dir C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t0\foo\target\debug\deps -L dependency=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t0\foo\target\debug\deps --extern bar=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t0\foo\target\debug\deps\bar.dll`
     Running `rustc --crate-name foo src\lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 -C metadata=cf582169413865dc -C extra-filename=-cf582169413865dc --out-dir C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t0\foo\target\debug\deps -L dependency=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t0\foo\target\debug\deps --extern bar=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t0\foo\target\debug\deps\bar.dll`
warning: unused `#[macro_use]` import
 --> src\lib.rs:2:13
  |
2 |             #[macro_use]
  |             ^^^^^^^^^^^^
  |
  = note: #[warn(unused_imports)] on by default

warning: unused `#[macro_use]` import
 --> src\lib.rs:2:13
  |
2 |             #[macro_use]
  |             ^^^^^^^^^^^^
  |
  = note: #[warn(unused_imports)] on by default

    Finished dev [unoptimized + debuginfo] target(s) in 0.84 secs
     Running `C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t0\foo\target\debug\deps\foo-87be52aa16cc53e8.exe`
error: test failed, to rerun pass '--lib'
', C:\Users\jim\.cargo\registry\src\gh.neting.cc-1ecc6299db9ec823\hamcrest-0.1.1\src\core.rs:31
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- plugin_integration stdout ----
	running `C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\release\cargo.exe test -v`
thread 'plugin_integration' panicked at '
Expected: execs
    but: exited with exit code: 3221225781
--- stdout

--- stderr
   Compiling foo v0.0.1 (file:///C:/Users/jim/src/cargo/target/x86_64-pc-windows-msvc/cit/t1/foo)
     Running `rustc --crate-name build_script_build build.rs --crate-type bin --emit=dep-info,link -C debuginfo=2 -C metadata=5363063d55d7c4d5 -C extra-filename=-5363063d55d7c4d5 --out-dir C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t1\foo\target\debug\build\foo-5363063d55d7c4d5 -L dependency=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t1\foo\target\debug\deps`
     Running `C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t1\foo\target\debug\build\foo-5363063d55d7c4d5\build-script-build`
     Running `rustc --crate-name foo src\lib.rs --emit=dep-info,link -C prefer-dynamic -C debuginfo=2 --test -C metadata=187507dd4dcf5f01 -C extra-filename=-187507dd4dcf5f01 --out-dir C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t1\foo\target\debug\deps -L dependency=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t1\foo\target\debug\deps`
     Running `rustc --crate-name foo src\lib.rs --crate-type dylib --emit=dep-info,link -C prefer-dynamic -C debuginfo=2 -C metadata=565f675dce05b118 --out-dir C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t1\foo\target\debug\deps -L dependency=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t1\foo\target\debug\deps`
     Running `rustc --crate-name it_works tests\it_works.rs --emit=dep-info,link -C debuginfo=2 --test -C metadata=a3a459c991a81bc4 -C extra-filename=-a3a459c991a81bc4 --out-dir C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t1\foo\target\debug\deps -L dependency=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t1\foo\target\debug\deps --extern foo=C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t1\foo\target\debug\deps\foo.dll`
    Finished dev [unoptimized + debuginfo] target(s) in 1.33 secs
     Running `C:\Users\jim\src\cargo\target\x86_64-pc-windows-msvc\cit\t1\foo\target\debug\deps\foo-187507dd4dcf5f01.exe`
error: test failed, to rerun pass '--lib'
', C:\Users\jim\.cargo\registry\src\gh.neting.cc-1ecc6299db9ec823\hamcrest-0.1.1\src\core.rs:31


failures:
    doctest_a_plugin
    plugin_integration

test result: FAILED. 5 passed; 2 failed; 0 ignored; 0 measured

mcgoo · 2017-05-01T15:27:01Z

I thought it might be because I am building using cargo rather than configure/make, but it appears that using cargo works fine in a modified CI build.

https://ci.appveyor.com/project/mcgoo/cargo/build/1.0.3/job/xrgse4388abpa9ng

The tests that fail are not the same test that fails in the CI for this PR so this may just be a strange coincidence.

Any advice on what I need to do to get the tests working much appreciated.

mcgoo · 2017-05-01T19:27:39Z

Ok, this looks good to me now and the tests pass on the CI server at least.

alexcrichton · 2017-05-02T15:03:22Z

Looks great to me, thanks! Want to squash the commits and I'll r+?

bors · 2017-05-08T18:31:21Z

☔ The latest upstream changes (presumably #4008) made this pull request unmergeable. Please resolve the merge conflicts.

alexcrichton · 2017-05-08T19:05:00Z

Ah sorry if I missed this! Want to resolve the conflicts? Feel free to ping me when the PR is ready as unfortunately github doesn't send out notifications when it's force-pushed :(

Make dynamic library search path handling for build scripts mirror the behaviour for cargo run etc. -L paths are taken and stripped of the native= and similar prefixes and added to the dynamic library search path if they are inside the target dir. Resolves rust-lang#3957

mcgoo · 2017-05-08T20:10:24Z

ping @alexcrichton

alexcrichton · 2017-05-08T21:11:28Z

@bors: r+

bors · 2017-05-08T21:11:29Z

📌 Commit b8158cf has been approved by alexcrichton

bors · 2017-05-08T21:11:34Z

⌛ Testing commit b8158cf with merge 9bf9bdd...

fix dynamic search path for build scripts fixes #3957

bors · 2017-05-08T22:38:57Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 9bf9bdd to master...

Fix add_plugin_deps-related tests. These tests were modified in #3974 in such a way that they stopped testing the `add_plugin_deps` code path. The tests can't be directly reverted because #3651 changed it so that dylib paths must be within the root output directory. I compromised by just copying the dylib into `$OUT_DIR`. Closes #6318.

rust-highfive assigned brson Apr 28, 2017

mcgoo changed the title ~~Build rs native path~~ fix dynamic search path for build scripts May 2, 2017

mcgoo force-pushed the build_rs_native_path branch from a236e99 to 125fdd8 Compare May 2, 2017 15:35

mcgoo force-pushed the build_rs_native_path branch 2 times, most recently from ddbf1a7 to cb348bd Compare May 8, 2017 19:20

mcgoo force-pushed the build_rs_native_path branch from cb348bd to b8158cf Compare May 8, 2017 19:26

bors added a commit that referenced this pull request May 8, 2017

Auto merge of #3974 - mcgoo:build_rs_native_path, r=alexcrichton

9bf9bdd

fix dynamic search path for build scripts fixes #3957

bors merged commit b8158cf into rust-lang:master May 8, 2017

bors mentioned this pull request May 8, 2017

Allow cargo:rustc-env in build scripts #3929

Merged

mcgoo deleted the build_rs_native_path branch May 8, 2017 23:25

This was referenced Nov 15, 2018

Build script dylib test issue #6318

Closed

Fix add_plugin_deps-related tests. #6327

Merged

ehuss added this to the 1.19.0 milestone Feb 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix dynamic search path for build scripts #3974

fix dynamic search path for build scripts #3974

mcgoo commented Apr 28, 2017

rust-highfive commented Apr 28, 2017

alexcrichton commented Apr 28, 2017

mcgoo commented Apr 28, 2017

mcgoo commented Apr 29, 2017

alexcrichton commented Apr 29, 2017

mcgoo commented Apr 29, 2017

alexcrichton commented Apr 29, 2017

mcgoo commented Apr 29, 2017

mcgoo commented May 1, 2017

mcgoo commented May 1, 2017

mcgoo commented May 1, 2017

alexcrichton commented May 2, 2017

bors commented May 8, 2017

alexcrichton commented May 8, 2017

mcgoo commented May 8, 2017

alexcrichton commented May 8, 2017

bors commented May 8, 2017

bors commented May 8, 2017

bors commented May 8, 2017

fix dynamic search path for build scripts #3974

fix dynamic search path for build scripts #3974

Conversation

mcgoo commented Apr 28, 2017

rust-highfive commented Apr 28, 2017

alexcrichton commented Apr 28, 2017

mcgoo commented Apr 28, 2017

mcgoo commented Apr 29, 2017

alexcrichton commented Apr 29, 2017

mcgoo commented Apr 29, 2017

alexcrichton commented Apr 29, 2017

mcgoo commented Apr 29, 2017

mcgoo commented May 1, 2017

mcgoo commented May 1, 2017

mcgoo commented May 1, 2017

alexcrichton commented May 2, 2017

bors commented May 8, 2017

alexcrichton commented May 8, 2017

mcgoo commented May 8, 2017

alexcrichton commented May 8, 2017

bors commented May 8, 2017

bors commented May 8, 2017

bors commented May 8, 2017