Open jayconrod opened 4 years ago
I think this is a good idea as it will lead to a noticeable improvement to the user experience for new users, and we should do this, unless we are able to discover a reason why doing so could be harmful or dangerous.
Given the installers already own the decision of where to place the Go installation and make it available (via an added entry to PATH or a platform-equivalent), and by not setting a GOPATH, they are implicitly relying on the default GOPATH value that go
uses (which is $HOME/go
or platform-equivalent), I think also adding GOBIN
to PATH
is an appropriate and in-scope task for an installer.
I'll think more about it, but I can't think of concerns with doing so.
I'll tentatively mark this for consideration for 1.15, since it seems like a minor and safe change to me (and the change doesn't need to be done in the main Go tree). It's an opportunity to use an upcoming beta release to try it out and get user feedback. That said, we are in the release freeze for 1.15, so if people think this decision needs more time or input, we can move this for 1.16 cycle.
/cc @golang/osp-team
This issue is currently labeled as early-in-cycle for Go 1.16. That time is now, so this is a friendly ping so the issue is looked at again.
@jayconrod Friendly ping on this. Does it sound good to you to move this issue to NeedsFix for 1.16 milestone?
I'll also /cc @stevetraut who's been working on improving the installation experience, I think this change can be beneficial.
Sure, it sounds like we're in agreement this is a good idea.
Does anyone on the release team have bandwidth to fix this for 1.16? I'm not familiar with the installers, and I'm not sure if I'll have bandwidth to look at this before the freeze.
Yes, I can take this.
It seems this has already been done for Windows 3 years ago. See issue #23025 and CL 104115.
Going to look into the current state of the macOS installer next. Edit: It doesn't have this change yet.
I've made progress on the side of the macOS installer, but ran into some usability questions around edge cases that we'll need to agree on answers to. I'll provide more detail and follow up on this during the 1.17 cycle.
I spent some time investigating this and found some additional constraints and challenges. It seems we can't make a simple change here without potentially needing larger UI and behavior changes, and corresponding updates to the installation/uninstallation instructions.
There are the following relevant factors:
GOPATH
environment variable; it leaves it to be completely user controlled.go
from.There's agreement it's clear what to do when an installer runs on a clean system, where neither GOPATH
nor GOBIN
are set. Such cases are likely when installing Go for the first time, and the user hasn't created a custom configuration. In this case, the macOS installer should ensure the default location for user installed Go binaries, as can be determined with $(go env GOPATH)/bin)
and currently resolves to "$HOME/go/bin", is added to PATH, so it's possible to invoke foo
after doing go install example.org/cmd/foo@latest
.
The Windows installer differs from macOS installer in that Windows sets GOPATH to "%USERPROFILE%\go", while macOS one doesn't set it (and rely on cmd/go's default "$HOME/go" one).
The Windows uninstaller has a mechanism to remove the environment variables it sets:
Note: Using this uninstall process for Windows will automatically remove windows environment variables created by the original installation.
The macOS uninstall instructions are to remove /usr/local/go
and the /etc/paths.d/go
file (which is where $GOROOT/bin is added to users' $PATH).
In the original issue, @jayconrod suggested:
(Note that neither GOBIN nor GOPATH are likely to be set when an installer runs. If they are set, we should respect them; the goal is just to ensure that the directory where go install writes binaries is added to PATH).
I explored that path:
But came to the conclusion that this might actually be counter-productive:
# TODO(dmitshur): Think more about what to do if GOBIN and GOPATH are already
# set to a custom value; maybe it's better to consider that out of scope for
# the installer (if the user has set a custom $GOBIN OR $GOPATH, it's likely
# they are an advanced user and have already added it to their $PATH, meaning
# the installer shouldn't try to do its own thing)?
So the macOS installer doesn't currently set the GOPATH environment variable, it uses the default one, or the one the user has specified. This is how it behaved for a long time, so suddenly overwriting a potential custom GOPATH without prompting the user would be too disruptive. Starting to prompt the user would add cognitive overhead, so it's a bigger change to consider.
Discussion above assumes it's possible for the installer to know what GOPATH
and GOBIN
are set to. This means it needs access both to the users environment variables, and also the files that go env
modifies via go env -w
. At this stage I'm unsure how viable it is, and how reliable it can be made.
Consider that the user may have those environment variables conditionally configured in a .bash_profile
or .zprofile
. Perhaps it's possible to determine the default shell, and invoke it in a way that creates a default environment, then figure out the configuration from there. I didn't explore this further.
Based on these findings, it seems making this change in the macOS installer may be a bigger undertaking than we anticipated, and needs a more holistic approach.
I considered one possible best-effort solution of appending $HOME/go/bin
to /etc/path.d/go
regardless, which may do the right thing in most cases. However, there'd be no way to opt out of this behavior, and it might be unwelcome to users whose GOPATH is intentionally elsewhere. I decided against it.
While talking about this briefly with @toothrot, he suggested a possible idea for the cmd/go
command to consider printing a prompt after go install example.com/cmd@latest
if it detects that the installed cmd
isn't in already PATH. If false positives can be eliminated (i.e., some users may intentionally choose not add GOPATH/bin to PATH), this could be similar to some other helpful messages cmd/go
prints in some situations.
I don't plan to keep working on this for now, so moving out the Go 1.17 milestone.
While talking about this briefly with @toothrot, he suggested a possible idea for the cmd/go command to consider printing a prompt after go install example.com/cmd@latest if it detects that the installed cmd isn't in already PATH. If false positives can be eliminated (i.e., some users may intentionally choose not add GOPATH/bin to PATH), this could be similar to some other helpful messages cmd/go prints in some situations.
This that this 2020 issue is stalled on determining the right solution, I think it would be highly valuable and appreciated if go install example.com/cmd@latest
checked whether the location where the cmd is installed is in the path, and displayed a message to the user about this.
Suggest that any such user facing message include a link to https://go.dev/doc/install where a more detailed explanation can be always be kept up to date.
—
If such a fix is going to still take some time, can we update the docs:
export PATH="$PATH:$(go env GOPATH)/bin"
to ~/.bash_profile
). Advanced users will know what they want to do (instead). Everyone else will be helped.This is a good idea to avoid having to set up the environment variables manually.
Ref: https://discussions.apple.com/thread/250911992?sortBy=best
We currently provide installers for Go for Windows (an .msi file) and macOS (a .pkg file).
Both installers add
$GOROOT/bin
toPATH
. This lets users run commands likego build
without having to editPATH
themselves.However, users still need to edit
PATH
in order to run binaries installed withgo install
orgo get
. This introduces unnecessary confusion for new developers. To avoid this, both installers should add$GOBIN
toPATH
as well.(Note that neither
GOBIN
norGOPATH
are likely to be set when an installer runs. If they are set, we should respect them; the goal is just to ensure that the directory wherego install
writes binaries is added toPATH
).