Skip to content

Conversation

@radar
Copy link

@radar radar commented Oct 15, 2019

Version details:

Version: 1.39.1
Commit: 88f15d17dca836346e787762685a40bb5cce75a8
Date: 2019-10-10T23:35:11.314Z
Electron: 4.2.10
Chrome: 69.0.3497.128
Node.js: 10.11.0
V8: 6.9.427.31-electron.0
OS: Darwin x64 18.7.0

What I expect to happen

VS Code's Git extension finds + uses Git

What actually happens

VS Code says it cannot find Git:

image

Steps to reproduce

TL;DR: VS Code should use command -v to detect commands, not which.

This is a bit tricky, so bear with me:

There is a node package called which:

https://www.npmjs.com/package/which

If this package is installed through Node, and a user is using asdf, this which-from-node can take precedence in $PATH. This will cause issues if there is no default Node version specified for asdf. An error like this can appear:

asdf: No version set for command which
you might want to add one of the following in your .tool-versions file:

nodejs 8.14.0

(I suspect this is only an issue on 'zsh' terminals -- I am unable to reproduce it in Bash)

To work around this which-conflict, we should use the built-in 'command' command. This comes standard in every shell program and is a viable alternative to 'which'.

When my shell works correctly, here's the output of which -a which:

which: shell built-in command
/Users/ryanbigg/.asdf/shims/which
/usr/bin/which

However, when it is misbehaving, I suspect the order is like this:

/Users/ryanbigg/.asdf/shims/which
which: shell built-in command
/usr/bin/which

And that's why it's trying to use the node which. When no node version is configured for asdf, then it fails to run that node-which, which then causes this extension to claim that it can't find git.

This PR (likely) fixes #81287 -- although I'm not 100% sure of that.

There is a node package called which:

https://www.npmjs.com/package/which

If this package is installed through Node, _and_ a user is using asdf, this which-from-node can take precedence in $PATH. This will cause issues if there is no default Node version specified for asdf. An error like this can appear:

		asdf: No version set for command which
		you might want to add one of the following in your .tool-versions file:

		nodejs 8.14.0

(I suspect this is only an issue on 'zsh' terminals)

To work around this which-conflict, we should use the built-in 'command' command. This comes standard in every shell program and is a viable alternative to 'which'.
radar added a commit to radar/node-which that referenced this pull request Oct 15, 2019
Naming the executable of this library `which` can cause issues -- such as microsoft/vscode#82552. 

I would like to suggest renaming this executable to `node-which`, so that people can clearly differentiate between built-in `which` (bash, zsh, etc) and the Node version.
@JoshTeperman
Copy link

👀

radar added a commit to radar/node-which that referenced this pull request Oct 16, 2019
Naming the executable of this library `which` can cause issues -- such as microsoft/vscode#82552.

I would like to suggest renaming this executable to `node-which`, so that people can clearly differentiate between built-in `which` (bash, zsh, etc) and the Node version.
@joaomoreno joaomoreno added the git GIT issues label Oct 16, 2019
@joaomoreno
Copy link
Member

So... what happens if the user has a command command ahead in their PATH variable, just like which?

@joaomoreno
Copy link
Member

Closing.

@joaomoreno joaomoreno closed this Nov 4, 2019
@ryenus
Copy link
Contributor

ryenus commented Nov 4, 2019

So... what happens if the user has a command command ahead in their PATH variable, just like which?

I believe that wouldn't be a problem because the shell builtin command should take precedence over things from $PATH.

From bash manual: Shell Builtin Commands

When the name of a builtin command is used as the first word of a simple command (see Simple Commands), the shell executes the command directly, without invoking another program.

@joaomoreno
Copy link
Member

joaomoreno commented Nov 4, 2019

Isn't which a builtin command, as was mentioned above?

@ryenus
Copy link
Contributor

ryenus commented Nov 5, 2019

Isn't which a builtin command, as was mentioned above?

In csh which is indeed a builtin, but not in sh/bash, @joaomoreno

@joaomoreno
Copy link
Member

Got it. Keeping my opinion: this is not worth the trouble.

isaacs pushed a commit to npm/node-which that referenced this pull request Nov 18, 2019
Naming the executable of this library `which` can cause issues -- such
as microsoft/vscode#82552.

Rename executable to `node-which`, so that people can clearly
differentiate between built-in `which` (bash, zsh, etc) and the Node
version.

PR-URL: #67
Credit: @radar
Close: #67
Reviewed-by: @isaacs
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

git GIT issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Git: "git not found" error, macOS, when xcodebuild license needs renewing

4 participants