Skip to content
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

Make use of the new pkglist metadata in more places #1162

Closed
wants to merge 3 commits into from

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Dec 19, 2017

This patch series teaches RpmOstreePackage about the new pkglist metadata format, and then makes the db diff API use it. There are a lot of advantages to this, the major one being one can perform a db diff with only the commit metadata present. See commit messages for more details.

Requires #1158 and #1160. (Only the last three commits are new here.)

@jlebon jlebon added the WIP label Dec 19, 2017
@jlebon
Copy link
Member Author

jlebon commented Dec 19, 2017

Marking as WIP for now; I'd like to add some tests for this.

@rh-atomic-bot
Copy link

☔ The latest upstream changes (presumably 97133bd) made this pull request unmergeable. Please resolve the merge conflicts.

In order to make use the new pkglist metadata more consummable, let's
add a function to create RpmOstreePackage arrays from it. Since some
APIs need to keep working as before, we use a `allow_noent` parameter to
distinguish between the two cases.

This is prep trying our best to make use of this metadata when possible
rather than checking out the rpmdb and initializing a `DnfSack`.
Add a function that can smartly perform diff operations on sorted
RpmOstreePackage arrays and make the db API use that. This allows us to
immediately take advantage of the benefits in a few places where diffs
are performed, including post-deployment tree diffs, and the legacy db
diff variant API. The upcoming `CachedUpdate` rework will also make use
of this (but with the notable difference of setting `allow_noent` to
`TRUE`).

Note this introduces a new `rpm_ostree_db_diff_ext` public API which has
the same interface as `rpm_ostree_db_diff` but also takes flags.
This is the first step towards unifying how we introspect packages from
a specific commit. We currently do this in three ways: libdnf, librpm,
and now `rpmostree.rpmdb.pkglist`. I'd like to get to a point where we
only have `rpmostree.rpmdb.pkglist` and libdnf, the latter only when
more complex queries are required.

This patch teaches the `db diff` command to make use of the new db diff
API so that it can work even on metadata-only commits. This is relevant
for use cases mentioned in coreos#558.

I didn't get rid of the `rpmhdrs_diff` functions right now because of
the `--changelogs` option: libdnf currently does not expose this, so we
fall back to the previous API in that case. OTOH, I wonder how much it's
actually used in the wild; maybe we could just nix it?
@jlebon jlebon force-pushed the pr/use-libdnf-for-db-cmd branch from a049ced to 9afa074 Compare December 20, 2017 22:46
@jlebon jlebon removed the WIP label Dec 20, 2017
@jlebon
Copy link
Member Author

jlebon commented Dec 20, 2017

Rebased and with tests! ⬆️

@cgwalters
Copy link
Member

Very nice!

@rh-atomic-bot r+ 9afa074

@rh-atomic-bot
Copy link

⌛ Testing commit 9afa074 with merge 3727370...

rh-atomic-bot pushed a commit that referenced this pull request Dec 30, 2017
Add a function that can smartly perform diff operations on sorted
RpmOstreePackage arrays and make the db API use that. This allows us to
immediately take advantage of the benefits in a few places where diffs
are performed, including post-deployment tree diffs, and the legacy db
diff variant API. The upcoming `CachedUpdate` rework will also make use
of this (but with the notable difference of setting `allow_noent` to
`TRUE`).

Note this introduces a new `rpm_ostree_db_diff_ext` public API which has
the same interface as `rpm_ostree_db_diff` but also takes flags.

Closes: #1162
Approved by: cgwalters
rh-atomic-bot pushed a commit that referenced this pull request Dec 30, 2017
This is the first step towards unifying how we introspect packages from
a specific commit. We currently do this in three ways: libdnf, librpm,
and now `rpmostree.rpmdb.pkglist`. I'd like to get to a point where we
only have `rpmostree.rpmdb.pkglist` and libdnf, the latter only when
more complex queries are required.

This patch teaches the `db diff` command to make use of the new db diff
API so that it can work even on metadata-only commits. This is relevant
for use cases mentioned in #558.

I didn't get rid of the `rpmhdrs_diff` functions right now because of
the `--changelogs` option: libdnf currently does not expose this, so we
fall back to the previous API in that case. OTOH, I wonder how much it's
actually used in the wild; maybe we could just nix it?

Closes: #1162
Approved by: cgwalters
@rh-atomic-bot
Copy link

☀️ Test successful - status-papr
Approved by: cgwalters
Pushing 3727370 to master...

@dustymabe
Copy link
Member

one question on this. does db diff still have the ability and/or the option to use the pure rpm database instead of the commit metadata? If so is there some sort of preference?

@cgwalters
Copy link
Member

Not currently...why would you want that?

@dustymabe
Copy link
Member

I guess consistency checking (i.e. does the rpmdb match the commit metadata) and/or compatibility with trees that started without rpm data in the commit metadata (i.e. rpm-ostree db diff <first f27 commit> <last f27 commit>)

@cgwalters
Copy link
Member

One can do consistency checking by doing manual checkouts, though we'd have to screw up pretty spectacularly for things to be different.

As far as compat, this PR looks for the commit metadata first, and falls back automatically do reading the rpmdb if it isn't present.

@dustymabe
Copy link
Member

ok

@jlebon jlebon deleted the pr/use-libdnf-for-db-cmd branch February 15, 2018 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants