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

Cli will report "Couldn't find cargo" when it can't find powershell #314

Open
raspirin opened this issue Dec 11, 2023 · 17 comments
Open

Cli will report "Couldn't find cargo" when it can't find powershell #314

raspirin opened this issue Dec 11, 2023 · 17 comments
Labels
A-cli Area: command line interface C-bug Category: bug tribble-reported This issue was reported through Tribble.

Comments

@raspirin
Copy link

This issue is reporting a bug in the code of Perseus. Details of the scope will be available in issue labels.
The author described their issue as follows:

When cli can't find powershell, instead of reporting "Couldn't find powershell", the cli will complain about "Couldn't find cargo".

The steps to reproduce this issue are as follows:

On Windows and powershell not in the PATH, run perseus serve -w

A minimum reproducible example is available at <>.

  • Hydration-related: false
  • The author is willing to attempt a fix: false
Tribble internal data

dHJpYmJsZS1yZXBvcnRlZCxDLWJ1ZyxBLWNsaQ==

@github-actions github-actions bot added A-cli Area: command line interface C-bug Category: bug tribble-reported This issue was reported through Tribble. labels Dec 11, 2023
@lukechu10
Copy link
Contributor

What do you mean by can't find powershell? Are you running perseus serve from cmd? Normally perseus shouldn't need powershell to work.

Also what happens when you just run cargo from the same shell?

@arctic-hen7
Copy link
Member

Actually, Perseus does require Powershell! For most commands, because there can be all sorts of issues with PATH configurations that we've had in the past (mostly legacy from v0.1.0), we run them in a shell, which, on Windows, is powershell -command "<some-command>". Right now, there is no validation step to ensure the shell is available in any way, though I'd never thought this would be an issue, probably because I barely understand Powershell and Windows in general.

I'd be happy to add a preflight check that makes sure we can actually run commands at all, which is probably a good idea! @raspirin what shell do you regularly use? Right now, from the sounds of things, the CLI is hardcoded to not work for you...

@raspirin
Copy link
Author

Well, I'm not familiar with Windows and Powershell either. I use Fish on Linux.
This weird issue happened when I tried to use Perseus on my friend's computer. We indeed used Powershell there, but even though we typed powershell into a Powershell instance, the shell couldn't recognize the command. This is what I mean by "Couldn't find Powershell". We added the location of powershell (C:\Windows\System32\WindowsPowerShell\v1.0) into PATH and then the cli stopped complaining about "Couldn't find cargo".
Also, cargo works just fine with that weird shell and PATH configuration.

arctic-hen7 added a commit that referenced this issue Dec 16, 2023
@arctic-hen7
Copy link
Member

Huh, that is weird. That sounds like a configuration-specific issue somewhere in Windows, but I've just added that pre-flight check. Try downloading the latest development version of the Perseus CLI (cargo install perseus-cli --git https://github.com/framesurge/perseus) and see if that works. Without Powershell in your PATH, you should get a more descriptive error.

We could certainly try getting Perseus to work without depending on a shell, but as I said, there are historical reasons for that, because v0.1.0 had all sorts of problems on Windows that were solved by using a shell.

@arctic-hen7
Copy link
Member

Actually this check seems to have broken every single test, give me a moment...

@arctic-hen7
Copy link
Member

Okay, I've completely removed the use of shells in the CLI for now, which seems to work well, at least locally and on CI. @raspirin can you confirm this works for you on a Windows box when you next can? Happy to cut a new release if it's all working for you. (I.e. even without Powershell in your PATH, the CLI should now work fine as long as you have the necessary tools installed.)

@raspirin
Copy link
Author

Good news, the cli won't report "Couldn't find cargo" with and without powershell in PATH.
But I got a cli error:

Error: couldn't execute command '& 'C:\Users\rin\AppData\Local\perseus\perseus_cli\cache\tools\wasm-bindgen-0.2.88/wasm-bindgen.exe' ./dist/target_wasm/wasm32-unknown-unknown/debug/app.wasm --out-dir dist/pkg --out-name perseus_engine --target web ' (this doesn't mean it threw an error, it means it couldn't be run at all)

Caused by:
        program not found

I could manually run wasm-bindgen using the location above though.
图片

Then I added powershell into PATH and switched back to a crates.io version of cli, the same project worked just fine as on my Linux machine.

I notice that the wasm-bindgen path is weird. It's not a completely windows-style path.
Any ideas? @arctic-hen7

@arctic-hen7
Copy link
Member

Hmm, that command doesn't look right at all, let me see if tool execution is still depending on the shell somehow...

@arctic-hen7
Copy link
Member

@raspirin I've created a new branch fix-314, could you try getting the CLI from there through cargo install? It will create a perseus_debug.log file which lists every command the CLI actually runs, my suspicion is that something is wrong with the way we split command strings into commands and arguments on Windows. Seeing that should help me understand what's going on here.

@raspirin
Copy link
Author

raspirin commented Feb 1, 2024

Sure. I will try this next weekend.

@raspirin
Copy link
Author

Sorry for the long delay. It's been a lot last couple of months.
Back to the cli problem. Somehow I can't install the patched cli from source. rustc will complain that it can't build the target. Not sure if I am doing right.

PS C:\Users\rin\Downloads\perseus-fix-314\packages\perseus-cli> rustc --version
rustc 1.78.0 (9b00956e5 2024-04-29)
PS C:\Users\rin\Downloads\perseus-fix-314\packages\perseus-cli> cargo install --path .
  Installing perseus-cli v0.4.2 (C:\Users\rin\Downloads\perseus-fix-314\packages\perseus-cli)
    Updating crates.io index
   Compiling serde v1.0.202
   Compiling thiserror v1.0.61
   Compiling tokio-util v0.7.11
   Compiling tokio-native-tls v0.3.1
   Compiling pin-project v1.1.5
   Compiling multer v2.1.0
   Compiling brotlic-sys v0.2.2
   Compiling futures-executor v0.3.30
   Compiling clap v4.5.4
   Compiling notify v5.0.0-pre.13
   Compiling tokio-stream v0.1.15
   Compiling brotlic v0.8.2
error[E0432]: unresolved import `winapi::shared::winerror`
 --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\notify-5.0.0-pre.13\src\windows.rs:9:21
  |
9 | use winapi::shared::winerror::ERROR_OPERATION_ABORTED;
  |                     ^^^^^^^^ could not find `winerror` in `shared`

   Compiling tungstenite v0.21.0
   Compiling futures v0.3.30
error[E0308]: mismatched types
   --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\notify-5.0.0-pre.13\src\windows.rs:277:29
    |
277 |         overlapped.hEvent = request_p;
    |         -----------------   ^^^^^^^^^ expected `winapi::ctypes::c_void`, found `libc::c_void`
    |         |
    |         expected due to the type of this binding
    |
    = note: `libc::c_void` and `winapi::ctypes::c_void` have similar names, but are actually distinct types
note: `libc::c_void` is defined in crate `core`
   --> /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\core\src\ffi\mod.rs:183:1
note: `winapi::ctypes::c_void` is defined in crate `winapi`
   --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\winapi-0.3.9\src\lib.rs:38:5
    |
38  |     pub enum c_void {}
    |     ^^^^^^^^^^^^^^^

error[E0308]: mismatched types
    --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\notify-5.0.0-pre.13\src\windows.rs:283:13
     |
