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 · 10 comments
Open

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

raspirin opened this issue Dec 11, 2023 · 10 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.

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

3 participants