-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
cmd/go: go install cmd@version errors out when module with main package has replace directive #44840
Comments
L28 contains a replace directive
Which is not valid for
Closing as working as intended |
yes obviously line 28 contains the directive. see the title. and then see this thread where I was told to open this issue. |
is the relevant line of that statement here. the fact is this directive wouldn't cause it to be interpreted differently than if it was the main module. and to any outside viewer, there is a main module, its the one being installed. |
reopening, I would recommend rephrasing the issue in terms of a proposal to relax the behaviour rather than as a bug report |
@seankhliao I'm just not sure if it is a bug or a proposal, the discussion around go installs (new) behavior was long, varied, and confused. =) this particular problem was pointed out during that discussion and seemingly ignored/disregarded/left open to be revisited. so here we are. |
#40276 was indeed discussed at length. Indeed, over the years even prior to that specific proposal, this issue has been discussed in various forms, many times. To say that the question of Deferring to @jayconrod on how best to take the discussion forward. |
I meant it was at one of those 3 at various points in that (and the related) thread. I wasn't calling out any particular stage, sorry for that confusion. |
I think there may be confusion around the concepts of a main module and the package main. In the case in this issue Also note that it is not something that used to work, it is a brand new feature that limits the number of cases in which it is enabled. Maybe we should change the terminology for main module to a different word? |
while technically true
returns the patched function from the replace + vendor workflow, and if you started with a clean environment there would be no other sources of the library in question. now did it work if trying to use a package as a dependency? no, and that was fine because it wasn't about using it as a library. many go programs are never intended to be used that way. now the rest of your argument is attempting to explain away for current limitations after the fact that go get / go install commands were fundamentally changed to support go modules while simultaneously ignoring vendor workflows of the tools and making that argument with a distinction that never existed until the tools were changed. now all that being said: installing a binary is different from managing dependencies. I believe this is an accepted fact and a reason go get was changed to be about managing dependencies and go install is being shift towards building and installing binaries. this fundamentally is a good change and I'm wholeheartedly behind it. I think the question we need to be asking is if the distinction between main package of the main module during installation matter? I'm going to assert it does not and that this error is erroneous, but during dependency management where someone tried to add a dependency that had a replace directive this error would be 100% correct though I'd start out with a warning in that case vs a hard error. |
Ah I believe I have a good explanation about why this should be supported. the replace directive is fundamentally about compilation, its meant to inform the build system how to patch dependencies. this is true both during development where you may need to patch them while debugging a problem which is when you compile your program. and during installation of a main module (previously the main package) onto a system. so during compilation there is no distinction between the main package and the main module, however, during dependency management replace directives have always been ignored and any software that was relying on a dependency where there was a replace directive was broken (conceptually if not in practice). and this is where we should be erroring out (i.e. in go get in this new world) when we see a replace directive. not during compilation (go install). |
Closing this issue since it's working as described in #40276. That being said, at some point in the future when we have more information, it will be reasonable to open a proposal to either ignore or apply I know this is a lot to ask (those issues are very long), but the main point of disagreement was what to do with To restart that discussion, we need new information: how is the new |
@jayconrod wrote:
As a go user, I reasonably expect Instead I get an extremely cryptic message like this
As a user of said From the instructions for
Which says the same thing as the error message, more or less. So it is consistent, but not helpful in my opinion! It is very hard having a large open source project without a few soft forks while you are waiting for upstreams to take your patches. I first noticed this problem with github's
|
Because we have `replace` statemetns in `submariner-operator/go.mod` to pin the K8s version, when run from outside of the submariner-operator repo `go install` declines to build `subctl` with: The go.mod file for the module providing named packages contains one or more replace directives. It must not contain directives that would cause it to be interpreted differently than if it were the main module. This issue is described upstream here: golang/go#44840 (comment) Signed-off-by: Daniel Farrell <dfarrell@redhat.com>
Because we have `replace` statements in `submariner-operator/go.mod` to pin the K8s version, when run from outside of the submariner-operator repo `go install` declines to build `subctl` with: The go.mod file for the module providing named packages contains one or more replace directives. It must not contain directives that would cause it to be interpreted differently than if it were the main module. This issue is described upstream here: golang/go#44840 (comment) Signed-off-by: Daniel Farrell <dfarrell@redhat.com>
Go dependencies are handled by package, not by module, and per-package splits are handled correctly. Having a separate package makes it easier to track dependencies, but it can cause friction when handling sub-module updates, and it leads to difficulties with go install (see <golang/go#44840>). Signed-off-by: Stephen Kitt <skitt@redhat.com>
Go dependencies are handled by package, not by module, and per-package splits are handled correctly. Having a separate package makes it easier to track dependencies, but it can cause friction when handling sub-module updates, and it leads to difficulties with go install (see <golang/go#44840>). Signed-off-by: Stephen Kitt <skitt@redhat.com>
Go dependencies are handled by package, not by module, and per-package splits are handled correctly. Having a separate package makes it easier to track dependencies, but it can cause friction when handling sub-module updates, and it leads to difficulties with go install (see <golang/go#44840>). Signed-off-by: Stephen Kitt <skitt@redhat.com>
Someone flagged recent discussion on this closed issue to me privately. As a general note, we don't watch closed issues very carefully - they're closed. Replace directives remain for local development. If you want to post software that others can go install, it needs to build without the replace directives. If we respected the replace directives during go install foo@latest, then that would create commands that can be installed that way but not using 'go get foo; go install foo' in a module where you are trying to track the version of foo you are using. We are also discussing integrating tools more explicitly with a module in #48429, and again there it would be a serious problem if tools existed that worked with 'go install foo@latest' but not with a regular build graph from another module. (And it is a non-starter for one tool in your module to force the use of its own replace directives. What if you don't want those replace directives? Or what if different tools have conflicting replace directives?) If you are using dependencies that don't function without replace directives, that may be a sign to rethink the use of those dependencies, or to upstream fixes that make the replace directives unnecessary. For the specific case of go4.org/unsafe/assume-no-moving-gc, there are two better answers than replace:
These work within the ecosystem and keep it healthy. Replace directives fragment the ecosystem and make it brittle. My post The Principles of Versioning in Go has a worked example. It looks at forced package downgrades, but replaces are analogous andhave all the same problems. We are aware of the problem of go4.org/unsafe/assume-no-moving-gc specifically. In May we worked with the upstream author on a change that makes it no longer break at each new Go release. So if you run 'go get go4.org/unsafe/assume-no-moving-gc@latest' in your own module one last time, you should stop seeing breakages from that package, and you can remove the replace. |
Hi @rsc, Thankyou for the above, but it seems to avoid addressing the key use case I and others in this issue seem to want: The particular problem is with the statement
which ignores a somewhat common practice:
It seems like you would class this as a misuse of replace directives based on what you said above? The problem with that, is that the Go project has given developers an almost perfect tool - replace directives - for dealing with this, so it is unrealistic to think people would not use replace directives in this scenario. AFAIK there are no other options that don't involve needing to change import paths of the forked software in every source file that consumes it, which is inelegant in the extreme! It's not like the users trying to use So you end up with unhappy users who have had the lovely convenience of |
@rsc: My understanding of this issue is that we care about the use case where you use Installing inside a go module is tricky as you describe. And there is no need to support it there because inside a go module you can just modify go.mod yourself and add the replace directive there. So, could we get |
Some more anecdotal evidence as to why this is an issue: We are using the k8s client libs, but recently this change was introduced in the openAPI Spec models: Now we can't upgrade
So we are left with:
Being a Lazy (Pragmatic...¯_(ツ)_/¯) Programmer, I chose option 3, as I think most Go Devs would. Now I can upgrade the module I want, without any of the side effects. It just works. ...But now we can't I would also argue that although the replace directive may have been intended for local development purposes, it is widely used across go projects to circumvent issues exactly like the one I posted here. Dependencies can get messy, and being able to "strategically" use a replace directive when necessary is a nice way to work around (hack) those issues. |
Regarding replace directives being "only for local development" reading back to the original vgo and modules proposals I find this about replace. From vgo tour
From Minimal Version Selection
(emphasis mine) also from Minimal Version Selection, section Who controls your build?
From the modules proposal:
(emphasis mine) From the Q&A section of the modules proposal:
PS. I couldn't find any mention in the proposal or in the vgo posts about replace being only appropriate for local builds but AFAICT the proposal did not introduce the distinction between local vs non-local builds, AFAICT that was initially introduced as a consequence of implementation details of |
This is temporary as long as gdamore/tcell#599 is not merged. Once that PR is merged, we can revert this.
* Run go install after go get to install plugins This fixes ignite#1163, but is not ideal... tooling binaries should be installed by running `go install [path]@[version]`, but protoc-gen-gocosmos uses a replace directive in the `go.mod` which is [incompatible with this method](golang/go#44840 (comment)). This fixes the problem for now, but adds plugin specific items to the go.mod of the scaffolded project, which shouldn't be there. * Update starport/pkg/cosmosgen/install.go Co-authored-by: İlker G. Öztürk <ilkergoktugozturk@gmail.com>
* Run go install after go get to install plugins This fixes ignite#1163, but is not ideal... tooling binaries should be installed by running `go install [path]@[version]`, but protoc-gen-gocosmos uses a replace directive in the `go.mod` which is [incompatible with this method](golang/go#44840 (comment)). This fixes the problem for now, but adds plugin specific items to the go.mod of the scaffolded project, which shouldn't be there. * Update starport/pkg/cosmosgen/install.go Co-authored-by: İlker G. Öztürk <ilkergoktugozturk@gmail.com>
See golang/go#44840 (comment) for why these are problematic for 'go install' and 'go get'.
See golang/go#44840 (comment) for why these are problematic for 'go install' and 'go get'.
So what do you do if you encounter a replace? lol I just installed go for the first time to use godo.
I don't get it. (get it?) What's |
@seanbethard you must git clone that project and run "go install ." within the directory. |
Our dependency tree pulls in the same or newer versions of /x/crypto and /x/text, remove the replace directives. This helps with go install (see <golang/go#44840>). Signed-off-by: Stephen Kitt <skitt@redhat.com>
Go dependencies are handled by package, not by module, and per-package splits are handled correctly. Having a separate package makes it easier to track dependencies, but it can cause friction when handling sub-module updates, and it leads to difficulties with go install (see <golang/go#44840>). Signed-off-by: Stephen Kitt <skitt@redhat.com>
This would be a lot less of a problem if the
It would still be a problem, but it would be significantly less of one. |
I think this bug intentionally exists to prevent people from leaning on replace directives instead of upstreaming their fixes. |
As someone who is using a lot of replace directives, this is a quite annoying limitation for go install. Example: https://github.com/aperturerobotics/bifrost#examples Here I have to say - well, you could use go install, but we have replace directives, so instead clone it and build. It defeats the purpose of go install. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
yes
What did you do?
What did you expect to see?
go install finishing successfully
What did you see instead?
given that genieql is the main module this error appears to be erroneous.
other attempts I made all of which i would have expected to work properly (and use to):
The text was updated successfully, but these errors were encountered: