Closed tamalsaha closed 2 years ago
oiooj
commented
6 years ago Checked two imports:
$ git log -n1 --format=format:%H a2bb5ac932dfea981d79ab395a0ab4ea09365d52
fatal: bad object a2bb5ac932dfea981d79ab395a0ab4ea09365d52
tamalsaha
commented
6 years ago You can see from our glide.yaml file that these are both forked packages: https://github.com/pharmer/pharmer/blob/master/glide.yaml
- package: github.com/spf13/pflag
repo: https://github.com/appscode/pflag.git
vcs: git
version: master
- package: github.com/graymeta/stow
repo: https://github.com/appscode/stow.git
version: master
vcs: gitSo, commit has to be searched in the fork, I believe.
oiooj
commented
6 years ago You can add a replace into go.mod :
replace "github.com/graymeta/stow" a2bb5ac932dfea981d79ab395a0ab4ea09365d52 => "github.com/appscode/stow" a2bb5ac932dfea981d79ab395a0ab4ea09365d52@oiooj , the issue is that vgo build does not generate any go.mod file that I can modify manually. What I am expecting as an user is vgo build automatically does that for me, since all necessary information is already there.
bcmills
commented
6 years ago vgo build could replicate the replacements from your glide.lock locally as replace directives, but that wouldn't work for other modules that import your (non-main) packages: by design¹, replace directives only apply locally.
It seems that in order for this repository to be compatible with vgo, you will need to rewrite the imports to point to the forks directly.
This closely relates to one of the examples in Part 9 of @rsc's blog series:
As another example, Go import paths are URLs. If code said
import "uuid", you’d have to ask whichuuidpackage. Searching foruuidon godoc.org turns up dozens of packages. If instead the code saysimport "github.com/pborman/uuid", now it’s clear which package we mean. Using URLs avoids ambiguity and also reuses an existing mechanism for giving out names, making it simpler and easier to coordinate with other programmers.
¹ See the “Replacing Modules” section of https://research.swtch.com/vgo-mvs.
bcmills
commented
6 years ago That said, it does seem like vgo build could be more error-tolerant: it could ignore errors for unresolvable packages and write out the rest of the go.mod file.
It seems likely that you'd still need to rewrite the imports in order for vgo to be able to build the module, so I'm not sure whether that level of error-tolerance is really any help: it would change the sequence of operations, but wouldn't save you any work overall.
rsc
commented
6 years ago Note that #25692 was an instance of this in govendor.
gopherbot
commented
6 years ago Change https://golang.org/cl/120075 mentions this issue: cmd/go/internal/modconv: support conversion preserves replace and exclude statement
tamalsaha
commented
6 years ago @rsc , I tried with the go1.11beta2 binary. I ran the following command:
env GO111MODULE=on go1.11beta2 mod -init -module github.com/appscode/voyager
You can see the generated go.mod file here: https://github.com/appscode/voyager/pull/1200/files
I see that it is missing all the forks. I also see that it is missing transitive dependencies. For example, in the glide.yaml file we have
- package: k8s.io/apimachinery
repo: https://github.com/pharmer/apimachinery.git
vcs: git
version: release-1.11
- package: k8s.io/apiserver
repo: https://github.com/pharmer/apiserver.git
vcs: git
version: k-1.11to fix some bugs in upstream Kubernetes repos. I don't see k8s.io/apimachinery in the generated go.mod file. I am still learning vgo. So, pardon my ignorance.
rsc
commented
6 years ago @tamalsaha It reads glide.lock, not glide.yaml. It doesn't have the lines for apimachinery because it doesn't convert replacements (the hashes listed in glide.lock don't exist in the main repos). If we started converting replacements (and first understood exactly what that meant, which we don't quite), then more might work. But I'm glad you are getting a go.mod now.
We won't be doing replacement conversions for Go 1.11.
krasi-georgiev
commented
6 years ago what happens when we see something like unknown revision e770c10b0f....... in the logs?
does it download the latest master or?
maybe it should be more explicit in the logs what happens there.
dmitris
commented
5 years ago We won't be doing replacement conversions for Go 1.11.
@rsc @sdboyer - do you think we will do for Go 1.12 (in particular, for Gopkg.lock files)? If you need a guinea pig for testing, I can gladly try any alpha/beta versions 😄
Should we close https://github.com/golang/go/issues/24087 as duplicate of this one, or keep them separate?
dmitris
commented
5 years ago I am able to generate go.mod from Gopkg.lock with the source fields converted to replace lines with this patch (demo PR inside fork): https://github.com/dmitris/go/pull/1/files (diff: https://gist.github.com/dmitris/9cd098a7b565f027bce16445cc2de278). Does it look sensible and clear to you? Should I add tests and raise a 'real' change request - though it would be somewhat competing with https://go-review.googlesource.com/c/go/+/126915 from https://github.com/golang/go/issues/24087 (/cc @oiooj)
tmm1
commented
5 years ago
vgo buildcould replicate the replacements from yourglide.locklocally asreplacedirectives, but that wouldn't work for other modules that import your (non-main) packages: by design¹,replacedirectives only apply locally.It seems that in order for this repository to be compatible with
vgo, you will need to rewrite the imports to point to the forks directly.¹ See the “Replacing Modules” section of https://research.swtch.com/vgo-mvs.
I have a fork of x/net with some fixes, and I use glide.lock to ensure my fork is used by all of my other dependencies. Is there any way of achieving this with vgo, short of forking each of my dependencies that may be using x/net and search/replacing their imports?
Helcaraxan
commented
5 years ago
vgo buildcould replicate the replacements from yourglide.locklocally asreplacedirectives, but that wouldn't work for other modules that import your (non-main) packages: by design¹,replacedirectives only apply locally. It seems that in order for this repository to be compatible withvgo, you will need to rewrite the imports to point to the forks directly. ¹ See the “Replacing Modules” section of https://research.swtch.com/vgo-mvs.I have a fork of
x/netwith some fixes, and I useglide.lockto ensure my fork is used by all of my other dependencies. Is there any way of achieving this with vgo, short of forking each of my dependencies that may be usingx/netand search/replacing their imports?
I would very much like to have an answer to that very question as well. Currently trying to move a fairly large codebase to being a module on Go 1.11.x and the inability to override third-party dependency requirements to use a specific patch version of another third-party project is problematic (*).
(*) Problematic as in requiring multi-month engineering work with significant inherent risk to update our fork to a newer version of the upstream which contains breaking changes between minors (golang/protobuf#607 sigh).
rafaelvanoni
commented
5 years ago I see that support for dep source has been integrated, is there an open issue to support glide's repo: directive? I'd love to contribute that bit.
thepudds
commented
5 years ago If anyone is looking at this for other tools, Go 1.13 picked up the ability to convert the source directive for dep when running go mod init in CL 126915. That CL might provide a roadmap for similar support for another tool.
Note: there probably first should be agreement from someone like @jayconrod or @bcmills that a given tool is worth pursuing, though.
rafaelvanoni
commented
5 years ago @jayconrod @bcmills Please let me know if that's something you'd like to see implemented
bcmills
commented
5 years ago @rafaelvanoni, that seems fine once the 1.14 tree opens, provided that the amount of code isn't huge.
thepudds
commented
5 years ago @rafaelvanoni just to briefly expand slightly more on Bryan's point, it's 100% fine to send a CL now, or whenever you might have the time. (Bryan's point is that it wouldn't get merged or probably reviewed until the 1.14 tree opens).
rafaelvanoni
commented
5 years ago Cool, just wanted to avoid duplicating work.
bcmills
commented
4 years ago Any further replace support should be contingent on fixing the existing source-independent bug for converted replace directives (#33406).
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (
go version)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env)?What did you do?
If possible, provide a recipe for reproducing the error. A complete runnable program is good. A link on play.golang.org is best.
I checked out master branch of https://github.com/pharmer/pharmer repo and ran
vgo build. This repo uses glide currently.What did you expect to see?
go.modfile.What did you see instead?