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

libdnf assumes rpmdb is in Berkeley DB format #362

Closed
pmatilai opened this issue Nov 1, 2017 · 9 comments
Closed

libdnf assumes rpmdb is in Berkeley DB format #362

pmatilai opened this issue Nov 1, 2017 · 9 comments

Comments

@pmatilai
Copy link
Member

pmatilai commented Nov 1, 2017

Discovered while looking into libsolv's BDB assumptions about the rpmdb, libdnf does it too by assuming there's always a Packages file in the rpmdb, this is not a valid assumption:

current_rpmdb_checksum(Pool *pool, unsigned char csout[CHKSUM_BYTES])
{
    const char *rpmdb_prefix_paths[] = { "/var/lib/rpm/Packages",
                                         "/usr/share/rpm/Packages" };

The hardwired database path is not appropriate either, it should get this from rpm configuration.

I'll look more into this later + hopefully provide patches / or at least suggestions how to deal with it, but filing now for awareness.

@Conan-Kudo
Copy link
Member

Wouldn't be one way to deal with this would be to get the %_dbpath and %_db_backend values and use those together to identify these paths?

Alternatively, is this even necessary with libsolv having the ability to load the rpmdb via librpm with openSUSE/libsolv#235?

@pmatilai
Copy link
Member Author

pmatilai commented Nov 2, 2017

Ultimately the filenames in that directory are nobody elses business that librpm, but yes the combination of %_dbpath and %_db_backend would be a step in the right direction. I don't know about rpm-ostree, but since it puts rpmdb into a different path it should set %_dbpath to that location too.

Of course, libdnf is not poking at rpmdb directory just for fun, it does it to see if the rpmdb was changed since the last time. That information is something that librpm should be providing via an API. And now that this came up, I remember going through this same discussion at least twice before in the last 8-9 years but nothing ever got implemented in rpm, so my bad I guess :-/

AFAICS libdnf does this to see if there is a valid rpmdb cache it can use, which seems fairly reasonable as such, but then libsolv also has similar logic in it. So there's probably something else to streamline etc besides rpm needing to add that rpmdb cookie API.

@Conan-Kudo
Copy link
Member

There's also another place where this assumption is made:

    /* setup a file monitor on the rpmdb, if we're operating on the native / */
    if (g_strcmp0(priv->install_root, "/") == 0) {
        rpmdb_path = g_build_filename(priv->install_root, "var/lib/rpm/Packages", NULL);
        file_rpmdb = g_file_new_for_path(rpmdb_path);

@j-mracek
Copy link
Member

j-mracek commented May 3, 2019

@pmatilai Is the issue still valid?

@pmatilai
Copy link
Member Author

pmatilai commented May 6, 2019

Um, sure it is, such an issue doesn't go away by itself.

@pmatilai
Copy link
Member Author

@dmach , you'll want to prioritize this higher as it's a blocker for any alternative db backend.

There's also a similar issue left in libsolv too, I'll open another tracker on that.

j-mracek pushed a commit that referenced this issue Oct 22, 2019
Failure to calculate rpmdb checksum is not a fatal error, it just means
that we cannot use the cached data. This is sufficient to make libdnf
usable with non-BDB backends (with a related fix on libsolv), but to
make the caching work in those setups more work is needed.
@pmatilai
Copy link
Member Author

pmatilai commented Mar 18, 2020

Re-ping:

While PR #824 fixed things to point of allowing libdnf to function with non-BDB backends, the caching still only works with BDB and this makes any alternative backend look worse performance-wise.

@pmatilai
Copy link
Member Author

So I'll have to admit I was talking based on assumptions that the cache actually works (as in, improves performance) and is actually used. Based on @j-mracek comments in #915, it's neither.

Maybe we should take a step back here and take a moment to understand what it is that libdnf actually needs, and why. I'd prefer the discussion in the context of this ticket than any particular PR because at least I clearly have no idea what's going on here.

@pmatilai
Copy link
Member Author

Fixed by #916 just now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants