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

macos: dyld: Library not loaded: @rpath/libtest-77ee8c29c330e4a3.dylib #267

Closed
steveej opened this issue Jun 9, 2022 · 16 comments
Closed
Labels
A-reuse-build Issues around support for reusing builds enhancement New feature or request help wanted Extra attention is needed

Comments

@steveej
Copy link
Contributor

steveej commented Jun 9, 2022

Note from maintainer: Here's an outline of the work required for this. Please dive in if you're interested!


we're having issues running cargo nextest in a nix-shell on a macos github actions runner.
we run the same steps on an ubuntu runner in parallel which succeeds, so this seems to be macos specific.

it may or may not have something to do that the tested source code is under /tmp, which on this macos runner has an implicit /private/ prefix.

some references for context:

(...)
dyld: Library not loaded: @rpath/libtest-77ee8c29c330e4a3.dylib
  Referenced from: /private/tmp/holochain_repo/target/fast-test/deps/hdk_derive-73ec051829ad694a
  Reason: image not found
error: creating test list failed
Caused by:
  for `hdk_derive::proc-macro/hdk_derive`, running command `/private/tmp/holochain_repo/target/fast-test/deps/hdk_derive-73ec051829ad694a --list --format terse` failed
Caused by:
  command ["/private/tmp/holochain_repo/target/fast-test/deps/hdk_derive-73ec051829ad694a", "--list", "--format", "terse"] exited with code <signal 6>
Error: Process completed with exit code 104.

please let me know if i can provide further debug output to help us understand what's happening.

@sunshowers
Copy link
Member

Thanks. This looks like an issue with a dynamic library provided by Rust, presumably created at build time, not being found at runtime.

How is cargo installed? I'm guessing a rustup based installation sets particular environment variables that a nix install doesn't.

@sunshowers
Copy link
Member

OK, I know what's going on—we need to grab the value of "rustc --print sysroot" and set the sysroot path as part of LD_FALLBACK_LIBRARY_PATH properly. Will fix in the next couple of days or you're welcome to fix it as well.

@sunshowers
Copy link
Member

This method needs to be updated to also grab the sysroot:

pub(crate) fn create_dylib_path(

@steveej
Copy link
Contributor Author

steveej commented Jun 9, 2022

thanks for looking into this @sunshowers.

How is cargo installed?

it's provided by a nix-shell derivation which makes use of functions from here: https://github.com/oxalica/rust-overlay/.

I'm guessing a rustup based installation sets particular environment variables that a nix install doesn't.

this is possible and could be easily resolved in the nix-shell setup as well.

i'm puzzled by the difference in linux and macos. the nix-shell input is identical for both environments. are you aware of any rust specific platform differences that influences what cargo-nextest sees?

@sunshowers
Copy link
Member

I'm not sure what nix is doing, but it does look like they had to work around a very similar issue on just macOS here: https://github.com/oxalica/rust-overlay/blob/44801306a2aa0e9aaa47588d615ce6df4acf18c6/mk-component-set.nix#L89

@sunshowers
Copy link
Member

This is exactly what we would have to fix, except we'll use DYLD_FALLBACK_LIBRARY_PATH rather than adding the rpath to the binary.

@steveej
Copy link
Contributor Author

steveej commented Jun 9, 2022

looks like there's already a TODO for this in the codebase:

// TODO: rustc sysroot library path? is this even possible to get?

@sunshowers
Copy link
Member

Yeah that definitely looks like it.

I'm not sure how this would work with reused builds. We currently promise not to require rustc, but we may not be able to uphold that promise when dealing specifically with proc-macro tests.

@steveej
Copy link
Contributor Author

steveej commented Jun 9, 2022

This is exactly what we would have to fix, except we'll use DYLD_FALLBACK_LIBRARY_PATH rather than adding the rpath to the binary.

it seems like this isn't effective. i ran the following on the github macos runner and it still produced the same error:

Mac-1654809093762:holochain runner$ nix-shell --pure --run "env DYLD_FALLBACK_LIBRARY_PATH=$(rustc --print sysroot) hc-test-standard-nextest"
Using /Users/runner/work/holochain/holochain as target prefix...
(...)
+ cargo nextest run --test-threads=2 --workspace --exclude holochain --lib --tests --cargo-profile fast-test
    Finished fast-test [unoptimized + debuginfo] target(s) in 1.10s
dyld: Library not loaded: @rpath/libtest-77ee8c29c330e4a3.dylib
  Referenced from: /Users/runner/work/holochain/holochain/target/fast-test/deps/hdk_derive-73ec051829ad694a
  Reason: image not found
error: creating test list failed

Caused by:
  for `hdk_derive::proc-macro/hdk_derive`, running command `/Users/runner/work/holochain/holochain/target/fast-test/deps/hdk_d73ec051829ad694a --list --format terse` failed

Caused by:
  command ["/Users/runner/work/holochain/holochain/target/fast-test/deps/hdk_derive-73ec051829ad694a", "--list", "--format", "] exited with code <signal 6>

on the positive side, the path looks healthy:

$ nix-shell --pure --run 'ls -lha $(rustc --print sysroot)/lib/libtest-77ee8c29c330e4a3.dylib'
Using /Users/runner/work/holochain/holochain as target prefix...
lrwxr-xr-x 1 root wheel 111 Jan  1  1970 /nix/store/qbci06vgxhgxzhjyi9rflgiikr27dmhy-rust-default-1.60.0/lib/libtest-77ee8c29c330e4a3.dylib -> /nix/store/88lhjdgxdkdwdichnji1r79lp25rdrwb-rustc-1.60.0-x86_64-apple-darwin/lib/libtest-77ee8c29c330e4a3.dylib

i'll look into whether the nextest-runner does not forward this environment variable to the test binary, which would be an explanation. or maybe we do need to modify the rpath after all.


EDIT:

whops, i used " instead of ' in the first command. so the shell expanded the command before entering the nix-shell and setting the env var. i repeated the command properly and it still fails in the same way so the above is still relevant.

@sunshowers
Copy link
Member

sunshowers commented Jun 9, 2022

You want DYLD_FALLBACK_LIBRARY_PATH=$(rustc --print sysroot)/lib

@steveej
Copy link
Contributor Author

steveej commented Jun 10, 2022

You want DYLD_FALLBACK_LIBRARY_PATH=$(rustc --print sysroot)/lib

with this correct path the workaround described above works indeed, thank you 🙌

now, i think it's worth putting a best-effort solution into cargo-nextest the way you described.

trying to think about how we could solve this without shelling out and thus depending on rustc.
in the special case when the runner is executed using cargo nextest, could query cargo for this info? OTOH, the result would still depend on rustc with its sysroot being installed so the dependency is there regardless.

@sunshowers sunshowers added the enhancement New feature or request label Jun 11, 2022
@sunshowers
Copy link
Member

sunshowers commented Jun 11, 2022

Agreed about supporting it properly by updating the sysroot, though given the complexity and edge case nature here (only occurs for proc-macro tests + not using cargo from rustup) I'm going to prioritize it downwards because a workaround exists for now.

However, if someone would like to send a PR then that would be really appreciated as well!

I'll outline the main things that need to be done, for both someone who would like to contribute a PR and also for my future self:

  1. Grab the host and target rust libdirs (turns out a better workaround is $(rustc --print target-libdir)) run with and without the --target option for host and target. Note while running rustc that it should look at $RUSTC first, just like our Cargo executions look at $CARGO first.
  2. Add them to the dylib path -- should dedup the dylib paths by using an IndexSet maybe.
  3. Add the following to RustBuildMeta and its serialized form RustBuildMetaSummary:
    a. The name of the target
    b. Names of libraries found at these paths (all files other than the ones ending in .rlib) (use just the file name).
  4. While creating archives, serialize them to a path e.g. target/nextest/_libdir.
  5. While reusing builds, use the libdirs in this order:
    a. First, prefer the libdir at the target/nextest path if the files within it exist. (This will need to obey target-dir remapping.)
    b. Then, if those aren't found, invoke rustc to get the libdir. There are three cases here:
    1. rustc is found. In this case, check to see if the file names exist. If they don't or the names are different, warn that some tests may not be able to be run.
    2. rustc exited with a non-zero status code. This should warn same as above.
    3. rustc is not found. In this case, produce a message with info!() saying that some tests may not be successful at running.

For testing:

  • Unit tests: e.g. for deduping paths -- we likely also want to build this in a way that rustc's output is mockable.

  • Integration tests: in CI, we basically want to run the fixture tests while bypassing rustup's cargo. The way to do this is essentially to use:

    #!/bin/bash
    set -e -o pipefail
    pushd "$(git rev-parse --show-toplevel)" 
    
    export CARGO="$(rustup which cargo)"
    # Required for dylib linking
    export RUSTFLAGS="-C prefer-dynamic"
    
    target/debug/cargo-nextest nextest run --manifest-path fixtures/nextest-tests/Cargo.toml --workspace
    # https://docs.rs/nextest-metadata/latest/nextest_metadata/enum.NextestExitCode.html#associatedconstant.TEST_RUN_FAILED
    if [[ $? -eq 100 ]]; then
        echo "test run failed as expected"
    else
        echo "expected exit code 100 (test run failed), found $?"
        exit 1
    fi

And a similar thing should be done for reused builds.

This is hard to do within the current nextest suite because it runs under cargo (we want to minimize the things that can add a libdir to the dylib path), so it should should be a script within the scripts directory so it can be tested outside of CI as well.

This is likely 2-3 days of work that I think can be done by a contributor willing to dive into this!

@sunshowers sunshowers added help wanted Extra attention is needed A-reuse-build Issues around support for reusing builds labels Jun 11, 2022
@gakonst
Copy link

gakonst commented Jan 3, 2023

@DaniPopes if you'd be down to do this, go for it

@NobodyXu
Copy link
Contributor

@sunshowers I've also encountered this when running:

export CARGO='cargo'

cargo-nextest nextest run --workspace  --target x86_64-unknown-linux-gnu --no-default-features --features rustls,fancy-with-backtrace,zstd-thin,log_release_max_level_debug,pkg-config

in our CI.

However, if I run:

export CARGO='cargo'

cargo nextest run --workspace  --target x86_64-unknown-linux-gnu --no-default-features --features rustls,fancy-with-backtrace,zstd-thin,log_release_max_level_debug,pkg-config

then it would work fine, presumably because cargo overwrites environment variable CARGO.

Also, I'm now trying to get cargo-nextest to work with cargo-zigbuild for cross compilation.
When cross compiling, we set export CARGO=cargo-zigbuild to use cargo-zigbuild instead of cargo for building binaries.

rvolosatovs added a commit to rvolosatovs/wasmCloud that referenced this issue May 31, 2023
See nextest-rs/nextest#267

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
rvolosatovs added a commit to rvolosatovs/wasmCloud that referenced this issue May 31, 2023
`proc-macro` crate tests require additional config

See nextest-rs/nextest#267

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
rvolosatovs added a commit to rvolosatovs/wasmCloud that referenced this issue May 31, 2023
`proc-macro` crate tests require additional config

See nextest-rs/nextest#267

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
rvolosatovs added a commit to wasmCloud/wasmCloud that referenced this issue May 31, 2023
`proc-macro` crate tests require additional config

See nextest-rs/nextest#267

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
9999years added a commit to 9999years/crane that referenced this issue Aug 30, 2023
Prevents this error:

```
(...)
dyld: Library not loaded: @rpath/libtest-77ee8c29c330e4a3.dylib
  Referenced from: /private/tmp/holochain_repo/target/fast-test/deps/hdk_derive-73ec051829ad694a
  Reason: image not found
error: creating test list failed
Caused by:
  for `hdk_derive::proc-macro/hdk_derive`, running command `/private/tmp/holochain_repo/target/fast-test/deps/hdk_derive-73ec051829ad694a --list --format terse` failed
Caused by:
  command ["/private/tmp/holochain_repo/target/fast-test/deps/hdk_derive-73ec051829ad694a", "--list", "--format", "terse"] exited with code <signal 6>
Error: Process completed with exit code 104.
```

nextest-rs/nextest#267
ipetkov pushed a commit to ipetkov/crane that referenced this issue Aug 30, 2023
Prevents this error:

```
(...)
dyld: Library not loaded: @rpath/libtest-77ee8c29c330e4a3.dylib
  Referenced from: /private/tmp/holochain_repo/target/fast-test/deps/hdk_derive-73ec051829ad694a
  Reason: image not found
error: creating test list failed
Caused by:
  for `hdk_derive::proc-macro/hdk_derive`, running command `/private/tmp/holochain_repo/target/fast-test/deps/hdk_derive-73ec051829ad694a --list --format terse` failed
Caused by:
  command ["/private/tmp/holochain_repo/target/fast-test/deps/hdk_derive-73ec051829ad694a", "--list", "--format", "terse"] exited with code <signal 6>
Error: Process completed with exit code 104.
```

nextest-rs/nextest#267
GauravBholaris added a commit to GauravBholaris/crane that referenced this issue Jan 31, 2024
Prevents this error:

```
(...)
dyld: Library not loaded: @rpath/libtest-77ee8c29c330e4a3.dylib
  Referenced from: /private/tmp/holochain_repo/target/fast-test/deps/hdk_derive-73ec051829ad694a
  Reason: image not found
error: creating test list failed
Caused by:
  for `hdk_derive::proc-macro/hdk_derive`, running command `/private/tmp/holochain_repo/target/fast-test/deps/hdk_derive-73ec051829ad694a --list --format terse` failed
Caused by:
  command ["/private/tmp/holochain_repo/target/fast-test/deps/hdk_derive-73ec051829ad694a", "--list", "--format", "terse"] exited with code <signal 6>
Error: Process completed with exit code 104.
```

nextest-rs/nextest#267
@sunshowers
Copy link
Member

sunshowers commented May 23, 2024

All right, so as the flurry of activity above suggests, we ended up being forced to fix this due to a rustup behavior change on Windows.

That should address this class of issues comprehensively, in all situations including archives (we now include libstd in archives -- unconditionally for now, but we could get smarter and/or add config knobs in the future).

After #1514, nextest will no longer depend on rustup. It'll continue to depend on cargo, of course, until #842 is resolved.

@sunshowers
Copy link
Member

cargo-nextest 0.9.72 is out with a fix for this. Try it out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-reuse-build Issues around support for reusing builds enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants