volta
volta copied to clipboard
volta-cli
Check volta shims for reachability during tool install
@charlespierce i think i addressed all comments. initially i thought it would make sense to make volta hard-crash with exitcode 1 if the PATH can't be resolved to the shim (i think that would be useful when volta is used in CI), but when going back to this code i saw that only some error conditions were treated as fatal.
Sorry about the delay here, @untitaker – will see if we can get this reviewed and merged here shortly (at the start of the new year, if nothing else)!
Ah, I messed up the merge commit; I will re-create it and force push the correct version in a bit!
@untitaker that would be great! I fixed up the one build issue I saw, so you should be unblocked. Let us know when it's fixed and we'll get this merged. Thanks again, and thanks for your patience!
Running this locally:
RUST_BACKTRACE=1 cargo test --all --features mock-network
gives me this output for most tests:
thread 'volta_run::inherited_pnpm' panicked at /Users/untitaker/.cargo/registry/src/index.crates.io-6f17d22bba15001f/mockito-0.31.1/src/lib.rs:1184:72:
called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }
stack backtrace:
0: rust_begin_unwind
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/panicking.rs:645:5
1: core::panicking::panic_fmt
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/panicking.rs:72:14
2: core::result::unwrap_failed
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/result.rs:1653:5
3: core::result::Result<T,E>::unwrap
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/result.rs:1077:23
4: mockito::Mock::with_body_from_file
at /Users/untitaker/.cargo/registry/src/index.crates.io-6f17d22bba15001f/mockito-0.31.1/src/lib.rs:1184:52
5: acceptance::support::sandbox::SandboxBuilder::distro_mock
at ./tests/acceptance/support/sandbox.rs:457:25
6: acceptance::support::sandbox::SandboxBuilder::distro_mocks
at ./tests/acceptance/support/sandbox.rs:470:20
7: acceptance::volta_run::inherited_pnpm
at ./tests/acceptance/volta_run.rs:475:13
8: acceptance::volta_run::inherited_pnpm::{{closure}}
at ./tests/acceptance/volta_run.rs:474:20
9: core::ops::function::FnOnce::call_once
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/ops/function.rs:250:5
10: core::ops::function::FnOnce::call_once
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
@chriskrycho can you or somebody else help me figure out what is broken with my devenv? I can't figure out the broken tests before that. I am following the instructions in CONTRIBUTING
Huh. That should have been fixed by some of the work @rwjblue and I did a little while back, but it's possible that we missed something there or that I did the merge above wrong or that we caused a regression. The issue is that the test mock is asking for a path named "tests/fixtures/node-v10.99.1040-darwin-arm64.tar.gz" (and so on for the other failing tests), and there is no darwin-arm64 fixture for Node v10.99, because that would be a nonsense thing to ask for. What I am trying to figure out now is whether that error came from, because it should certainly be entirely unrelated to your changes here! My current best bet is that I messed up merging into your branch somehow!
there is no darwin-arm64 fixture for Node v10.99, because that would be a nonsense thing to ask for.
darwin-arm64 does make sense to me, that's just any new MacBook. So I suppose the devenv does not work on any M1 or M2 macbook? Not sure
I switched to an x86 laptop for this PR, and everything worked out. Fixed the remaining test failure, so I think this PR should be ready to merge.
Aggghhh. I am merging main in and merging this PR! Your note got lost in my notifications somehow, @untitaker – so sorry about the delay.
Ugh, and now there’s a failure in the Windows CI. I am going to kick it off again in case it was spurious. I will check back in on it tomorrow.
Oh, and to answer this bit:
darwin-arm64 does make sense to me, that's just any new MacBook. So I suppose the devenv does not work on any M1 or M2 macbook? Not sure
The issue there was that the Node version it’s asking for, Node v10.99, does not have an Apple Silicon build (that shipped several versions later than that). The build works locally on my M1 machine as well, so that wasn't the issue.
If you or @charlespierce or someone with a Windows box is able to get to the bottom of the most recent test failure here, I still would love for us to get this merged.
@chriskrycho I want to be able to debug this in CI but I am unable to do so due to approved workflows. I think I have found the issue.
If that doesn't fix it, I can also just disable the test on Windows. It's too much fuzz for such a simple change.
Just triggered CI again, so hopefully it’ll be green. Assuming we get to ✅ we’ll merge it!