Add support for generating lockfiles to rpmtree #80
Conversation
|
@rmohr are you able to take a look at this? |
This change introduces generation of the lockfile from the rpmtree command. Following the previous patterns with the other bazel files, this adds some helper methods to bazel.go and creates a handler object to wrap this up for rpmtree. Worth noting is that to make this work we also have to tweak AddTree() to take the configuration name because the generated repos are different when using the lockfile. This change leaves the MODULE.bazel configuration up to the user as there may be some additional configuration for bazeldnf that is beyond the scope of what rpmtree is doing and we don't want to interfere with that.
(1) The error checking at the beginning isn't necessary because we also
want to support WORKSPACE mode.
(2) If we pass the default configname through to AddTree() then we'll
generate the WORKSPACE file as if we had a lock file, which is not
what we desire
With a minor re-ordering of the independant operations in rpmtree we can reduce the surface area of the handler interface. This enables us to simply the interface and remove some redundant bits.
There was a problem hiding this comment.
I think it would nice to see an example of the lock file in action.
Compared to my format of the lock file I have the feeling your format means either someone needs to know which RPMs to add for an rpmtree, or you need to add them all. This is fine for small repos that don't have multiple apps built from it, but image a repo (and this is what I have @ Booking.com) composed of multiple container images, not all of them require all the available RPMs.
Maybe we should discuss first what the lock file should look like and the complexity we would like to support? In my case the idea is to make it clear everything that's being used, why a certain dependency is needed, easy generation and update of the lock file, simpler starlark code to make use of it through proxy repositories
| URLs []string `json:"urls"` | ||
| } | ||
|
|
||
| type Config struct { |
There was a problem hiding this comment.
Should we have a lock file version field? Or we add that one when we start adding extra fields?
| if err != nil { | ||
| return err | ||
| } | ||
| return os.WriteFile(path, configJson, 0666) |
There was a problem hiding this comment.
Shouldn't this be 0644?
| type RPM struct { | ||
| Name string `json:"name"` | ||
| SHA256 string `json:"sha256"` | ||
| URLs []string `json:"urls"` |
There was a problem hiding this comment.
Should we get version and arch here as well? For an SBOM that would be helpful
|
Just for me for the context, what do you mean with "This change introduces the necessary bits for producing lockfiles using rpmtree in lieu of having to hand craft them"? Do you mean to have an easy way to pin some of the RPMs without having to specify the pinned ones on the commandline when resolving dependencies? |
This change introduces the necessary bits for producing lockfiles using rpmtree in lieu of having to hand craft them. It also tweaks the rpmtree() invocation in the BUILD file so that it correctly points at the lockfile derived repos.