281  |         let ret = winbase::ReadDirectoryChangesW(
     |                   ------------------------------ arguments to this function are incorrect
282  |             handle,
283  |             req_buf,
     |             ^^^^^^^ expected `winapi::ctypes::c_void`, found `libc::c_void`
     |
     = note: `libc::c_void` and `winapi::ctypes::c_void` have similar names, but are actually distinct types
note: `libc::c_void` is defined in crate `core`
    --> /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\core\src\ffi\mod.rs:183:1
note: `winapi::ctypes::c_void` is defined in crate `winapi`
    --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\winapi-0.3.9\src\lib.rs:38:5
     |
38   |     pub enum c_void {}
     |     ^^^^^^^^^^^^^^^
note: function defined here
    --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\winapi-0.3.9\src\um\winbase.rs:2072:12
     |
2072 |     pub fn ReadDirectoryChangesW(
     |            ^^^^^^^^^^^^^^^^^^^^^

Some errors have detailed explanations: E0308, E0432.
For more information about an error, try `rustc --explain E0308`.
error: could not compile `notify` (lib) due to 3 previous errors
warning: build failed, waiting for other jobs to finish...
error: failed to compile `perseus-cli v0.4.2 (C:\Users\rin\Downloads\perseus-fix-314\packages\perseus-cli)`, intermediate artifacts can be found at `C:\Users\rin\Downloads\perseus-fix-314\target`.
To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.

@arctic-hen7
Copy link
Member

Maybe try with cargo install --git https://github.com/framesurge/perseus --branch fix-314 perseus-cli? There are a few reasons why installing from within the repo might not work...

@laloutre87
Copy link

Hello. "&" seems to be a valid operator in Powershell (never used powershell, so no idea what it does, but it doesn"t throw an error), while it's not in the regular command prompt.
I tried with removing the "&" from the command string formatting, and it seems to work.

( install.rs, two lines with "let (wb_path, wo_path) = (format!" in it . )

@arctic-hen7
Copy link
Member

Oh! Very greatly appreciated! I'll update the branch with this and then could you confirm the CLI works for you @raspirin?

@laloutre87
Copy link

Hi. Also note that you still depend on powershell for the function run_cmd_directly in cmd.rs .

@raspirin
Copy link
Author

It doesn't compile even though I try to install it from the repo. I believe the error is the same as my last try.

PS C:\Users\rin> rustc --version
rustc 1.80.1 (3f5fd8dd4 2024-08-06)
PS C:\Users\rin> cargo install --git https://github.com/framesurge/perseus --branch fix-314 perseus-cli
    Updating git repository `https://github.com/framesurge/perseus`
  Installing perseus-cli v0.4.2 (https://github.com/framesurge/perseus?branch=fix-314#e6620cdb)
    Updating crates.io index
     Locking 245 packages to latest compatible versions
      Adding aho-corasick v0.7.20 (latest: v1.1.3)
      Adding base64 v0.21.7 (latest: v0.22.1)
      Adding bitflags v1.3.2 (latest: v2.6.0)
      Adding cargo-lock v8.0.3 (latest: v10.0.0)
      Adding cargo_metadata v0.15.4 (latest: v0.18.1)
      Adding cargo_toml v0.15.3 (latest: v0.20.5)
      Adding command-group v2.1.0 (latest: v5.0.1)
      Adding core-foundation v0.9.4 (latest: v0.10.0)
      Adding encode_unicode v0.3.6 (latest: v1.0.0)
      Adding foreign-types v0.3.2 (latest: v0.5.0)
      Adding foreign-types-shared v0.1.1 (latest: v0.3.1)
      Adding generic-array v0.14.7 (latest: v1.1.0)
      Adding h2 v0.3.26 (latest: v0.4.6)
      Adding headers v0.3.9 (latest: v0.4.0)
      Adding headers-core v0.2.0 (latest: v0.3.0)
      Adding hermit-abi v0.3.9 (latest: v0.4.0)
      Adding http v0.2.12 (latest: v1.1.0)
      Adding http-body v0.4.6 (latest: v1.0.1)
      Adding hyper v0.14.31 (latest: v1.5.0)
      Adding hyper-tls v0.5.0 (latest: v0.6.0)
      Adding idna v0.5.0 (latest: v1.0.2)
      Adding indicatif v0.17.0-beta.1 (latest: v0.17.8)
      Adding inotify v0.9.6 (latest: v0.11.0)
      Adding linux-raw-sys v0.4.14 (latest: v0.6.5)
      Adding minify-js v0.4.3 (latest: v0.6.0)
      Adding mio v0.7.14 (latest: v1.0.2)
      Adding miow v0.3.7 (latest: v0.6.0)
      Adding multer v2.1.0 (latest: v3.1.0)
      Adding nix v0.26.4 (latest: v0.29.0)
      Adding notify v5.0.0-pre.13 (latest: v6.1.1)
      Adding ntapi v0.3.7 (latest: v0.4.1)
      Adding parse-js v0.10.3 (latest: v0.24.0)
      Adding reqwest v0.11.27 (latest: v0.12.8)
      Adding rustls-pemfile v1.0.4 (latest: v2.2.0)
      Adding security-framework v2.11.1 (latest: v3.0.0)
      Adding sync_wrapper v0.1.2 (latest: v1.0.1)
      Adding system-configuration v0.5.1 (latest: v0.6.1)
      Adding system-configuration-sys v0.5.0 (latest: v0.6.0)
      Adding tokio-tungstenite v0.21.0 (latest: v0.24.0)
      Adding toml v0.5.11 (latest: v0.8.19)
      Adding toml v0.7.8 (latest: v0.8.19)
      Adding toml_edit v0.19.15 (latest: v0.22.22)
      Adding tungstenite v0.21.0 (latest: v0.24.0)
      Adding unicode-width v0.1.14 (latest: v0.2.0)
      Adding wasi v0.11.0+wasi-snapshot-preview1 (latest: v0.13.3+wasi-0.2.2)
      Adding windows-sys v0.48.0 (latest: v0.59.0)
      Adding windows-sys v0.52.0 (latest: v0.59.0)
      Adding windows-targets v0.48.5 (latest: v0.52.6)
      Adding windows_aarch64_gnullvm v0.48.5 (latest: v0.52.6)
      Adding windows_aarch64_msvc v0.48.5 (latest: v0.52.6)
      Adding windows_i686_gnu v0.48.5 (latest: v0.52.6)
      Adding windows_i686_msvc v0.48.5 (latest: v0.52.6)
      Adding windows_x86_64_gnu v0.48.5 (latest: v0.52.6)
      Adding windows_x86_64_gnullvm v0.48.5 (latest: v0.52.6)
      Adding windows_x86_64_msvc v0.48.5 (latest: v0.52.6)
      Adding winnow v0.5.40 (latest: v0.6.20)
      Adding winreg v0.50.0 (latest: v0.52.0)
      Adding zerocopy v0.7.35 (latest: v0.8.7)
      Adding zerocopy-derive v0.7.35 (latest: v0.8.7)
   Compiling proc-macro2 v1.0.89
   Compiling unicode-ident v1.0.13
   Compiling windows_x86_64_msvc v0.52.6
   Compiling cfg-if v1.0.0
   Compiling serde v1.0.213
   Compiling bytes v1.8.0
   Compiling pin-project-lite v0.2.15
   Compiling itoa v1.0.11
   Compiling memchr v2.7.4
   Compiling version_check v0.9.5
   Compiling autocfg v1.4.0
   Compiling fnv v1.0.7
   Compiling futures-core v0.3.31
   Compiling futures-sink v0.3.31
   Compiling typenum v1.17.0
   Compiling futures-task v0.3.31
   Compiling futures-io v0.3.31
   Compiling pin-utils v0.1.0
   Compiling equivalent v1.0.1
   Compiling httparse v1.9.5
   Compiling futures-channel v0.3.31
   Compiling tinyvec_macros v0.1.1
   Compiling hashbrown v0.15.0
   Compiling log v0.4.22
   Compiling percent-encoding v2.3.1
   Compiling windows_x86_64_msvc v0.48.5
   Compiling tinyvec v1.8.0
   Compiling once_cell v1.20.2
   Compiling windows-targets v0.52.6
   Compiling windows-sys v0.59.0
   Compiling windows-sys v0.52.0
   Compiling form_urlencoded v1.2.1
   Compiling unicode-bidi v0.3.17
   Compiling generic-array v0.14.7
   Compiling byteorder v1.5.0
   Compiling http v0.2.12
   Compiling tracing-core v0.1.32
   Compiling getrandom v0.2.15
   Compiling slab v0.4.9
   Compiling thiserror v1.0.65
   Compiling rand_core v0.6.4
   Compiling windows-targets v0.48.5
   Compiling serde_json v1.0.132
   Compiling indexmap v2.6.0
   Compiling ryu v1.0.18
   Compiling quote v1.0.37
   Compiling tracing v0.1.40
   Compiling libc v0.2.161
   Compiling try-lock v0.2.5
   Compiling shlex v1.3.0
   Compiling syn v2.0.85
   Compiling unicode-normalization v0.1.24
   Compiling mime v0.3.17
   Compiling native-tls v0.2.12
   Compiling httpdate v1.0.3
   Compiling cpufeatures v0.2.14
   Compiling cc v1.1.31
   Compiling want v0.3.1
   Compiling http-body v0.4.6
   Compiling windows-sys v0.48.0
   Compiling utf8parse v0.2.2
   Compiling tower-service v0.3.3
   Compiling unicase v2.8.0
   Compiling winapi v0.3.9
   Compiling lazy_static v1.5.0
   Compiling base64 v0.21.7
   Compiling anstyle v1.0.9
   Compiling semver v1.0.23
   Compiling crossbeam-utils v0.8.20
   Compiling anstyle-parse v0.2.6
   Compiling idna v0.5.0
   Compiling mime_guess v2.0.5
   Compiling http v1.1.0
   Compiling multer v2.1.0
   Compiling encoding_rs v0.8.35
   Compiling regex-syntax v0.8.5
   Compiling data-encoding v2.6.0
   Compiling crypto-common v0.1.6
   Compiling block-buffer v0.10.4
   Compiling camino v1.1.9
   Compiling digest v0.10.7
   Compiling is_terminal_polyfill v1.70.1
   Compiling winnow v0.5.40
   Compiling colorchoice v1.0.3
   Compiling utf-8 v0.7.6
   Compiling url v2.5.2
   Compiling socket2 v0.5.7
   Compiling mio v1.0.2
   Compiling sha1 v0.10.6
   Compiling headers-core v0.2.0
   Compiling aho-corasick v0.7.20
   Compiling encode_unicode v0.3.6
   Compiling heck v0.5.0
   Compiling regex-automata v0.4.8
   Compiling spin v0.9.8
   Compiling adler2 v2.0.0
   Compiling option-ext v0.2.0
   Compiling strsim v0.11.1
   Compiling clap_lex v0.7.2
   Compiling unicode-width v0.1.14
   Compiling parse-js v0.10.3
   Compiling miniz_oxide v0.8.0
   Compiling console v0.15.8
   Compiling crossbeam-channel v0.5.13
   Compiling headers v0.3.9
   Compiling brotlic-sys v0.2.2
   Compiling rustls-pemfile v1.0.4
   Compiling include_dir_macros v0.7.4
   Compiling crc32fast v1.4.2
   Compiling bitflags v1.3.2
   Compiling sync_wrapper v0.1.2
   Compiling number_prefix v0.4.0
   Compiling scoped-tls v1.0.1
   Compiling ipnet v2.10.1
   Compiling minify-js v0.4.3
   Compiling flate2 v1.0.34
   Compiling command-group v2.1.0
   Compiling schannel v0.1.26
   Compiling anstyle-wincon v3.0.6
   Compiling anstyle-query v1.1.2
   Compiling winapi-util v0.1.9
   Compiling filetime v0.2.25
   Compiling same-file v1.0.6
   Compiling anstream v0.6.17
   Compiling walkdir v2.5.0
   Compiling regex v1.11.1
   Compiling tar v0.4.42
   Compiling include_dir v0.7.4
   Compiling clap_builder v4.5.20
   Compiling notify v5.0.0-pre.13
   Compiling ctrlc v3.4.5
error[E0432]: unresolved import `winapi::shared::winerror`
  --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\notify-5.0.0-pre.13\src\windows.rs:9:21
   |
9  | use winapi::shared::winerror::ERROR_OPERATION_ABORTED;
   |                     ^^^^^^^^ could not find `winerror` in `shared`
   |
note: found an item that was configured out
  --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\winapi-0.3.9\src\shared\mod.rs:84:38
   |
84 | #[cfg(feature = "winerror")] pub mod winerror;
   |                                      ^^^^^^^^
   = note: the item is gated behind the `winerror` feature

error[E0308]: mismatched types
   --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\notify-5.0.0-pre.13\src\windows.rs:277:29
    |
277 |         overlapped.hEvent = request_p;
    |         -----------------   ^^^^^^^^^ expected `winapi::ctypes::c_void`, found `libc::c_void`
    |         |
    |         expected due to the type of this binding
    |
    = note: `libc::c_void` and `winapi::ctypes::c_void` have similar names, but are actually distinct types
note: `libc::c_void` is defined in crate `core`
   --> C:\Users\rin\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib/rustlib/src/rust\library\core\src\ffi\mod.rs:184:1
    |
184 | pub enum c_void {
    | ^^^^^^^^^^^^^^^
note: `winapi::ctypes::c_void` is defined in crate `winapi`
   --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\winapi-0.3.9\src\lib.rs:38:5
    |
38  |     pub enum c_void {}
    |     ^^^^^^^^^^^^^^^

error[E0308]: mismatched types
    --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\notify-5.0.0-pre.13\src\windows.rs:283:13
     |
281  |         let ret = winbase::ReadDirectoryChangesW(
     |                   ------------------------------ arguments to this function are incorrect
282  |             handle,
283  |             req_buf,
     |             ^^^^^^^ expected `winapi::ctypes::c_void`, found `libc::c_void`
     |
     = note: `libc::c_void` and `winapi::ctypes::c_void` have similar names, but are actually distinct types
note: `libc::c_void` is defined in crate `core`
    --> C:\Users\rin\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib/rustlib/src/rust\library\core\src\ffi\mod.rs:184:1
     |
184  | pub enum c_void {
     | ^^^^^^^^^^^^^^^
note: `winapi::ctypes::c_void` is defined in crate `winapi`
    --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\winapi-0.3.9\src\lib.rs:38:5
     |
38   |     pub enum c_void {}
     |     ^^^^^^^^^^^^^^^
note: function defined here
    --> C:\Users\rin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\winapi-0.3.9\src\um\winbase.rs:2072:12
     |
2072 |     pub fn ReadDirectoryChangesW(
     |            ^^^^^^^^^^^^^^^^^^^^^

   Compiling indicatif v0.17.0-beta.1
   Compiling shell-words v1.1.0
   Compiling fs_extra v1.3.0
   Compiling fmterr v0.1.1
Some errors have detailed explanations: E0308, E0432.
For more information about an error, try `rustc --explain E0308`.
error: could not compile `notify` (lib) due to 3 previous errors
warning: build failed, waiting for other jobs to finish...
error: failed to compile `perseus-cli v0.4.2 (https://github.com/framesurge/perseus?branch=fix-314#e6620cdb)`, intermediate artifacts can be found at `C:\Users\rin\AppData\Local\Temp\cargo-installZajjRC`.
To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.

@arctic-hen7
Copy link
Member

Yes, this is an issue in the notify dependency in that branch. Let me fix part of this on main and then rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: command line interface C-bug Category: bug tribble-reported This issue was reported through Tribble.
Projects
None yet
Development

No branches or pull requests

4 participants