search

amake / go2port

A tool for generating MacPorts portfiles for Go projects
BSD 3-Clause "New" or "Revised" License
6 stars 7 forks source link

The go.sum shouldn't be used to retrieved the dependencies of a module #5

Open dgsb opened 3 years ago

dgsb commented 3 years ago

The go.sum contains only cryptographic hashes for some version of packages. It doesn't gives the authority on which package version must be used for the build, if the go.sum file has not been cleaned up it may contains several versions which have been used over time.

With this line https://github.com/amake/go2port/blob/0298d8d3f83d296ab586a0ef928c259723c1dac0/go2port.go#L550 we choose the last version found in the go.sum file for a given package which may not be the one actually used as the dependency computed by through the go.mod files specifications.

We may expect that the highest version is actually the one used but we do not have strong guarantee for that either

amake commented 3 years ago

Ok, so what should we do about it?

dgsb commented 3 years ago

I wonder if you could just fetch the go.mod too and compute the dependency tree using this file only with the help of the go command. I don't have time right now to test that though.

jamesog commented 3 years ago

The go tool can tell you exactly what it intends to use after MVS is applied.

go list -m all

prints all required modules and versions.

go list -m -json all

will do the same but in JSON format for tools to consume. It looks like the method go list uses to do this is internal only so other tools can't import the same package to replicate it, so it means execing out.

I suppose the tricky thing for this tool is currently it's simply pulling down go.sum on its own and parsing it. In theory the go list method could work by also pulling down go.mod but I'm not sure if it would get upset by not actually having the whole tree.

If you like I can have a go at making that modification.

amake commented 3 years ago

If you like I can have a go at making that modification.

If you think it's a more robust approach, and it doesn't mean rewriting the entire tool, then sure, that would be great.

On the other hand if it's a significant departure from the current implementation then maybe it should be a new tool entirely and this one should be deprecated.

(This project is the full extent of my knowledge and practical use of golang, so anything more complex will be beyond my ability/appetite to maintain.)

jamesog commented 3 years ago

I've got a work in progress that seems to do the right thing. The downside is that, depending on the project, it can inflate the vendored dependencies quite a lot compared to the existing approach. This is because the special all module name to go list -m will list all dependent modules, their dependent modules, etc., some of which are not required for build. Those extra dependencies can't easily be excluded.

Going with this approach might be good, though, as it would mean we could think about enabling module mode by default in the build infrastructure; GO111MODULE won't be supported forever. If you don't have all of the dependent modules then module mode gets unhappy.

In looking at this (I'm still pretty new to contributing ports) I'm wondering if there are improvements that can be made to how MacPorts handles this, as it's very hard to port anything that uses a custom domain name but that's not for this issue/repo I guess. :-)