Rpmdb checkout cleanup #149
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For a future patch, I want to add an API to get an rpmts for a commit, instead of a hawkey Sack, because libsolv doesn't expose some optimized queries that we can get by just going directly to librpm, such as package file owners.
It was only used to access the yumdb, which we don't use because: - It badly exacerbates the OSTree one-HTTP-request-per-object issue - We're assembling multiple repos on the server side, so things like who took the action aren't relevant. But the reason I did this patch at the moment is because I want to unify the code that's creating tempdirs from commits so we can feed real files to librpm.
As far as I can tell, this is basically a way to specify the temporary directory. That significantly complicates the code as it now needs to keep track of whether or not it owns the temporary directory. This hinders unifying this code with the hawkey query path. Because of this, and since I'm not aware of a use case for specifying this tempdir, let's remove it.
This allows us to more cleanly encapsulate things.
hawkey and libsolv are both patched to look in `usr/share/rpm` if the db can't be found in `/var/lib/rpm`. However, librpm itself isn't. One *can* override it with a macro...which is process global. Yuck. Needs fixing. Anyways, we can just make a symlink. That's a lot easier than writing a patch for librpm and waiting a billion years to be able to use it everywhere we care about. This will help unify the librpm tempdir code with the hawkey tempdir code.
And now, finally the actual goal is achieved. \o/ Only one code path dealing with extracting the rpm database from an OSTree commit. An astute reader would notice that the `root` member of the struct was actually only necessary as of a few commits ago. But said astute reader would also realize it's kind of late in the evening and not worth rebasing it to where it would properly go.
While `rpm-util.c` may not best describe this, it's where most of this code is ending up. Let's further centralize things. We more consistently return an `RpmOstreeRefSack` instead of a `HySack`, where the former supports refcounting and knows how to clean up its temporary directory if it was allocated from a commit.
The refsack code was using the latter, and it stood out. Now that we're making use of explicit export markers, there's no need to uglify internal APIs with a leading `_`.
This API is intended for use by the caching framework to hold open an rpmdb for the previous commit.
|
Above comments aside, looks like some nice cleanup here. 👍 |
More code de-duplication.
- Can also give you a file descriptor - Takes a constant string as input, returning a mutated string as a separate variable which means that one can check whether the variable is `NULL` to know whether or not one needs to `rm -rf` it on error paths.
|
Ok, pushed some fixups ⬆️ |
|
Looks good. 👍 |
|
Fixed up with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This series is prep for #146