[Buildroot] [RFC PATCH v1 1/6] package/go: implement go modules integration

Arnout Vandecappelle arnout at mind.be
Fri Apr 5 21:59:18 UTC 2019



On 05/04/2019 19:49, Christian Stewart wrote:
> Hi Arnout,
> 
> Arnout Vandecappelle <arnout at mind.be> writes:
[snip]
>>  I don't understand why it can't be removed. The APPLY_EXTRACT_GOMOD doesn't do
>> anything if a vendor tree is already present, and in all our current go packages
>> the vendor tree is already present...
> 
> The hook can probably be removed but we need to make the almost-empty
> go.mod somewhere. 
> 
>>  Oh, hang on, you probably still have to create an almost-empty go.mod for the
>> ones which don't have a vendor.conf.

 My bad, I forgot that that was also done in the post-extract hook.


> I'm almost sorry I mentioned vendor.conf or gopkg.toml in the first
> place, becuase it seems like you're now distracted by the other formats.

 :-)


> It's not really a good idea to use the auto conversion at runtime since
> the result is not as deterministic as having a predetetermined go.mod,
> and the conversion requires network lookups.

 AFAIU, packages which use vendor.conf or gopkg.toml will have their vendor
trees bundled, no? Otherwise other users wouldn't have a reliable way of
downloading the package anyway, right?


> Pretend the other formats don't exist. They are there for legacy only anyway.

 Okay. Anyway, the go.mod will be autogenerated since it will be missing, and
supposedly the vendor tree is there.


>>>>  If you can give an example of a package that only has a few of these, I could
>>>> create an example of how it might work without any extra infra. Although I'm not
>>>> entirely sure if such an example would be very useful, except to show that it
>>>> would be extremely hard to maintain.
>>>
>>> This doesn't make any sense to me.
>>
>>  What doesn't make sense to you? Do you mean that you don't understand how it
>> could be done with additional downloads, or do you mean that you don't
>> understand how it could not be useful, or do you mean that you don't understand
>> how it could be hard to maintain, or do you mean that you fail to parse the
>> entire sentence?
> 
> I suppose you're saying you want to add more download statements in the
> package and then download all of the dependencies into the vendor/ tree
> using the Buildroot mechanism.

 Not really download statements; _EXTRA_DOWNLOADS, and a post-extract hook to
extract them. In terms of complexity, it's pretty similar as carrying the go.mod
in Buildroot. But of course, this was would not be able to use an existing
upstream go.mod.

> This is not viable 

 Why is it not viable?

> and I don't see why we
> would even consider it to begin with.

 Because we have the option of either using the existing mechanisms, or to
extend the infra to cover go modules. To evaluate whether the extra infra is
useful, it is interesting to look at what it would look at what would be needed
without it.


>>  Can you point out which bit causes the modules to be downloaded?
> 
> GOPROXY=direct go mod vendor, or you might try executing the build with
> the series applied, compiling any Go package, and you'll see the
> download happen during that step.

 I'll admit that I hadn't done that yet. So I applied only the first patch and
tried to build docker-cli. It doesn't do any downloading of dependencies because
the vendor/ tree is already there (the patch has an explicit condition for that).

 BTW, the build step then does automatic conversion of vendor.conf, but the
build fails with:

can't load package: package cmd/docker: cannot find package "." in:
        /home/arnout/src/buildroot/output/host/lib/go/src/cmd/docker

 I'm probably doing something wrong :-)

[snip]
>>> Why would this be the only case where adding a
>>> patch would not work? With a new download method, it would go:
>>>
>>>  1. Download base package (containing go.mod)
>>>  2. (optional) Copy in go.mod and go.sum from Buildroot tree
>>>  3. Execute "go mod vendor." Note, go.sum ensures the result is consistent.
>>>  4. Compress the result and store in dl/ tree.

 I just noticed something missing: you'll need to add host-go to the
DOWNLOAD_DEPENDENCIES of go packages if you want to use the go tool in the
download step.

>>>  5. Later, in the extract step, extract this tarball
>>>  6. Apply any patches (this can also target the vendor/ tree if needed)
>>>  7. Build the package with -mod=vendor arg which uses the vendor/ tree.
>>
>>  Exactly, so it is still possible to use the patch approach instead of changing
>> the version.
> 
> ... which was the point of me listing the order of things happening.
> However, you can also see why it might be useful to have go.mod and
> go.sum in the tree, considering that you are not patching BEFORE running
> "go mod download" or "go mod vendor." You need an opportunity to
> override those files earlier on, especially if a dependency no longer
> exists, and you're looking to update the code URL for it.

 Agreed.

 I'm not communicating my message very well apparently. I am not saying that a
