Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
06393993 opened this issue May 9, 2024 · 14 comments · Fixed by #1499, wez/wezterm#5444 or vectordotdev/vector#20572

Comments

@06393993
Copy link
Contributor

06393993 commented May 9, 2024

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!");
}
@06393993
Copy link
Contributor Author

06393993 commented May 9, 2024

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.

@sunshowers
Copy link
Member

sunshowers commented May 9, 2024

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.)

@06393993
Copy link
Contributor Author

06393993 commented May 9, 2024

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.

@06393993
Copy link
Contributor Author

06393993 commented May 9, 2024

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.

@06393993
Copy link
Contributor Author

06393993 commented May 9, 2024

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 RFC: Templating CARGO_TARGET_DIR to make it the parent of all target directories 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.

@06393993
Copy link
Contributor Author

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?

@link2xt
Copy link

link2xt commented May 12, 2024

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.

@sunshowers
Copy link
Member

@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.

@cwfitzgerald
Copy link

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.

@sunshowers
Copy link
Member

@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.

link2xt added a commit to deltachat/deltachat-core-rust that referenced this issue May 13, 2024
link2xt added a commit to deltachat/deltachat-core-rust that referenced this issue May 13, 2024
link2xt added a commit to deltachat/deltachat-core-rust that referenced this issue May 13, 2024
@bjorn3
Copy link

bjorn3 commented May 13, 2024

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

@sunshowers
Copy link
Member

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.

EliahKagan added a commit to EliahKagan/gitoxide that referenced this issue May 17, 2024
This sets the `RUSTUP_WINDOWS_PATH_ADD_BIN` environment variable in
the multi-platform test job, to work around
nextest-rs/nextest#1493 which is keeping
the test runner from working on Windows.

See also ruffle-rs/ruffle#16342, which gave
the idea for this, and various other projects that have made such
changes, linked inhttps://github.com/nextest-rs/nextest/issues/1493.
EliahKagan added a commit to EliahKagan/gitoxide that referenced this issue May 17, 2024
This sets the `RUSTUP_WINDOWS_PATH_ADD_BIN` environment variable in
the multi-platform test job, to work around
nextest-rs/nextest#1493 which is keeping
the test runner from working on Windows.

See also ruffle-rs/ruffle#16342, which gave
the idea for this, and various other projects that have made such
changes, linked in nextest-rs/nextest#1493.
06393993 added a commit to 06393993/nextest that referenced this issue May 18, 2024
... to WA nextest-rs#1493 for the "Test with latest nextest release" step on Windows temporarily.
06393993 added a commit to 06393993/nextest that referenced this issue May 18, 2024
... to WA nextest-rs#1493 for the "Test with latest nextest release" step on Windows temporarily.
06393993 added a commit to 06393993/nextest that referenced this issue May 18, 2024
... to WA nextest-rs#1493 for the "Test with latest nextest release" step on Windows temporarily.
zanieb pushed a commit to astral-sh/ruff that referenced this issue May 19, 2024
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

The recent issues with the windows CI seem to be caused by
nextest-rs/nextest#1493. With this
nextest-rs/nextest#1493 (comment)
as a fix.

(Let's see if it works)
bogdan2412 added a commit to bogdan2412/wezterm that referenced this issue May 20, 2024
It seems to be caused by nextest-rs/nextest#1493, with this comment
as a fix:
nextest-rs/nextest#1493 (comment)
bogdan2412 added a commit to bogdan2412/wezterm that referenced this issue May 20, 2024
It's currently failing when running tests with errors of the form:

> error: creating test list failed
>
> Caused by:
>   for `wezterm-config-derive`, command
>   `'D:\a\wezterm\wezterm\target\debug\deps\wezterm_config_derive-f1c7f0f2de220b6e.exe'
>   --list --format terse` exited with code 0xc0000135: The specified
>   module could not be found. (os error 126)

This seems to have been caused by nextest-rs/nextest#1493, with this comment
as a fix: nextest-rs/nextest#1493 (comment)
bogdan2412 added a commit to bogdan2412/wezterm that referenced this issue May 20, 2024
It's currently failing when running tests with errors of the form:

> error: creating test list failed
>
> Caused by:
>   for `wezterm-config-derive`, command
>   `'D:\a\wezterm\wezterm\target\debug\deps\wezterm_config_derive-f1c7f0f2de220b6e.exe'
>   --list --format terse` exited with code 0xc0000135: The specified
>   module could not be found. (os error 126)

This seems to have been caused by nextest-rs/nextest#1493, with this comment
as a fix: nextest-rs/nextest#1493 (comment)
wez pushed a commit to wez/wezterm that referenced this issue May 20, 2024
It's currently failing when running tests with errors of the form:

> error: creating test list failed
>
> Caused by:
>   for `wezterm-config-derive`, command
>   `'D:\a\wezterm\wezterm\target\debug\deps\wezterm_config_derive-f1c7f0f2de220b6e.exe'
>   --list --format terse` exited with code 0xc0000135: The specified
>   module could not be found. (os error 126)

This seems to have been caused by nextest-rs/nextest#1493, with this comment
as a fix: nextest-rs/nextest#1493 (comment)
jszwedko added a commit to vectordotdev/vector that referenced this issue May 21, 2024
Workaround for issue in nextest caused by a change in rustup:
nextest-rs/nextest#1493

Fixes:

```
error: creating test list failed

Caused by:
  for `vector-config-macros`, command
  `'C:\a\vector\vector\target\debug\deps\vector_config_macros-4fc11cf764d99dff.exe' --list --format
  terse` exited with code 0xc0000135: The specified module could not be found. (os error 126)
```

Signed-off-by: Jesse Szwedko <jesse.szwedko@datadoghq.com>
github-merge-queue bot pushed a commit to vectordotdev/vector that referenced this issue May 21, 2024
Workaround for issue in nextest caused by a change in rustup:
nextest-rs/nextest#1493

Fixes:

```
error: creating test list failed

Caused by:
  for `vector-config-macros`, command
  `'C:\a\vector\vector\target\debug\deps\vector_config_macros-4fc11cf764d99dff.exe' --list --format
  terse` exited with code 0xc0000135: The specified module could not be found. (os error 126)
```

Signed-off-by: Jesse Szwedko <jesse.szwedko@datadoghq.com>
sunshowers pushed a commit that referenced this issue May 22, 2024
... to WA #1493 for the "Test with latest nextest release" step on Windows temporarily.
@sunshowers
Copy link
Member

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

@sunshowers
Copy link
Member

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

jszwedko added a commit to vectordotdev/vector that referenced this issue May 28, 2024
Should resolve nextest-rs/nextest#1493

Signed-off-by: Jesse Szwedko <jesse.szwedko@datadoghq.com>
github-merge-queue bot pushed a commit to vectordotdev/vector that referenced this issue May 28, 2024
Should resolve nextest-rs/nextest#1493

Signed-off-by: Jesse Szwedko <jesse.szwedko@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants