-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rpm: consult dnf database for repository information #869
base: main
Are you sure you want to change the base?
Conversation
5acce04
to
5a9d76e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #869 +/- ##
==========================================
- Coverage 55.39% 55.23% -0.17%
==========================================
Files 282 283 +1
Lines 17836 17923 +87
==========================================
+ Hits 9881 9899 +18
- Misses 6923 6990 +67
- Partials 1032 1034 +2 ☔ View full report in Codecov by Sentry. |
rpm/dnf.go
Outdated
if fi, err := fs.Stat(sys, `root/buildinfo/content_manifests`); errors.Is(err, nil) && fi.IsDir() { | ||
// This is a RHEL layer, skip. | ||
return nil, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Update this to check the files for a marker value and continue if found.
Should look for from_dnf_hint
key in the yaml files.
See also: containerbuildsystem/atomic-reactor#2140
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Punting on this unless we determine it's needed.
I created rpm-software-management/rpm#3505 related to this |
10b07ff
to
e59f165
Compare
8c86d88
to
7a459ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made it through most of the files, but I'll need to come back to this another time. Would you mind making more commits and then squashing things at the end so it's easier to see the diffs after each round? Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These txter test files are not net-new, I think they should be removed from rpm/testdata
environment.RepositoryIDs[i] = layerArtifacts.Repos[i].ID | ||
v, _ := url.ParseQuery(pkg.RepositoryHint) | ||
if id := v.Get("repoid"); id != "" { | ||
environment.RepositoryIDs = v["repoid"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will leave inconsistencies in how the repo IDs are reported in index report. Packages with no specific repos will report their repo ids as numerical with the ability to be referenced back to repository
objects (like current behaviour):
"repository_ids": [
"1",
"2",
...
But packages that have a specific repoid specified by the DNF DB will have the repo slug that can not be directly referenced back to a repository
object:
"repository_ids": [
"rhel-9-for-x86_64-appstream-rpms"
]
We should think of a way to make these consistent to keep the expectation that "repository_ids"
will refer to keys in the "repository"
section and not a mix of the claircore.Repository.ID
and (what will be persisted as) the claircore.Repository.Name
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could do something like this:
func lookupRepositoryID(repos []*claircore.Repository, slug string) string {
for _, r := range repos {
if r.Name == slug {
return r.ID
}
}
return ""
}
and then
if slug := v.Get("repoid"); slug != "" {
if id := lookupRepositoryID(layerArtifacts.Repos, slug); id != "" {
environment.RepositoryIDs = []string{id}
}
This should always find a repo as repository scanner should be finding the same repos as the package scanner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This suggestion is incomplete in that we need to append
all repositories who's Name
== repoid, not just the first one as the example code does
d8a2dad
to
e6a4e27
Compare
Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]>
Using these should give us a nice "todo" list when the time comes. Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]>
This adds two helpers: one to discover repoids, and one to annotate a stream of Packages. Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]> See-also: https://issues.redhat.com/browse/KONFLUX-6218
Signed-off-by: Hank Donnay <[email protected]> See-also: https://issues.redhat.com/browse/KONFLUX-6218
Signed-off-by: Hank Donnay <[email protected]> See-also: https://issues.redhat.com/browse/KONFLUX-6218
Signed-off-by: Hank Donnay <[email protected]> See-also: https://issues.redhat.com/browse/KONFLUX-6218
Signed-off-by: Hank Donnay <[email protected]> See-also: https://issues.redhat.com/browse/KONFLUX-6218
These changes make the
rhel
machinery consult both therpm
anddnf
databases when possible.See-also: #809