Buildroot-supplied go.mod is not useful. I'm only saying that it is not
absolutely needed, and that the current infra probably already covers a lot if
not all of the use cases we have in reality. While explaining why I think that,
I apparently didn't make it clear that I'm only trying to compare the different
options we have, and weigh that against the priorities of the project.

[snip]
>>> The goal of copying go.mod in from the Buildroot tree is to allow you to
>>> override / provide go.mod if there is not one or if the existing one is
>>> outdated with security issues,
>>
>>  There are two concerns here, and I think we should consider them separately.
>>
>> 1. Make sure that building with -mod works.
>>   a. Upstream has a go.mod -> nothing needs to be done.
> 
> This is the currently implemented logic in the PoC patches.
> 
>>   b. Upstream has no go.mod, but has vendor.conf or .toml -> nothing needs to be
>> done, the Go tool handles these as well.

 I think my analysis was wrong. It should have been:

a. Upstream has a go.mod.
 -> Dependencies have to be downloaded. Handled by the current patch, with the 3
limitations mentioned.

b. Upstream has a bundled vendor/ tree (as part of the tarball or as git
submodules).
 -> go.mod has to be created with just the module name. Handled by the current
patch, no limitations.

c. Upstream doesn't have a bundled vendor/ tree, only vendor.conf or gopkg.toml.
 -> not reproducible because the vendor.conf and gopkg.toml don't encode the
version to download. (Note: I looked at the vendor.conf of docker-cli and it
does have a version, so I'm not sure if this is true).

 For the c case, I indeed see no other way than a Buildroot-provided go.mod that
I can think of.


> I don't recommend this, since the conversion is not deterministic and

 Could you maybe explain why it is not deterministic?

> does network calls, as I mentioned above.

 The network calls are the same as in case a I think, no?


> Copying in the go.mod and go.sum might have spooked you because of the
> line count and the hashes in the file. 

 Yes indeed :-)

 Note that I don't think we need go.sum. We do with the current patch, but if
the go download becomes a download method, we end up with a tarball that
contains all dependencies, and the buildroot hashing is done on that tarball. So
checksumming is done (though it's a little late, so finding out what went wrong
in case of hash mismatch might be tricky).

[snip]
>>  Let me rephrase the question... As far as I understand, after the current patch
>> series, the Go tool will still not download anything for the existing packages,
>> because the vendor tree is already there. Is that correct? If not, could you
>> point out where the download happens?
> 
> "go mod vendor"
> 
> Although, if you look at the code, if the vendor tree is present, this
> is not called.

 That was my point: all current packages have a vendor tree, so it never gets
called.


> When compiling, go build -mod=vendor will tell the tool to use vendor/
> and use the go.mod file only for the module path line (roughly
> equiv to just the 1-line go.mod file).
> 
>>  Ah, of course, because the go.mod is autogenerated, it will still work. But
>> then why are those changes in docker-containerd.mk needed? I still don't get it :-(
> 
> This series could be done without those, using vendor in the tree. It's
> an example.
> 
>> I put this separate because AFAIU setting GOPROXY to off is not
>> strictly needed. In the current situation, the Go tool is never going
>> to download anything anyway, because the go.mod is empty.
> 
> It's strictly needed. The Go tool will analyze the code and determine
> the import paths of all the code files, and then generate / update
> go.mod. You might want to run the patch series and observe the behavior yourself.

 The behaviour I observe (with only patch 1 and removing GOPROXY=off) is that
there is a single-line go.mod (generated by Buildroot) and no go.sum, and no
downloads are done. Which makes sense, because the vendor tree is already there.


 Note that I'm not saying that GOPROXY should not be set to off. What I'm saying
is that that is pretty much independent of the rest of the go module support.

[snip]
>>  For this one, it would be nice if there could be an alternative that doesn't
>> require copying the entire go.mod and go.sum, but rather override only one or
>> two. Because I still think that that is going to be the only use case of this
>> feature.
> 
> Maybe, but you're making a lot of absolute statements here without
> actually using the Go modules workflow yourself to know why those
> statements are or aren't true.

 I don't intend to make absolute statements, but I can hardly write AFAIU on
every line :-)

 Regards,
 Arnout


> Please have a look at my reasoning above
> and see what you think.
> 
> Best regards,
> Christian 
> 


More information about the buildroot mailing list