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

installation: allow to use different build settings for tools installation #825

Closed
hyangah opened this issue Oct 22, 2020 · 5 comments
Closed

Comments

@hyangah
Copy link
Contributor

hyangah commented Oct 22, 2020

I thought I already filed an issue to track this, but obviously I didn't?

Recently we observed multiple cases where reusing the project build configuration for tool installation is not desirable. Example cases are

Currently, the settings that are involved in tool installation are

  • "go.goroot","go.alternateTools"."go", and the version chosen by go.environment.choose command: determines the Go version
  • "go.toolsGopath": determines the tool installation path (${go.toolsGopath}/bin).
  • "go.toolsEnvVars": determines the environment vars. This is also used for tools execution such as go build, gopls, dlv...

I propose to introduce a new setting that takes an object

"go.toolsInstall": {
   "go": "",  // path to the go command 
   "env": {},  // extra env vars to apply
   "flags": [],  // extra flags users want to pass e.g. -insecure
}

When they are not set, we fall back to the old behavior.
This setting is machine scope (https://code.visualstudio.com/api/references/contribution-points).

We also deprecate "go.toolsGopath" sometime next year.

cc @suzmue @stamblerre @ianthehat @ramya-rao-a

@ramya-rao-a
Copy link
Contributor

go.toolsEnvVars applies to all the Go tools that are run by the extension. In the new proposal, you have flags. Do you plan to apply these flags to all the Go tools or are you thinking of these flags to apply only to the go cmd?

@ramya-rao-a
Copy link
Contributor

Also, the new proposal does not have a counterpart to go.toolsGopath. What is our answer for folks who used this to ensure that the Go tools used by the extension dont pollute their gopath?

@hyangah
Copy link
Contributor Author

hyangah commented Oct 26, 2020

@ramya-rao-a Thanks for the comment.

go.toolsEnvVars applies to all the Go tools that are run by the extension.

Yes, I am proposing not to apply the env vars if go.toolsInstall.env is set.
go.toolsEnvVars were introduced to set environment variables necessary to build user projects
(cross-compile, different compilation flags, etc. See microsoft/vscode-go#932, microsoft/vscode-go#632). Most of those environment
variables are not applicable to the go command invoked for tool installation.

GOPROXY and HTTP-related env vars may be applicable though. During migration,
we can prompt users and suggest new settings when users invoke tool installation and
we see the use of go.toolsEnvVars.

In the new proposal, you have flags. Do you plan to apply these flags to all the Go tools or are you thinking of these flags to apply only to the go cmd?

Yes, this setting applies only to the go command used to install the tools. So, these flags are only go command flags.

Also, the new proposal does not have a counterpart to go.toolsGopath. What is our answer for folks who used this to ensure that the Go tools used by the extension dont pollute their gopath?

The tool installation is now done in module-aware mode and shouldn't pollute GOPATH except downloading modules and checksum in GOPATH/pkg. If users find they want to use separate module caches, they can configure by setting the env field.

@gopherbot gopherbot added this to the Untriaged milestone Apr 8, 2021
@stamblerre stamblerre modified the milestones: Untriaged, Backlog Apr 9, 2021
@hyangah hyangah modified the milestones: Unplanned, v0.31.0 Jan 5, 2022
@hyangah hyangah modified the milestones: v0.31.0, v0.32.0 Jan 18, 2022
@hyangah hyangah added the go1.18 label Jan 26, 2022
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/387956 mentions this issue: package.json: add go.toolsManagement.go

gopherbot pushed a commit that referenced this issue Mar 1, 2022
This new setting allows users to specify the go command
for tools installation.

The go version selected for go.toolsManagement.go is not yet
used for version check in goInstallTools.ts inspectGoToolVersion yet.

For #825

Change-Id: Ic7a401b2089fe1e412ede32be474ee0f309c32d4
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/387956
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
@hyangah
Copy link
Contributor Author

hyangah commented Mar 1, 2022

v0.32.0 will introduce a new go.toolsManagement.go.
Not sure about env and flags at this moment.

Q. Users need to use different environment variables?
A. The issue in #628 was from the GOOS/GOARCH/GOROOT. We already unset them for tools installation.

Q. Users need to use special build flags.
A. The issue #814 was about -insecure flag, but that flag is deprecated.

It's possible that users may need different env vars & flags - in that case, for now users can wrap the go command in a script and place the script in the go.toolsManagement.go.

@hyangah hyangah closed this as completed Mar 1, 2022
@hyangah hyangah self-assigned this Mar 3, 2022
@golang golang locked and limited conversation to collaborators Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Tools Management
  
Needs triage
Development

No branches or pull requests

4 participants