Enable resolving multiple architectures of the same package#175
Conversation
|
bd5b2fa to
d21d63e
Compare
059d437 to
fdd563d
Compare
ab528aa to
798d43b
Compare
|
| if len(rpms) > 1: | ||
| fail("Package resolved multiple times, not implemented.") | ||
| for rpm in rpms: | ||
| if "arch" not in rpm or "package" not in rpm: |
There was a problem hiding this comment.
why shouldn't this be a failure case instead of a skip case?
There was a problem hiding this comment.
This is indeed an issue, and I have a valid use case when the len(rpms) will be > 1. With this commit I'm fixing it cf97632 in the circular dependencies branch.
The failure becomes relevant with the e2e/bazel-bzlmod-lock-file which is a totally valid case of 2 lock files mentioning the same file: libvirt-libs-11.0.0-1.fc42.x86_64.rpm. If we would like to do any kind of deduplication detection we should detect 2 entries in the lock files with same name and different checksum, which wouldn't be "legal" in a rpm repository. But the use case of multiple lock files mentioning the same file is totally valid. All of those should resolve to the same entry though.
Don't forget that given how bzlmod works it's not multiple times that our module gets initialized but 1 time with all the calls together.
There was a problem hiding this comment.
@kellyma2, bothrpm["arch"] and rpm["package"] are marked as optional, therefore it would be improper to interrupt in this case. An example of simpler lockfile (with only urls and sometimes name – that get translated here to just rpm["id"]) was provided by @manuelnaranjo.
@manuelnaranjo, I don't understand the concern. Lockfiles are processed separately and the only interference is when there is an rpm_repository to be created with the same repo name – in which case arbitrary one is used for this conflict (ignoring lockfile content).
Otherwise, lock files are handled separately, each one has its own packages_metadata collected and passed to the _alias_repository, that is the source of rpms argument to the alias macro.
Nevertheless, inspired by your comments, I'm sending for review an updated version that should handle a bit more gracefully some corner cases that may appear in manually crafted lockfiles.
9acad12 to
41203f8
Compare
|
I've been thinking and researching about this topic, and turns out we can extract the architecture from the file name. I think we need to change entirely how we deal with this, the blob it self should be a single entity based on the file name no matter how many lock files or strict rpms we get passed, the name of the rpm_repository instance should be as much as possible the name of the file, we will have to do some mangling because some characters are not allowed, but not a major thing. This approach will not only allow for multiple archs of the same package but also for multiple versions across all of the bazeldnf instances of a given bazel module. I would say per lock file we should have only 1 version for a given package, I would even challenge if multiple archs make sense. What do you folks think? I have the code for this I just didn´t finish integrating it. Then we should have entities that collect the dependencies and binds the dependency tree together, and finally aliases for those things we've been requested to make public. This is basically what my circular dependencies fix branch does btw |
After setting up this function for identifying unique files (rmohr#172, rmohr#173) we can finally incorporate package arch here. This change affects the repository naming, but users should consider it implementation detail and use alias repository to refer to RPMs, so this change should be transparent to proper users.
After extensive preparations (see e.g. changes referencing multi-arch proposal rmohr#166), we can simply remove the conflict.
|
I removed some commits from this PR so that we can discuss them separately:
The part that remained is the dependency resolution that will allow creating multi-arch lock file – which is a valuable artifact by itself. |
A series of small changes to finally implement / enable #166.
This stems quite naturally from all the preparatory work.
Let Id contain package arch
After setting up this function for identifying unique files (#172, #173) we can finally incorporate package arch here.
This change affects the repository naming, but users should consider it implementation detail and use alias repository to refer to RPMs, so this change should be transparent to proper users.
Enable resolving multiple architectures of the same package
After extensive preparations (see e.g. changes referencing multi-arch proposal #166),
we can simply remove the conflict.