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

go.alternateTools does not automatically add .exe (or similar) on Windows, missing "which" call? #3731

Open
jakebailey opened this issue Mar 24, 2025 · 8 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@jakebailey
Copy link

What version of Go, VS Code & VS Code Go extension are you using?

Version Information
  • Run go version to get version of Go from the VS Code integrated terminal.
    • go version go1.24.1 linux/amd64
  • Run gopls -v version to get version of Gopls from the VS Code integrated terminal.
    • golang.org/x/tools/gopls v0.31.1-0.20250324134734-bf12eb7e7db4+dirty
  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.
    • 1.99.0-insider dc289883be5d37d5d2b2f7d30926aa42a3123437
  • Check your installed extensions to get the version of the VS Code Go extension
    • 0.47.1
  • Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) > Go: Locate Configured Go Tools command.
    • gopls: /home/jabaile/go/bin/gopls (version: v0.31.1-0.20250324134734-bf12eb7e7db4+dirty built with go: go1.25-2d2bcdd2a)
      gotests: not installed
      gomodifytags: not installed
      impl: not installed
      goplay: not installed
      dlv: /home/jabaile/go/bin/dlv (version: v1.24.1 built with go: go1.24.1)
      golangci-lint: /home/jabaile/work/TypeScript-go/_tools/custom-gcl (version: v1.64.6+dirty built with go: go1.24.1)

Share the Go related settings you have added/edited

Run Preferences: Open Settings (JSON) command to open your settings.json file.
Share all the settings with the go. or ["go"] or gopls prefixes.

    "go.lintTool": "golangci-lint",
    "go.alternateTools": {
        "golangci-lint": "${workspaceFolder}/_tools/custom-gcl${env:GOEXE}"
    },

Describe the bug

In order to support users of a repo that are using all OSs, I've had to use ${env:GOEXE} to conditionally add .exe on Windows via the Go extension's environment variable provider when configuring our golangci-lint custom built binary.

It seems to me like this shouldn't be required; using a library like https://www.npmjs.com/package/which on the config would automatically net the correct path with extension for the tool.

Steps to reproduce the behavior:

  1. Configure go.alternateTools without an extension on Windows.
  2. Run the tool (save to lint, or similar)
  3. Linter is not run.
@gopherbot gopherbot added this to the Untriaged milestone Mar 24, 2025
@firelizzard18
Copy link
Contributor

I think you're misunderstanding which. which does the same PATH lookup that is done when you execute some-cmd in a shell or via a system call such as system("some-cmd") (in Go, exec.Command("some-cmd").Run()).

What you're asking for is not a PATH lookup. You are asking for the extension to conditionally append .exe when running on Windows. While I see the value of what you're requesting, vscode-go expects go.alternateTools to provide the full path to a file so it makes sense that it uses that value directly without altering it (disclaimer: this is a personal opinion and I am not a representative of vscode-go).

@jakebailey
Copy link
Author

jakebailey commented Mar 24, 2025

I don't think I'm misunderstanding which. I'm referring to which in terms of the npm package's behavior, which handles PATHEXT. That can be manually implemented for sure, but it's certainly less effort to just let some lib do it. I assume that go.alternateTools already does some sort of PATH-based absolute lookup already in any case, since I can pass in a string like golangci-lint and it works (on Linux, anyway).

@jakebailey
Copy link
Author

@firelizzard18
Copy link
Contributor

Let's say that ${workspaceFolder} resolves to /home/alice/src/app. It appears that you're suggesting vscode-go calls which("/home/alice/src/app/_tools/custom-gcl"), possibly with arguments. That is not the intended usage of which as documented; the intended usage is for which to be called on the command name, not the full file path (or a pseudo file path). In your case this could be which("custom-gcl") which would work if you added ${workspaceFolder}/_tools/ to PATH. Calling it on a pseudo file path may work but that does not match any of the examples shown in the documentation.

That can be manually implemented for sure, but it's certainly less effort to just let some lib do it.

