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

Wrong behavior when using system commands with non-bash or non-cmd.exe shells #203

Open
Halkcyon opened this issue Jun 14, 2022 · 7 comments
Labels

Comments

@Halkcyon
Copy link

Halkcyon commented Jun 14, 2022

I'm attempting to use shells besides those in the title, but there is wrong behavior present in this extension.

image

In PowerShell or nushell, neither && nor & are supported. When attempting to use PowerShell v7+, & causes cargo-watch to spawn a background job instead.

When attempting to use the cmd:trail form via -- $COMMAND_LINE, it still uses the wrongly-formed & echo after the command.


platform: Windows 10
cargo: v1.61.0
cargo-watch: v8.1.1

@passcod
Copy link
Member

passcod commented Jun 15, 2022

Oh, that's interesting and I completely missed that. 🙃 I'm going to have to sit down and figure that one out.

As a workaround, you can do something like:

cargo watch -n -- powershell.exe -e 'your command line'

As a side note, you're writing

cargo watch -x build --use-shell=powershell.exe -s 'ni .trigger'

obviously --use-shell currently makes cargo watch uses powershell.exe for the entire thing, but would you be wanting/expecting cargo watch to use different shell for different commands or was that just an artifact of how you wrote the command?

@passcod passcod removed the critical label Jun 15, 2022
@Halkcyon
Copy link
Author

Halkcyon commented Jun 15, 2022

I don't see the -n as an option in the help text? My goal here was trying to get web server hot reloading on code change following a suggestion in the documentation to create a temporary .trigger file.

I think it makes sense that use-shell affects the whole invocation, but I'm not sure how you would support arbitrarily connected commands. In my nushell/Windows PowerShell example, you can only join commands using ; but that's just a separator and won't fail on a non-0 RC.

Ultimately, I think the & echo "... behavior is surprising and perhaps unnecessary with use-shell?

@passcod
Copy link
Member

passcod commented Jun 15, 2022

Ugh, I keep forgetting option differences with watchexec. You're right, there is no -n, so there's no workaround after all

@passcod
Copy link
Member

passcod commented Jun 15, 2022

I'm not sure how you would support arbitrarily connected commands

Ultimately the watchexec runtime will just support running multiple processes natively, so it won't matter. For now, cargo watch tries to approximate it, and, as you've found, sometimes runs into the wall.

@Halkcyon
Copy link
Author

Halkcyon commented Jun 21, 2022

Do you have an idea you'd like to explore? I'd suggest not appending any extra command output when using the -- form @passcod. I can add a contribution to correct that behavior and anticipate minor impact to people who want to see that echo output

@passcod
Copy link
Member

passcod commented Jun 21, 2022

Right, sorry, an update: as of 5 days ish ago the Watchexec runtime supports this natively (it was not super hard to implement). I've also decided to move Cargo Watch to the newer runtime sooner rather than later. So what will happen is:

  • I'll finish releasing the Watchexec runtime. There's three new crates that need to be just slightly polished and published.
  • I'll incorporate the new bits in Watchexec CLI 1.20.0.
  • I'll finish porting Cargo Watch to the new Watchexec runtime.
  • I'll release Cargo Watch 9.0.0, which will fix this bug.

I'm anticipating this to take 1-2 weeks. Update: aiming for a weekend release by 10 July. Well, that didn't go as planned.

However, if you want to, you can put up a PR that does exactly what you've just said, and I'll release that as 8.1.3 asap, with the understanding that exact behaviour will probably change slightly when 9 gets released.

@passcod
Copy link
Member

passcod commented Dec 29, 2022

Workaround will be in 8.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants