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

Shell integration breaks paths inside npm commands on Windows #206285

Closed
connor4312 opened this issue Feb 26, 2024 · 12 comments
Closed

Shell integration breaks paths inside npm commands on Windows #206285

connor4312 opened this issue Feb 26, 2024 · 12 comments
Assignees
Labels
papercut 🩸 A particularly annoying issue impacting someone on the team under-discussion Issue is under discussion for relevance, priority, approach

Comments

@connor4312
Copy link
Member

  1. Have a package.json like this:
{
    "scripts": {
        "foo": "npm run bar",
        "bar": "tsc"
    },
    "devDependencies": {
        "typescript": "^4.9.4"
    }
}
  1. Enable shell integration
  2. Run npm run foo. Get an error like "'tsc' is not recognized as an internal or external command"
  3. Disable shell integration
  4. Run npm run foo and it works
@connor4312 connor4312 added the papercut 🩸 A particularly annoying issue impacting someone on the team label Feb 26, 2024
@meganrogge
Copy link
Contributor

hmm, this works for me

Image

@connor4312
Copy link
Member Author

connor4312 commented Feb 26, 2024

Ah, I think this is related to Python, it's injecting a PATH to override the default one in the terminal.
image

Disabling Python resolves the issue. Maybe shell integration re-applies that PATH in nested commands and doesn't re-read the changes npm made to add its local binaries?

@Tyriar
Copy link
Member

Tyriar commented Feb 26, 2024

Python does indeed set $PATH, not prepend/append to it. cc @karrtikr

@Tyriar Tyriar added the under-discussion Issue is under discussion for relevance, priority, approach label Feb 26, 2024
@connor4312
Copy link
Member Author

connor4312 commented Feb 26, 2024

Having looked closer at the screenshot, it seems like that syntax the terminal shows has it prepending the path (though it duplicates everything I already had in my path which is not really necessary.)

Interestingly when I manually call an env.exe in the npm script, it shows a PATH that looks just like the PATH outside of the script with the addition of the expected .bin folder. But actually calling any command without specifying an absolute path now fails: npm isn't found, and running echo %path% just says "ECHO is on.". Echoing the environment is something that also works if I turn shell integration off.

@karrtikr
Copy link
Contributor

Python does indeed set $PATH, not prepend/append to it. cc @karrtikr

@Tyriar If you look closely, Python is still prepending to PATH, it just prepends a whole lot.

(though it duplicates everything I already had in my path which is not really necessary.)

Right, unfortunately we have to prepend PATH twice (once at process creation and once at shell integration level) because Python extension cannot reliably predict when shell integration works: microsoft/vscode-python#22905.

@karrtikr
Copy link
Contributor

My guess is something in shell integration script is adding to PATH incorrectly.

@connor4312 Can you try some other way of printing PATH in cmd instead of echo? For eg. try running,

python -c"import os;print(os.environ['PATH'])"

@connor4312
Copy link
Member Author

connor4312 commented Feb 27, 2024

Yes, the PATH actually looks correct (#206285 (comment)). Because there's also a difference in how echo works I suspect shell integration is doing something, as changes to the PATH would not account for that.

@karrtikr
Copy link
Contributor

karrtikr commented Feb 27, 2024

Actually now that I think about it, Command prompt does not even have shell integration script support, does it? @Tyriar So I wouldn't expect shell integration setting to have any effect.

@connor4312 I assume Command prompt is set as default profile based on "echo %path%" command you mentioned above.

@connor4312
Copy link
Member Author

no, I'm using powershell, but it seems like npm run uses cmd.exe to run its scripts.

@karrtikr karrtikr assigned meganrogge and unassigned karrtikr Feb 27, 2024
@karrtikr
Copy link
Contributor

I see gotcha. Feel free to reassign if Python extension can do anything from our end to solve this.

@connor4312
Copy link
Member Author

This is still pretty painful, I have to turn off shell integration constantly 😕

In the copilot repo for example:

Image

@connor4312
Copy link
Member Author

fyi, something changed and I no longer get this issue 🤷

@Tyriar Tyriar closed this as completed Apr 19, 2024
@microsoft microsoft locked and limited conversation to collaborators Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
papercut 🩸 A particularly annoying issue impacting someone on the team under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

5 participants