nextest-rs/nextest

`cargo nextest run --workspace` fails with DLL missing if a macro lib exists

Closed this issue · 14 comments

PS C:\src\rust-test> cargo nextest run --workspace
warning: virtual workspace defaulting to `resolver = "1"` despite one or more workspace members being on edition 2021 which implies `resolver = "2"`
note: to keep the current resolver, specify `workspace.resolver = "1"` in the workspace root's manifest
note: to use the edition 2021 resolver, specify `workspace.resolver = "2"` in the workspace root's manifest
note: for more details see https://doc.rust-lang.org/cargo/reference/resolver.html#resolver-versions
warning: virtual workspace defaulting to `resolver = "1"` despite one or more workspace members being on edition 2021 which implies `resolver = "2"`
note: to keep the current resolver, specify `workspace.resolver = "1"` in the workspace root's manifest
note: to use the edition 2021 resolver, specify `workspace.resolver = "2"` in the workspace root's manifest
note: for more details see https://doc.rust-lang.org/cargo/reference/resolver.html#resolver-versions
   Compiling proc-macro2 v1.0.82
   Compiling unicode-ident v1.0.12
   Compiling rust-test v0.1.0 (C:\src\rust-test\hello_world)
   Compiling quote v1.0.36
   Compiling syn v2.0.61
   Compiling hello_macro v0.1.0 (C:\src\rust-test\hello_macro)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 2.82s
error: creating test list failed

Caused by:
  for `hello_macro`, command `'C:\src\rust-test\target\debug\deps\hello_macro-621f285b4f25d626.exe' --list --format terse` exited with code 0xc0000135: The specified module could not be found. (os error 126)
--- stdout:

--- stderr:

---

I am using the pre-built cargo-next 0.9.70 from https://get.nexte.st/latest/windows. I am using the latest cargo 1.78.0.

PS C:\src\rust-test> cargo nextest --version
cargo-nextest-nextest 0.9.70
PS C:\src\rust-test> cargo --version
cargo 1.78.0 (54d8815d0 2024-03-26)

Note that this issue doesn't exist on cargo 1.70.0 with the same cargo-nextest binary.

This can be reproduced by the following file structure:

PS C:\src\rust-test> tree /f
Folder PATH listing for volume ???
Volume serial number is EC09-0909
C:.
│   .gitignore
│   Cargo.lock
│   Cargo.toml
│
├───hello_macro
│   │   Cargo.toml
│   │
│   └───src
│           lib.rs
│
└───hello_world
    │   Cargo.toml
    │
    └───src
            main.rs

Cargo.toml

PS C:\src\rust-test> cat Cargo.toml
[workspace]

members = [
    "hello_world",
    "hello_macro",
]

hello_macro/Cargo.toml

PS C:\src\rust-test> cat .\hello_macro\Cargo.toml
[package]
name = "hello_macro"
version = "0.1.0"
edition = "2021"

[lib]
proc-macro = true

[dependencies]
syn = { version = "2", features = ["extra-traits"] }
quote = "1.0"
proc-macro2 = "1.0"

hello_macro/src/lib.rs is an empty file.

hello_world/Cargo.toml

PS C:\src\rust-test> cat .\hello_world\Cargo.toml
[package]
name = "rust-test"
version = "0.1.0"
edition = "2021"

[dependencies]

hello_world/src/main.rs

PS C:\src\rust-test> cat .\hello_world\src\main.rs
fn main() {
    println!("Hello, world!");
}

cargo nextest run --workspace seems to want to execute the C:\src\rust-test\target\debug\deps\hello_macro-621f285b4f25d626.exe binary which depends on std-49e3d1aefc00cc02.dll installed under $RUSTUP_HOME/toolchains/stable-x86_64-pc-windows-msvc/bin, but is not available in the DLL search path.

Thanks for the issue.

Are you using rustup-installed cargo? I believe this is the same as or similar to #267. We should definitely fix this at some point, but this hasn't been a problem at my workplace so I don't think I'll personally get to it any time soon.

This comment captures the scope of work that needs to be done: #267 (comment) -- as mentioned there, I'd estimate around 16-24 hours of work for a motivated contributor. I'd love it if you were interested in fixing this!

Meanwhile you can try adding the directory where the standard library DLL exists to your PATH, and see if that works. (On macOS the env var is DYLD_LIBRARY_PATH, and on Linux/others the env var is LD_LIBRARY_PATH.)

Are you using rustup-installed cargo?

Yes, exactly.

I'd love it if you were interested in fixing this!

Let me check the details and decide whether I will have time to come up with a patch. I only have Windows and Linux machines, so probably can't test if things will break for Mac.

Actually I can't even run test for nextest

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

        FAIL [  16.921s] integration-tests::integration test_run_after_build

--- STDOUT:              integration-tests::integration test_run_after_build ---

running 1 test
test test_run_after_build ... FAILED

failures:

failures:
    test_run_after_build

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 16 filtered out; finished in 16.78s


--- STDERR:              integration-tests::integration test_run_after_build ---
thread 'test_run_after_build' panicked at integration-tests\tests\integration\main.rs:273:5:
assertion `left == right` failed: correct exit code for command
command: "D:\\nextest\\target\\debug/cargo-nextest-dup.exe" "nextest" "--manifest-path" "C:\\Users\\kaiyili\\AppData\\Local\\Temp\\nextest-fixturekxLaQD\\src\\Cargo.toml" "run" "--binaries-metadata" "C:\\Users\\kaiyili\\AppData\\Local\\Temp\\nextest-fixturekxLaQD\\src\\target\\binaries_metadata.json"
exit code: Some(104)
--- stdout ---


--- stderr ---
info: experimental features enabled: setup-scripts
error: creating test list failed

Caused by:
  for `cdylib-example`, command `'C:\Users\kaiyili\AppData\Local\Temp\nextest-fixturekxLaQD\src\target\debug\deps\cdylib_example-81217361b5632431.exe' --list --format terse` exited with code 0xc0000135: The specified module could not be found. (os error 126)
--- stdout:

--- stderr:

---



  left: Some(104)
 right: Some(100)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

------------

Let me downgrade my cargo binary first.

The following patch will fix the issue for me on my machine:

PS C:\src\nextest> git diff
diff --git a/nextest-runner/src/list/test_list.rs b/nextest-runner/src/list/test_list.rs
index 8ae64c76..54ee1ad9 100644
--- a/nextest-runner/src/list/test_list.rs
+++ b/nextest-runner/src/list/test_list.rs
@@ -478,6 +478,7 @@ impl<'g> TestList<'g> {
             updated_dylib_path.push("/usr/local/lib".into());
             updated_dylib_path.push("/usr/lib".into());
         }
+        updated_dylib_path.push(r#"C:\src\rustup\toolchains\stable-x86_64-pc-windows-msvc\bin"#.into());

         std::env::join_paths(updated_dylib_path)
             .map_err(move |error| CreateTestListError::dylib_join_paths(new_paths, error))

so I would be happy to create the patch.

However, I am confused about #267 (comment), and am wondering if you can answer making it clear to me?

  1. The missing DLL(std-49e3d1aefc00cc02.dll) is under the $(rustc --print sysroot)/bin folder, not the $(rustc --print target-libdir) folder. I am wondering if they are actually different bugs?
  2. In 3.b it mentions that names of libraries found at these paths (all files other than the ones ending in .rlib) (use just the file name) need to be stored. I assume only the folder name needs to be stored instead of the actual library files?
  3. In 4. it mentions that "While creating archives, serialize them to a path e.g. target/nextest/_libdir", what does creating archive mean? "Serializing them" means copy the library files(*.DLL/*.so) to the folder or does it mean we store the path in a file?
  4. In 5.a it says "This will need to obey target-dir remapping.". Does it refer to rust-lang/rfcs#3371? I am thinking that we are just adding to an environment variable to extend the paths where the dynamic linker searches for libraries, so I don't quite understand how a different target directory will influence that.
  5. I actually don't quite understand 5.b, but it may become clear once I figure out other bits.

Thanks.

After going over the code base, my plan is:

  1. add the path of $(rustc --print sysroot) to BuildPlatforms
  2. create a new BuildPlatformsSummary to replace PlatformSummary, which includes both the old PlatformSummary and the new sysroot path fields. (not sure about the best practice here to maintain the backwards compatibility for old archive format)
  3. in RustBuildMeta::<TestListState>::dylib_paths, we add the new self.build_platforms.sysroot / "bin" path to the result list.

To initialize the new BuildPlatforms::sysroot field, BuildPlatforms::new will be modified, and BuildPlatforms::from_summary will be added:

  • The signature of BuildPlatforms::new will be changed to pub fn new(reader: impl io::BufRead, target: Option<TargetTriple>) -> Result<Self, UnknownHostPlatform>. The caller(tests or BaseApp::new) is responsible to provide the stdout of the rustc --print sysroot --target XXX. The test provides a fake string.
  • BuildPlatforms::from_summary should just deserialize from BuildPlatformsSummary. And RustBuildMeta<State>::from_summary will replace the BuildPlatforms::new call with this new BuildPlatforms::from_summary call.

For BaseApp::new,

  • discover_build_platforms will be changed to only discover the TargetTriple(strictly speaking Option<TargetTriple>), we change the function name to discover_target_triple.
  • the command rustc --print sysroot --target XXX is assembled, executed, and the stdout will be passed to BuildPlatforms::new with the TargetTriple.
  • The XXX in the --target XXX is filled based on the TargetTriple. If TargetTriple is None, we don't attach the --target argument at all.

In addition, we also use the same technique used in cargo_path to find the rustc path.

WDYT?

This problem appeared recently in GitHub Actions CI and I still cannot figure out why it happened. Downgrading/pinning nextest does not help, downgrading Rust from 1.78 to 1.77 also does not help. It worked with Rust 1.77 recently, about week or two ago, and there have been no nextest releases for the last two weeks. Pinning CI runner to windows-2019 instead of windows-latest also did not help.

Wonder why this problem started to appear now.

@06393993 thank you for digging in! Your plan looks really solid and pretty close to what I'd have done.

create a new BuildPlatformsSummary to replace PlatformSummary, which includes both the old PlatformSummary and the new sysroot path fields. (not sure about the best practice here to maintain the backwards compatibility for old archive format)

I wouldn't worry too much about it -- we don't really guarantee cross-version compat for archives.

This seems to be caused by a change in a default in rustup 1.27.1 rust-lang/rustup#3703 changed the default of the env var RUSTUP_WINDOWS_PATH_ADD_BIN to "false" where it was true previously. If I add RUSTUP_WINDOWS_PATH_ADD_BIN=1 to my environment, nextest works fine.

@cwfitzgerald thanks, good catch! That would definitely cause this.

I think this should likely be fixed in nextest. The plan @06393993 described should address this.

Why is libstd getting dynamically linked? Proc macros statically link libstd under normal circumstances.

Why is libstd getting dynamically linked? Proc macros statically link libstd under normal circumstances.

I think that is true of proc macros, but not of test binaries for proc-macro crates.

Going to finish this up and post about it in #267.

cargo-nextest 0.9.72 should include the fix for this -- it'll be out shortly, so please try it out!