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

fix(go): reflect on proper path for GOROOT #1661

Merged
merged 6 commits into from
Feb 15, 2024
Merged

fix(go): reflect on proper path for GOROOT #1661

merged 6 commits into from
Feb 15, 2024

Conversation

wheinze
Copy link
Contributor

@wheinze wheinze commented Feb 12, 2024

The last patches to GOROOT did miss to take over the /go suffix in the path, which ends up in various arbitrary errors when running go CLI commands.

The suffix has been added properly before: 786220c#diff-352f25ac643733cdd2fb54a78ced508b3658cab947230ab610b968f154840413L54

}
if settings.go_set_goroot {
set("GOROOT", tv.install_path());
set("GOROOT", self.goroot(tv));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to also extract the tarball to this directory

Copy link
Contributor Author

@wheinze wheinze Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh i see, so this is much more of a migration issue. when reinstalling the go versions with mise 2024.2.14, it's fine, as the directory layout doesn't differ from the expectations.

so I'm wondering what your actual suggestion might be? is there a better way to transition already installed versions of Go into the new layout, or would you revert to the old directory layout (by extracting the tarball etc.)?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the error you're seeing exactly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the error you're seeing exactly?

once (re-)installed with a recent version of mise (2024.2.14), the issues do not occur anymore.

unfortunately, i did a reinstall of all my Go versions by running mise uninstall go --all before, so i could just give a recap of my journey, but cannot grep any errors anymore without doing a repro of the old setup:

Having a Go version installed with an older version of mise before upgrading to one of the recent versions (I would have to figure out which version exactly did cause this issue, but would assume it to be 2024.2.8, as this is the version including these changes)), results in issue related to GOROOT being set to ...mise/installs/go/<version>, while still being installed to ...mise/installs/go/<version>/go, with a directory layout like this in ...mise/installs/go/<version>:

├── go
├── ...
└── packages

In concrete this means, I ran go get -u ./... in a project with a Go 1.22 installed via mise which returned:
go: GOPROXY list is not the empty string, but contains no entries.

Having a look into my configs, it turned out that I did not set GOPROXY anywhere, nor did mise. I tried the same Go command with a system-wide version of Go, which did succeed. So the issues must be related to mise.
Then I did fire an strace to check, what paths are seeked to find the go.env file, that is defining the defaults for GOPROXY.

The strace:

LC_ALL=C strace -fe open,openat go get -u ./...

did show, that a go.env was tried to be found here: ...mise/installs/go/<version>/go.env, but it was located here: ...mise/installs/go/<version>/go/go.env.

Out of curiosity I just copied go.env up a directory, so this actual error was gone, but other ones popped up unsurprisingly:

Trying to install dependencies in a Go project resulted in errors that the std library wasn't present like sync: package sync is not in std....

This led me to add the proper path back to mise via this PR, as `GOROOT´ wasn't aligned with my mise-issued installations of Go. But now knowing this has also changed in the installation process made up my question on how to properly solve this, i would call it, migration issue.

Probably the easiest way to reproduce would be:

  1. install mise <2024.2.8
  2. install Go 1.22
  3. upgrade to latest mise
  4. try to use Go

Copy link
Contributor Author

@wheinze wheinze Feb 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tl;dr I wouldn't propose to merge this PR anymore, as I think other solutions would be more valuable here.

@jdx what do you suggest on how to proceed here? should I close this PR and we just keep this in mind, whenever other users might encounter some issues related to this changes?

Copy link
Owner

@jdx jdx Feb 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't heard any other reports around this. We could just modify GOROOT if we detect the "go" directory is in the install dir

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't heard any other reports around this. We could just modify GOROOT if we detect the "go" directory is in the install dir

yep, sounds reasonable, i added a check: 54eb463

@wheinze wheinze changed the title fix(go): use proper path for GOROOT fix(go): reflect on proper path for GOROOT Feb 15, 2024
@jdx jdx enabled auto-merge (squash) February 15, 2024 14:21
@jdx jdx merged commit aed9563 into jdx:main Feb 15, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants