search

Homebrew / homebrew-core

🍻 Default formulae for the missing package manager for macOS (or Linux)
https://brew.sh
BSD 2-Clause "Simplified" License
13.42k stars 12.2k forks source link

[epic] Remove GOPATH for go modules formula #47627

Closed chenrui333 closed 10 months ago

chenrui333 commented 4 years ago

Historically, golang builds need to happen under the GOPATH, as more and more formula has moved to go-modules, we can deprecate the GOPATH usage.

jonchang commented 4 years ago

Looks like there are 291 formula that still use GOPATH.

* [x] acmetool * [ ] ahoy * [x] akamai * [x] algernon * [x] aliyun-cli * [x] alp * [ ] amazon-ecs-cli * [x] annie * [x] antibody * [x] anycable-go * [ ] apache-brooklyn-cli * [x] apm-server * [ ] aptly * [x] armor * [x] assh * [x] atlantis * [x] auditbeat * [x] aurora * [x] aws-es-proxy * [x] aws-iam-authenticator * [x] aws-okta * [x] benthos * [x] bettercap * [x] bitrise * [x] borg * [x] buildkit * [x] c14-cli * [x] calicoctl * [x] cayley * [x] certstrap * [ ] cf-tool * [x] cfssl * [x] chamber * [x] charm * [x] chronograf * [x] cig * [x] circleci * [x] cli53 * [x] clipper * [x] cointop * [x] collector-sidecar * [x] consul-backinator * [x] consul-template * [x] consul * [x] container-diff * [x] convox * [ ] corectl * [x] cql * [x] ctop * [ ] cwlogs * [x] dashing * [x] dcos-cli * [x] deis * [x] deisctl * [ ] dep * [ ] devd * [x] devspace * [x] direnv * [x] dive * [x] dnscontrol * [x] dnscrypt-proxy * [ ] docker-credential-helper-ecr * [ ] docker-credential-helper * [x] docker-gen * [x] docker-ls * [ ] docker-machine-driver-hyperkit * [ ] docker-machine-driver-vmware * [ ] docker-machine-driver-vultr * [ ] docker-machine-parallels * [ ] docker-machine * [ ] docker-swarm * [ ] docker * [ ] docker2aci * [ ] dockerize * [ ] dockviz * [ ] dockward * [x] doctl * [x] drone-cli * [ ] dvm * [x] elvish * [ ] emp * [x] envconsul * [x] etcd * [x] exercism * [x] faas-cli * [x] fabio * [x] filebeat * [ ] fleetctl * [ ] flint-checker * [x] fluxctl * [x] fn * [ ] forego * [x] fork-cleaner * [x] fortio * [x] frpc * [x] frps * [x] frugal * [x] fsql * [x] gauge * [ ] gdm * [x] gdrive * [x] geoipupdate * [x] ghr * [ ] git-appraise * [x] git-sizer * [x] git-town * [x] github-markdown-toc * [ ] github-release * [x] gitlab-runner * [ ] glide * [ ] glooctl * [ ] go-bindata * [x] go-jira * [x] go-md2man * [ ] go * [ ] go@1.10 * [ ] go@1.11 * [ ] go@1.12 * [ ] go@1.9 * [ ] goad * [x] gobuster * [ ] gocryptfs * [ ] gofabric8 * [x] gollum * [x] gomplate * [x] goose * [ ] gopass * [x] gor * [x] goreman * [x] gost * [x] gotags * [ ] govendor * [x] gowsdl * [ ] gox * [x] grafana * [x] gron * [ ] grv * [ ] gx-go * [x] gx * [x] heartbeat * [x] helm * [ ] helm@2 * [x] helmsman * [x] hey * [x] hivemind * [x] hostess * [x] hub * [x] iamy * [x] immortal * [ ] influxdb * [ ] infrakit * [x] inlets * [ ] ipfs * [x] iron-functions * [ ] ironcli * [ ] istioctl * [ ] jabba * [x] jd * [x] jfrog-cli-go * [x] jid * [ ] jp * [x] juju * [x] jump * [x] jvgrep * [x] k3d * [x] k6 * [x] kapacitor * [x] karn * [ ] kedge * [x] kompose * [ ] kops * [x] kube-aws * [x] kubeaudit * [x] kubebuilder * [x] kubeless * [x] kubeprod * [ ] kubernetes-cli * [x] kubernetes-service-catalog-client * [x] kyma-cli * [ ] landscaper * [x] lean-cli * [x] leaps * [x] lego * [x] lf * [x] linkerd * [ ] lxc * [x] mage * [ ] mailhog * [ ] massren * [x] megacmd * [x] metricbeat * [x] micro * [x] minio-mc * [x] minio * [x] mmark * [ ] modd * [ ] mpdviz * [x] nats-server * [x] nats-streaming-server * [x] node_exporter * [x] nomad * [x] nsq * [x] oauth2_proxy * [x] octant * [x] opa * [ ] openshift-cli * [ ] operator-sdk * [x] overmind * [x] packer * [x] peco * [ ] perkeep * [x] pgweb * [x] piknik * [ ] pilosa * [x] prest * [x] protoc-gen-go * [x] prototool * [ ] pulumi * [x] pumba * [ ] pup * [ ] qpm * [ ] rack * [x] rancher-cli * [ ] rancher-compose * [x] rke * [x] s-search * [x] scc * [x] scw * [x] serve * [x] shellz * [x] shfmt * [x] ship * [ ] sift * [ ] skaffold * [x] skopeo * [x] slackcat * [x] slacknimate * [x] smimesign * [ ] snag * [ ] snap-telemetry * [x] sonobuoy * [x] spaceinvaders-go * [x] srclib * [x] ssh-vault * [x] ssllabs-scan * [ ] step * [x] stolon * [ ] stout * [x] syncthing-inotify * [ ] syncthing * [x] td * [ ] teleconsole * [x] telegraf * [x] teleport * [ ] termshare * [x] terraform-docs * [x] terraform-inventory * [x] terraform * [ ] terraform@0.11 * [x] terraformer * [x] terragrunt * [x] terrahelp * [ ] textql * [x] the_platinum_searcher * [x] traefik * [x] traefik@1 * [x] triangle * [x] ultralist * [ ] uru * [ ] vault * [x] vegeta * [x] velero * [ ] vert * [x] virgil * [x] virustotal-cli * [x] vultr * [x] warp * [x] websocketd * [x] wego * [x] wellington * [x] wsk * [x] wskdeploy * [x] wu * [x] yq
alebcay commented 4 years ago

While we're here - would it be worth standardizing some of the flags that we use for building Go formulae? Specifically, I would advocate for always including

jonchang commented 4 years ago

It looks like Debian sets -trimpath but doesn't strip binaries with -s -w. Would it be useful to add something like std_go_args (like we have for CMake builds)?

alebcay commented 4 years ago

Would it be useful to add something like std_go_args (like we have for CMake builds)?

I think it would be useful. I've already seen some formulae floating around with stripped binaries, which is why I've brought this up. From my limited testing with the PRs above, there's a reduction of 15-20% in total formula size. I personally don't think it would be a bad idea to strip binaries, seeing as Go binaries are all statically linked.

SMillerDev commented 4 years ago

Maybe you want to wait @chenrui333 and do gopath at the same time as the go_std_args?

chenrui333 commented 4 years ago

Would it be useful to add something like std_go_args (like we have for CMake builds)?

+1 for std_go_args idea.

We can definitely pipeline the work between std_go_args and gopath removal though. After that we can refine our template for easy bootstrap golang formula.

FiloSottile commented 4 years ago

You can use the GOFLAGS environment variable to always set -trimpath. I'd recommend against stripping DWARF, as people might want to use a debugger or a profiler on their binaries.

SMillerDev commented 4 years ago

That's not what's suggested here, this is about GOPATH

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

chenrui333 commented 4 years ago

checkin, give some more time for this issue.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

chenrui333 commented 4 years ago

checkin, give some more time for this issue.

carlocab commented 3 years ago

Looks like many of failures in go1.16 (#71289) need fixing by removing GOPATH. These are the formula that have failed to build on both Mojave and Catalina:

chenrui333 commented 3 years ago

it is also a huge opportunity to disable/remove go formulae now.

we can definitely temporarily to have some go formulae build with go@1.15 though.

nklmilojevic commented 3 years ago

I have started from the bottom of the list, for now wego, vert and uru don't have go modules in their repos.

SMillerDev commented 3 years ago

Could you make sure to make an upstream issue for those (mentioning that this will break go 1.16 compatibility) and link them here? That way we can keep track of the status.

nklmilojevic commented 3 years ago

^ Will do it one by one

nklmilojevic commented 3 years ago

@SMillerDev it seems that all of these formulas that are left contain repos that don't have go.mod at all, so that is why there is GOPATH still there for install command. I think these formulas will have to update or keep depending on go@1.15.

MikeMcQuaid commented 3 years ago

it is also a huge opportunity to disable/remove go formulae now.

Let's try and only do this for Go formulae that aren't widely used. Remember we're primarily a binary package manager for our users; it doesn't matter to them if we can't build from source.

vvvvv commented 3 years ago

regarding mailhog:
There is already a ticket but the maintainer seems to be pretty unresponsive as the last contribution is from jun 2020

vvvvv commented 3 years ago

regarding termshare:
Last update was 7 years ago, maintainer doesn't answer issues or pr.
The project seems to be pretty much dead so I'd suggest removing it.

Bo98 commented 3 years ago

GO111MODULE=auto is probably an acceptable fix that can be done quite quickly for all of the above (can be batch scripted).

I'm quite happy 1.16 is making this change though. Formulae that are not built using Go modules have never had reproducible builds, and have had a history of breaking.

Going forward, we should probably not accept any new formula that uses GO111MODULE=auto/off (adding an audit to prevent it), but that's a debate for later.

vvvvv commented 3 years ago

regarding modd:
HEAD builds just fine as upstream already uses go modules in master not in the latest version.
I don't know what to do in this case, just change the version to commit 8983974 ? There's also already a ticket regarding arm support https://github.com/cortesi/modd/issues/95

vvvvv commented 3 years ago

regarding mpdviz:
Last update was 7 years ago.
Project seems to be dead.

vvvvv commented 3 years ago

regarding path-extractor:
Project seems to be dead.
@chenrui333 filed an issue on 29 Jul 2020 noting the absence of any license and maintainer didn't responde.

Bo98 commented 3 years ago

For path-extractor, that is a valid reason to disable the formula. disable! because: :no_license.

TimothyGu commented 3 years ago

I see that people have started to add GO111MODULE=auto to a bunch of packages. However, according to https://blog.golang.org/go116-module-changes, that won't work in Go 1.17:

We plan to drop support for GOPATH mode in Go 1.17. In other words, Go 1.17 will ignore GO111MODULE. If you have projects that do not build in module-aware mode, now is the time to migrate.

Instead of adding GO111MODULE everywhere, which is only a temporary band-aid, I think it would be better to properly convert formulae to not use GOPATH at all.

Bo98 commented 3 years ago

Instead of adding GO111MODULE everywhere, which is only a temporary band-aid, I think it would be better to properly convert formulae to not use GOPATH at all.

For pretty much all of the affected formulae, they don't use Go modules and only work in GOPATH. This requires upstream to release updates to correct. We've already put in a lot of effort to migrate to Go modules and the last group are out of our control.

GO111MODULE is a reasonable workaround so that Go 1.16 can ship this weekend. Otherwise, we're going to have to wait a lot longer while we wait for various updates from upstream.

Once Go 1.16 is shipped, we then have a known timeframe (until 1.17) for the affected formulae to be updated to use Go modules. If upstream do not issue an update to support this, then we will disable those formulae, marking as unmaintained or similar.

roopakv commented 3 years ago

@Bo98 would you be able to update the comment https://github.com/Homebrew/homebrew-core/issues/47627#issuecomment-780944517 to reflect the state of the world :)

more of these have been opened/fixed and if I'm going to be honest I'm confusing myself 😂

carlocab commented 3 years ago

Whoops; I got it. Give me a few minutes.

roopakv commented 3 years ago

Is there any reason I shouldn't update all formulae and add ENV["GO111MODULE"] for all applicable formula in one PR? or at least group in ~5s or 10s?

Since we are not actually updating a release is that allowed?

carlocab commented 3 years ago

or at least group in ~5s or 10s?

Something like this should be fine. However, please do check that they actually don't support modules before using this workaround.

roopakv commented 3 years ago

@carlocab this look ok? https://github.com/Homebrew/homebrew-core/pull/71610

carlocab commented 3 years ago

Yes, that's fine. Thank you!

roopakv commented 3 years ago

All right! we are all done ( i think). We have PRs for all the tools listed in @carlocab's comment.

While we have filed issues with a LOT of the tools there I doubt that many of them will actually upgrade to go modules by the time go1.17 is out. Would the idea be to deprecate them or disable them or add a go1.16 formula when that time comes?

Bo98 commented 3 years ago

Possibly both - we could deprecate as "unmaintained" if there is no response, but we'll let them depend on go@1.16 for that deprecation period.

I'm very likely going to start tests with 1.17 RC1 when that lands rather than the final version so that we're better prepared.

For 1.16: we just now need to double check a couple of the other failures but they should be easy to prove they are pre-existing or not and then we can rebase on master (with the bootstrap change we agreed on in https://github.com/Homebrew/homebrew-core/issues/71370#issuecomment-781541940).

carlocab commented 3 years ago

Thanks very much for all the help, @roopakv. There are still a handful of formulae that don't use GOPATH but nevertheless did failed with go1.16: https://github.com/Homebrew/homebrew-core/pull/71289#issuecomment-783077433

SMillerDev commented 2 years ago

Is this still a problem with current Go versions?

alebcay commented 2 years ago

It generally is not a problem (using GOPATH was never broken across the board, just in some corner cases like we've seen in the past); however usage of modern Go modules is definitely preferred. I would consider this on par with a style issue since I don't foresee Go fully disabling the deprecated functionality.

I may make one more pass through the list and post remaining items as a comment (for posterity). We can continue working on it if preferred or close this soon.

alebcay commented 2 years ago

Upstream HEAD has module support but has not landed in a release

Upstream is still not using modules yet

Deprecated/disabled

Corner cases

rahul3002 commented 1 year ago

I would like to contribute to this Project

SMillerDev commented 1 year ago

Go for it

rahul3002 commented 1 year ago

can anyone guide me

alebcay commented 1 year ago

Hi @rahul3002, I encourage you to take a look at our contributing guide for info on how to contribute in this repository (Homebrew/homebrew-core), which stores formulae (packages) for Homebrew.

You may also be interested in contributing to the main brew repo (https://github.com/Homebrew/brew), which contains the code for the package manager itself. You can also refer to the contributing guidelines for the brew repo for info on how to contribute there.

You can also find more information relevant to contributing at our docs site. I would ask that you to take further questions to our Discussions forum, as this thread is dedicated to other purposes. Thanks for your interest!

osalbahr commented 1 year ago

I see that the "in progress" label was removed but this issue is still open and there is still a "help wanted" label. What does that mean?

iMichka commented 11 months ago

Not sure for the in progress label. Looking at it, we have been working on this since 2019 ... so progress has been slow. There are less than 50 formulae where GOPATH could be removed, so any help finishing this task is welcome.

chenrui333 commented 10 months ago

Considering now we are at go 1.21, gonna use gopath deprecation for further tracking or formulae deprecations.