It seems to me that the obvious solution is if (process.platform === 'win32' && !execFilePath.includes('.')) { execFilePath += '.exe'; }. That doesn't seem like much effort.

@jakebailey
Copy link
Author

That is not the intended usage of which as documented

The code in which has this case explicitly supported:

  // If it has a slash, then we don't bother searching the pathenv.
  // just check the file itself, and that's it.

It is not uncommon to call which like this in my experience.

In your case this could be which("custom-gcl") which would work if you added ${workspaceFolder}/_tools/ to PATH.

I think you're speaking hypothetically, but I cannot ask people to do this; it's a non-standard location specific to a repo checkout. The VS Code settings are the best place to set this and it work consistently without trying to figure out how to ensure PATH is always updated with the right stuff. I'd rather keep using the GOEXE trick than to ask people to futz with PATH.

It seems to me that the obvious solution is if (process.platform === 'win32' && !execFilePath.includes('.')) { execFilePath += '.exe'; }. That doesn't seem like much effort.

The resolution would also have to handle .cmd, .bat, and potentially anything else depending on PATHEXT.

@h9jiang
Copy link
Member

h9jiang commented Mar 25, 2025

Hi @jakebailey

Thanks for raising this issue. I agree that I would prefer setting alternateTools instead of adding this custom-gcl to PATH.

The entire binary look up logic is quite complicated in Go. I spent some time to understand this but there is still some ambiguity the require further investigation.

To understand your request, today, you are adding ${env:GOEXE} to alternateTools so the config can be used across different OSes. (this add .exe in windows, does not add anything in linux). You are proposing: this behavior can be easily implemented by which module that you mentioned.

As mentioned, the binary look up is kind of complicated in vscode go and in go. There are:

  • env var GOBIN for install binaries.
  • env var GOPATH/bin used for install binaries.

I think GOBIN is overwriting GOPATH if both are present.

I assume that go.alternateTools already does some sort of PATH-based absolute lookup already in any case, since I can pass in a string like golangci-lint and it works (on Linux, anyway).

Yes. I have not look deep enough but from the observation, vscode-go definitely lookup binaries on GOPATH, GOBIN and PATH. (I'm not so sure of the look up order yet). So if which module can handle the all the lookup automatically, I think it would make the maintainer's life much easier.

By reading from documentation, the alternateTools should provide either absolute path or the name of the binary in GOPATH/bin, GOROOT/bin or PATH. A use case mentioned here is "wrapper script for Go tools", I would image users will add "/path/to/script/shell.sh".

I will spend more time to understand tool installation and path resolve. Refactoring those code for readability purposes and understand the situation better. But I'm not sure whether which could be useful in go binary path lookup.

@h9jiang h9jiang added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 25, 2025
@h9jiang h9jiang modified the milestones: Untriaged, vscode-go/backlog Mar 25, 2025
@jakebailey
Copy link
Author

jakebailey commented Mar 25, 2025

Yes, I would expect that you could simply write (psuedocodeish):

import path from "path";
import which from "which";


function getCmd(cmd: string, gopathBin: string | undefined, gobin: string | undefined): string {
	let path = "";
	if (gopathBin) path += gopathBin + path.delimiter;
	if (gobin) path += gobin + path.delimiter;
	path += process.env.PATH;

	return which.sync(cmd, { path });
}

And it "just works".

Though, I don't know the order either. I'd be surprised if GOPATH or GOBIN took precedence over $PATH.

@firelizzard18
Copy link
Contributor

Provide either absolute path or the name of the binary

This is the key point (from the documentation Hongxiang linked). It's not explicitly stated but I think it's pretty clearly implied that it means the "absolute path of the binary file", which I would interpret as the entire path including any extension that may be necessary. In other words, if a name (without slashes) is provided, it uses a lookup process, but if a path (with slashes) is provided it expects that to be a literal file path. Personally if I provide a file path I expect that path to be used as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants