There are several benefits:
GOPATH
GOROOT
This is based on #16, or it wouldn't compile otherwise.
There are still some difference that I am investigating, so it's not perfect just yet.
5 new commits added
Simplify code duplication in CollectProjectDeps.
Replace findPackageLocation by go.build.Import.
Replace Go internals with go/build.
Remove unused functions and make one private.
Fix compile against latest urfave/cli.
So for tests, I searched the specs for all that definegoipath and cloned all of them (note this is not all Go packages.) Then ran golist as packaged and golist from this PR and compared the results for all possible arguments.
goipath
golist
Current golist crashes on:
but this PR works again.
There are only 2 differences out of 510 packages with goipath (out of approximately 642 that mention golang at all):
Both of these report themselves in the --imported section, but the new one includes the version (i.e., github.com/cespare/xxhash -> github.com/cespare/xxhash/v2). I assume this might be a problem, since we don't add it in the provides, but I don't know for sure. Can you confirm @nim?
--imported
github.com/cespare/xxhash
github.com/cespare/xxhash/v2
As a note, the original code ran a resolve on the test imports, but not the regular imports. I tried adding a resolve on the regular imports as well, but it doesn't help. I also tried setting GO111MODULE=on, and while it does stop it from erroring, it still returns the same /v2 import path back.
GO111MODULE=on
/v2
The v2 is module versioning. That's something we need to support to have working Go packaging. It's good that your PR starts picking it up when the previous code does not.
However you are right that just doing it that way is going to break things. It needs changes in other parts of golist and in the macro code (and the current go-rpm-macros code should be in a good enough shape to allow those changes but that ’s not what is in Fedora).
Looking at how github.com/cespare/xxhash handled the module transition, assuming it is representative of the conversion of a real-world codebase to modules, here is the list of the things I see needing doing (paper analysis, real work implementation and testing is needed)
we have the new module descriptors. That means golist needs to include the go.mod file as part of the project files, otherwise it will not end up in the -devel package and rpm will (rightfully) ignore it when generating deps
go.mod
then once the go.mod file is in the devel package we need to check the golist call that generates the package provides actually sees it (should probably just work with the code in go-rpm-macros and will almost certainly fail in some case with the code in Fedora because some of the approximations I fixed this year and which are still in the Fedora macro code will definitely hurt module parsing)
then we have the fact that modules break the identity between the URL and the import path. The code is still at https://github.com/cespare/xxhash but the logical name is now github.com/cespare/xxhash/v2. No matter this is a case that is handled nicely by go-rpm-macros, that just means you need to declare github.com/cespare/xxhash/v2 as goipath and https://github.com/cespare/xxhash as forgeurl
https://github.com/cespare/xxhash
forgeurl
the module says v2 depends on v1 which means you need a separate v1 spec and package with just github.com/cespare/xxhash/ goipath (and be careful to pull from the V1 branch not the v2 one). If you package the v2 code as github.com/cespare/xxhash/ you are doing a mistake, that's not its real import path
github.com/cespare/xxhash/
And that should be sufficient to have github.com/cespare/xxhash/ work in a module world. However, this is a simple module conversion with a single go.mod per git repo, and no version constrains in this module file
but, what happens when a single git repo ships several go.mod files with a logical name that has nothing to do with the URL path ? I suspect things will break badly. the latest go-rpm-macros code is a lot more flexible, but it still assumes some form of identity between the filesystem layout and the import layout. How badly will this identity break with modules? I have no idea.
and then you have the version qualifiers in the module files, we need to map them rpm side one way or another
Ok, after more work evolving to module support seems a bit simpler than I though, because go mod need requires people to clean up their mess and that's also the assumption of the current go-rpm-macros.
Though it will definitely require the coding of a helper in go, since the json structure go mod edit outputs is pretty much unworkable in shell, lua is not available at this stage of the rpm build, and I don't think we need to inject a fourth language in our go tooling (after lua+shell due to how rpm works, and golang for golist).
The 1000-dollar question is how much this new helper can subsume current golist functions, or if we end up with 2 helpers because go module is almost there for our needs but not completely.
rebased onto 6652d08143a1f126aa1ad03c60bb4f0f20b24da5
I'm going to merge this, as those two packages are failing due to the way they use Go modules, and getting testing working everywhere is more important. They can just be patched to work right in the meantime. Of course, as more things move to modules, this will be harder to do, but I guess you're working on that or we can get to it later.
Pull-Request has been merged by qulogic
There are several benefits:
GOPATHandGOROOT, and returns error message more consistent with regular go tools.This is based on #16, or it wouldn't compile otherwise.
There are still some difference that I am investigating, so it's not perfect just yet.