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 geting a repo with a go.mod file does not honor the replace directive #30354

Closed
jtarchie opened this issue Feb 22, 2019 · 8 comments
Closed

Comments

@jtarchie
Copy link

What version of Go are you using (go version)?

$ go version

go version go1.11.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/pivotal/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/pivotal/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.4/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.4/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/pivotal/workspace/om/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/0y/h4cjh45d0bq37xl93_ry97jh0000gn/T/go-build049197697=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

We have a binary dependency that people install via go get or go install.
When they do, they received an error of a missing type.
This is because the type is associated with a dependency is coming a forked version of that dependency, using the replace directive in the go.mod file.

What the user sees:

~: go get github.com/pivotal-cf/om
# github.com/pivotal-cf/om/commands
go/src/github.com/pivotal-cf/om/commands/s3_client.go:64:3: undefined: s3.ConfigV2Signing

If you build it manually, it works:

~: git clone https://github.com/pivotal-cf/om
~: cd om
~/om: go build main.go
~/om: ./main -v
unknown

What did you expect to see?

An error signifying that the go get did not honor the go.mod file when pulling down dependencies.

What did you see instead?

It works normally as expected.

@antong
Copy link
Contributor

antong commented Feb 22, 2019

I think replace only works in the top-level main module:

exclude and replace directives only operate on the current (“main”) module. exclude and replace directives in modules other than the main module are ignored when building the main module.

@bcmills
Copy link
Contributor

bcmills commented Feb 22, 2019

This is working as designed. replace directives have no effect outside of your own module, so if you invoke go get outside of your own module, they have no effect.

replace is intended to support local development, not permanent forks: if you need to permanently fork a dependency, your fork must have its own, unique import path.

@bcmills bcmills closed this as completed Feb 22, 2019
@jtarchie
Copy link
Author

jtarchie commented Feb 22, 2019

Can you define local development in this perspective?

If you are defining from a local machine, yes. Sure this works. If it is just me.

As I have a team, that is distributed. For me to then have something that they have to work with, I have to change an entire import path of a rather large project.

The replace directive allows me to give them something to develop against, while it is determined from a project development standpoint, do we keep the fork or wait while a PR (the exact reason this fork exists at the moment) is accepted.

The comment "This is working as designed". Does not necessarily mean that it cannot be changed. This made it feel that you were trying to understand my usage.

I'm on a large team, which has been trying to embrace the new workflow, but has had difficulty learning the hard edges that go mod introduces.

The wiki page does its best to address these. But from UI perspective, if I am trying intuit features, without any feedback from the CLI that I am doing something wrong, I'm going to think it is a bug or missing feature.

@bcmills
Copy link
Contributor

bcmills commented Feb 22, 2019

For me to then have something that they have to work with, I have to change an entire import path of a rather large project.

Or you can change the install instructions: you can ask them to git clone your module, or to run go mod init && go mod edit -replace some/module=some/other/module && go get github.com/pivotal-cf/om.

Building binaries with ad-hoc patches of dependencies is fairly common today with GOPATH and vendor. That's one of the things we're trying to get away from in module mode.

For example: suppose that one of your users experiences a panic in the dependency and reports it upstream. The upstream maintainer never released that version, never tested it, and knows nothing about it: how are they supposed to diagnose it? On the other hand, if your users have explicitly added a replace directive or built from within your repository, they'll know that the dependencies aren't pristine and that they should talk to you before reporting upstream issues.

@jtarchie
Copy link
Author

jtarchie commented Mar 8, 2019

I don't agree with this workflow.

I as a consumer of an upstream dependency A, shouldn't care up its dependencies. I just A and whatever it sees best for itself to run.

The fact that I have to know and lock dependency A's upstream dependencies is a leaky abstraction. It means I have to worry more about using dependency A, rather than trusting upstream that they have a working go.mod file that has been tested against.

Related to PRs and changing all the import statements. That's just needless maintenance.

We are currently on this fork, for the sole purpose of we have a PR to the core master. When that's accepted (based the upstream maintainer's time frame), we could pull it in just by removing the replace directive and everything would still Just Work™.

Having to have my fork change all the import statements also affects my upstream PR. Meaning there would be a commit on the PR of all the import statements changing from github.com/username/repo to github.com/jtarchie/repo. And that upstream maintainer would not accept that PR.

So that would require me to have to branches. One that is the PR and one that is the PR + the import statement naming change. And for any reason the PR has to be changed (say maintainer request/comments), we'd have to update two branches.

The fork that I have is not permanent, it is a temporary issue.

The replace directive perceptual intention is to alias. Having that definition be consumed by downstream users of our library would be beneficial.

I understand that this a feature working as designed. But that doesn't mean it cannot be changed.

nikkolasg added a commit to drand/kyber that referenced this issue Dec 28, 2019
gonzaloserrano added a commit to gonzaloserrano/netcheck that referenced this issue Jan 21, 2020
42wim added a commit to 42wim/matterbridge that referenced this issue Mar 8, 2020
42wim added a commit to 42wim/matterbridge that referenced this issue Mar 8, 2020
42wim added a commit to 42wim/matterbridge that referenced this issue Mar 8, 2020
simu added a commit to vshn/go-icinga2-client that referenced this issue Mar 27, 2020
This addresses golang/go#30354 which states
that `replace` is not to be used for importing a fork, leading to issues
when trying to `go get` vshn/signalilo.
simu added a commit to vshn/signalilo that referenced this issue Mar 27, 2020
This is necessary because `replace` is not supported by `go get` to
import a fork of a module, see golang/go#30354
simu added a commit to vshn/signalilo that referenced this issue Mar 27, 2020
This is necessary because `replace` is not supported by `go get` to
import a fork of a module, see golang/go#30354
@tooolbox
Copy link

Everything that @jtarchie said about the workflow for using & merging patches is true. It's pretty terrible.

@bcmills can this be reconsidered? Is it a matter of creating a proposal?

@jtarchie
Copy link
Author

@tooolbox, don’t bother. This is the official method to propose a new feature is the mailing list.
I’ve just not taken the time because its exhausting having so methods to be denied. 😦

The +1 method does not work here.

@jakesylvestre
Copy link

I'm in complete agreement with @jtarchie, this is a huge pain. We'll have a single breaking change in a dependency we don't control propogate across all our repos and have to add a replace on all of them.

gcla added a commit to gcla/termshark that referenced this issue Jan 18, 2021
I am doing this is an attempt to make go get work again with branch
targets other than master. The problem is that if you issue

go get github.com/gcla/termshark/v2/cmd/termshark@somebranch

then Go doesn't apply the replace directives from termshark's go.mod; I
think because it considers termshark.go's package main to not be part of
the module. Here's a description of the issue:

golang/go#30354

I certainly did not appreciate this until I read it. This is an
experimental branch to see if I can go get from a branch by moving all
the logic of main() under the umbrella of termshark's go.mod.
galeone added a commit to galeone/tfgo that referenced this issue Feb 2, 2021
@golang golang locked and limited conversation to collaborators Nov 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants