qulogic opened a new pull-request against the project: `golist` that you are following: `` Replace Go internals with go/build ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/17
qulogic commented on the pull-request: `Replace Go internals with go/build` that you are following: `` So for tests, I searched the specs for all that define`goipath` 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.
Current `golist` crashes on:
* github.com/gengo/grpc-gateway * github.com/shurcooL/githubql
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):
* github.com/cespare/xxhash * github.com/google/go-github
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? ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/17
qulogic commented on the pull-request: `Replace Go internals with go/build` that you are following: `` 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. ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/17
nim commented on the pull-request: `Replace Go internals with go/build` that you are following: `` 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-macro code should be in a good enough shape to allow those changes but that ’s not what it 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
- 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%60 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%60 as `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 banch not the v2 one)
- 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
``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/17
nim commented on the pull-request: `Replace Go internals with go/build` that you are following: `` 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. ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/17
qulogic commented on the pull-request: `Replace Go internals with go/build` that you are following: `` 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. ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/17
qulogic merged a pull-request against the project: `golist` that you are following.
Merged pull-request:
`` Replace Go internals with go/build ``
golang@lists.fedoraproject.org