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: don't set GOTOOLCHAIN=local #202030

Open
4 tasks done
andig opened this issue Dec 21, 2024 · 17 comments
Open
4 tasks done

Go: don't set GOTOOLCHAIN=local #202030

andig opened this issue Dec 21, 2024 · 17 comments
Labels
bug Reproducible Homebrew/homebrew-core bug

Comments

@andig
Copy link
Contributor

andig commented Dec 21, 2024

brew gist-logs <formula> link OR brew config AND brew doctor output

❯ brew doctor
Your system is ready to brew.

Verification

  • My brew doctor output says Your system is ready to brew. and am still able to reproduce my issue.
  • I ran brew update and am still able to reproduce my issue.
  • I have resolved all warnings from brew doctor and that did not fix my problem.
  • I searched for recent similar issues at https://github.com/Homebrew/homebrew-core/issues?q=is%3Aissue and found no duplicates.

What were you trying to do (and why)?

Coming from golang/go#70949 where Go fails to install newer toolchain for compiling module. This is due to https://github.com/Homebrew/homebrew-core/blob/master/Formula/g/go.rb#L93. While this may be the right setting for compiling brew formulas, it breaks using Go for other purposes.

What happened (include all command output)?

go build

go: errors parsing go.mod:
go.mod:206: unknown block type: tool

What did you expect to happen?

Go 1.23 downloads Go 1.24 and compiles successfully.

Step-by-step reproduction instructions (by running brew commands)

n/a
@andig andig added the bug Reproducible Homebrew/homebrew-core bug label Dec 21, 2024
@carlocab
Copy link
Member

Does doing export GOTOOLCHAIN=auto not fix this for you?

@andig
Copy link
Contributor Author

andig commented Dec 21, 2024

Yes it does (or rather, I've used go env -w GOTOOLCHAIN=auto). Would it make sense to apply the local only for building brew formulae but not for client builds? I.e. stick it in another place? It feels wrong to improve formula build constraints onto totally unrelated client builds.

@carlocab
Copy link
Member

Per the caveats,

❯ brew info go
[snip]
==> Caveats
Homebrew's Go toolchain is configured with
  GOTOOLCHAIN=local
per Homebrew policy on tools that update themselves.
[snip]

so formulae build constraints aren't really relevant here (even if you are correct to suggest that we do want GOTOOLCHAIN=local when building formulae).

@andig
Copy link
Contributor Author

andig commented Dec 21, 2024

per Homebrew policy on tools that update themselves.

That refers to brew-installed tools? It doesn't seem to make much sense to impose this restriction on local dev work.

It feels (no pun intented!) that the real suggestion here it to use brew Go only as far as it's required for other formulas but switch to go.dev-downloaded go for local dev work.

carlocab added a commit that referenced this issue Dec 21, 2024
See #202030 and golang/go#70949.

In particular, our current `go.env` contains

    # Automatically download newer toolchains as directed by go.mod files.
    # See https://go.dev/doc/toolchain for details.
    GOTOOLCHAIN=local

The comment about automatically downloading newer toolchains applies
only whenever `GOTOOLCHAIN=auto`, but we change that to
`GOTOOLCHAIN=local`. Therefore, let's get rid of the comment that no
longer applies after our `GOTOOLCHAIN` change.
@carlocab
Copy link
Member

That refers to brew-installed tools?

Yes.

It feels (no pun intented!) that the real suggestion here it to use brew Go only as far as it's required for other formulas but switch to go.dev-downloaded go for local dev work.

You could do that. You could also configure GOTOOLCHAIN locally if local doesn't work for you. Not really sure why that's a huge impediment (and, indeed, a larger impediment than the effort of managing your own toolchains downloaded from go.dev).

@andig
Copy link
Contributor Author

andig commented Dec 21, 2024

It isn't once you understand what's happening. It just seems to me that setting this locally also defeats the purpose of the original setting regarding self-updating tools. So it's really a no-op to have this in the first place?

@carlocab
Copy link
Member

That argument holds only if every user of the Go formula sets GOTOOLCHAIN=auto locally. Do you have any evidence that this is the case?

@andig
Copy link
Contributor Author

andig commented Dec 21, 2024

Obviously not. But if users do so to retain standard go behaviour it does. So imho it makes the use of it as part of the Go formula instead of the Go-dependent formulas brittle.

@carlocab
Copy link
Member

In that case I'm not persuaded that it's a no-op.

It's certainly a no-op sometimes, if a user decides to override it. But it being sometimes a no-op is nowhere near as strong a reason to switch back to setting auto as it would be if it were always (or almost always) a no-op.

At this stage I'm not convinced we have good reasons to change this default given that:

  1. it is clearly documented (in caveats, plus the misleading comment in go.env has now been removed)
  2. it is trivial to override

But I'll tag @Homebrew/core here in case someone else has a different opinion.

@andig
Copy link
Contributor Author

andig commented Dec 21, 2024

I don't have strong feelings either way now that I'm aware of it. I can imagine though that- although caveat documented- this may be overlooked by other users, too. If that can be prevented by enforcing the policy in a better way that would imho be preferred over unexpected changed to the Go standard.

Thanks for looking into this!

gafreax pushed a commit to gafreax/homebrew-core that referenced this issue Dec 21, 2024
See Homebrew#202030 and golang/go#70949.

In particular, our current `go.env` contains

    # Automatically download newer toolchains as directed by go.mod files.
    # See https://go.dev/doc/toolchain for details.
    GOTOOLCHAIN=local

The comment about automatically downloading newer toolchains applies
only whenever `GOTOOLCHAIN=auto`, but we change that to
`GOTOOLCHAIN=local`. Therefore, let's get rid of the comment that no
longer applies after our `GOTOOLCHAIN` change.
@MikeMcQuaid
Copy link
Member

At this stage I'm not convinced we have good reasons to change this default given that:

  1. it is clearly documented (in caveats, plus the misleading comment in go.env has now been removed)
  2. it is trivial to override

I agree with this reasoning 👍🏻

@andig
Copy link
Contributor Author

andig commented Dec 23, 2024

If there was another mechanism for ensuring that formulas keep being compiled with the Go toolchain version that comes with the formula than the current one (non-standard go.env), and that mechanism did not impose not updating Go toolchains outside of formula usage, would you prefer such a mechanism over the current one?
I realise there may be work involved developing it and it might be more effort than the current solution.

@stefanb
Copy link
Member

stefanb commented Dec 23, 2024

From the brew maintainer's perspective it feels ok to use predictable, prescribed Go version to build a formulae and that can really be up to the maintainers to agree up on, as it generally should not affect the programs for end users to notice at all.

But from Go developer's perspective this changes the Go's behaviour and can be seen as breaking it, changing it compared to what would be installed from the official Go release channels, changing it for end users (Go developers).

@Bo98
Copy link
Member

Bo98 commented Dec 23, 2024

I guess the main point to me is: are you wanting to use Homebrew Go or upstream Go? If it's Homebrew Go then you need GOTOOLCHAIN=local because Go does not support auto-installing anything that's not upstream builds AFAIK. If it's upstream Go, then maybe you need upstream Go to begin with or to install something like goenv (which we have a formula for) that manages upstream builds.

We aren't the only package manager to do this. I checked a random few and Alpine, CentOS, Fedora and Gentoo all change the default.

Debian however notably don't do anything yet but have considered GOTOOLCHAIN=path. This is maybe something we could consider doing to make it automatically pick up older Go formulae like [email protected] etc. Though they would still need to be installed manually.

@andig
Copy link
Contributor Author

andig commented Dec 23, 2024

because Go does not support auto-installing anything that's not upstream builds AFAIK

Not sure what this means. I can use brew go and do

go env -w GOTOOLCHAIN=auto

and it will happily download other toolchains (from go.dev) as required by go.mod. This is enough to compile non-brew software that requires a different toolchain version.

If it's upstream Go, then maybe you need upstream Go to begin with

You don‘t afaiu. The main point for brew Go for me is that I can always have bleeding edge Go along with updates for anything else using brew.

@FiloSottile
Copy link
Contributor

Hello! I would like to plead for you not to disable the GOTOOLCHAIN mechanism.

First, I would argue it's very different from a self-update mechanism: the Go install in the Cellar doesn't change, and if you cd out of a module and run go version you always get the Homebrew version. I perfectly understand why you have a policy against self-updates, and I would hate for installed versions to get out of sync with the version tracked in Homebrew, but that's not what would happen here. This is more like downloading the correct version of a Go dependency specified in the go.mod. That dependency just happens to be the standard library and the compiler.

Second, it's very useful. Go is all about self-contained builds, and fetching the compiler if needed is a wonderful user experience. It even carries into VS Code: you can change the go.mod line and boom, new version within that module. Disabling it here turns all that off :(

Third, turning it off is kinda confusing. I would say I am more familiar with Go and Homebrew than the average user, and it took me quite a bit to figure out what was going on here. I think partially this is because Homebrew delightfully got me used to expecting the upstream defaults, unlike the heavyhanded approach of some distributions, so it took me a while before I realized what could be going on.

Thank you for listening ✨

@MikeMcQuaid
Copy link
Member

First, I would argue it's very different from a self-update mechanism: the Go install in the Cellar doesn't change, and if you cd out of a module and run go version you always get the Homebrew version.

I didn't realise this, thanks for explaining @FiloSottile.

This is more like downloading the correct version of a Go dependency specified in the go.mod.

This sounds pretty similar to how e.g. ruby-install functions.

I think partially this is because Homebrew delightfully got me used to expecting the upstream defaults, unlike the heavyhanded approach of some distributions, so it took me a while before I realized what could be going on.

This is something we aim for.

Given all this:

I agree with this reasoning 👍🏻

I no longer agree and think it'd be nicer to unset GOTOOLCHAIN=local and defer to upstream defaults outside of Homebrew itself (which should set this in the build environment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Reproducible Homebrew/homebrew-core bug
Projects
None yet
Development

No branches or pull requests

6 participants