Bug 1534123 - repoquery: --whatrequires is broken with rich deps
Summary: repoquery: --whatrequires is broken with rich deps
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: libdnf
Version: 30
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: ---
Assignee: Lukáš Hrázký
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: dnf-community
TreeView+ depends on / blocked
 
Reported: 2018-01-13 11:42 UTC by Igor Gnatenko
Modified: 2020-05-04 12:03 UTC (History)
12 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1698034 (view as bug list)
Environment:
Last Closed: 2020-05-04 12:03:41 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Igor Gnatenko 2018-01-13 11:42:01 UTC
⋊> ~/P/f/r/rust-openssl on master ⨯ for p in (sudo dnf -q repoquery --whatrequires rust-bitflags0.9-devel); echo $p; and sudo dnf -q repoquery --requires $p | grep bitflags; end                          12:38:47
rust-atk-sys-devel-0:0.5.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-clap-devel-0:2.29.1-1.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-devicemapper-devel-0:0.15.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-gdk-devel-0:0.7.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-gdk-pixbuf-sys-devel-0:0.5.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-gdk-sys-devel-0:0.5.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-gio-devel-0:0.3.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-gio-sys-devel-0:0.5.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-git2-devel-0:0.6.11-1.fc28.noarch
(crate(bitflags) >= 0.9.0 with crate(bitflags) < 0.10.0)
rust-glib-devel-0:0.4.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-glib-sys-devel-0:0.5.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-gobject-sys-devel-0:0.5.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-gtk-devel-0:0.3.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-gtk-source-sys-devel-0:0.5.0-1.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-gtk-sys-devel-0:0.5.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-nix-devel-0:0.9.0-4.fc28.noarch
(crate(bitflags) >= 0.9.0 with crate(bitflags) < 0.10.0)
rust-openssl-devel-0:0.9.23-2.fc28.noarch
(crate(bitflags) >= 0.9.0 with crate(bitflags) < 0.10.0)
rust-pango-devel-0:0.3.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-pango-sys-devel-0:0.5.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-pulldown-cmark-devel-0:0.1.0-2.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
rust-syntex_syntax-devel-0:0.59.1-3.fc28.noarch
(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0)
⋊> ~/P/f/r/rust-openssl on master ⨯ dnf --version                                                                                                                                                          12:39:42
2.7.5
  Installed: dnf-0:2.7.5-5.fc28.noarch at Fri 05 Jan 2018 04:36:53 PM GMT
  Built    : Fedora Project at Wed 03 Jan 2018 10:56:10 AM GMT

  Installed: rpm-0:4.14.0-5.fc28.x86_64 at Thu 16 Nov 2017 10:44:14 AM GMT
  Built    : Fedora Project at Tue 07 Nov 2017 11:56:33 AM GMT

---

This is blocking me to rewrite some part of infrastructure which relies on yum.

Comment 1 Igor Gnatenko 2018-01-13 11:43:26 UTC
For those who didn't understand issue -- dnf returns list of 21 packages which it thinks depend on rust-bitflags0.9-devel while truth is that only 3 packages depend on it.

Comment 2 Fedora End Of Life 2018-02-20 15:32:03 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 28 development cycle.
Changing version to '28'.

Comment 3 Jaroslav Mracek 2018-10-01 16:10:06 UTC
Please if the problem is based on that a query always return for condition in richdeps true this is not a bug, but by design. Thanks a lot for understanding.

Comment 4 Igor Raits 2018-10-01 21:01:44 UTC
How come it is by design?

Returning package which has nothing to do with requirement in command line is a bug.

I don't know how exactly it is implemented in repoquery, but livsolv returns proper result.

Also end builddep works fine, so this is definitely a bug in repoquery.

Comment 5 Jaroslav Mracek 2018-10-02 07:36:22 UTC
Probably you can provide a solution, because you are an author of rich deps implementation in libdnf (Query). Please can you help us?

Comment 6 Igor Raits 2018-10-02 09:43:31 UTC
(In reply to Jaroslav Mracek from comment #5)
> Probably you can provide a solution, because you are an author of rich deps
> implementation in libdnf (Query). Please can you help us?

I never designed libdnf's Query and I never liked it. For reference, this libsolv code works fine:

#!/usr/bin/python3
import solv

pool = solv.Pool()
pool.setarch('x86_64')

repo = pool.add_repo('available')

bitflags_compat = repo.add_solvable()
bitflags_compat.name = 'rust-bitflags0.9-devel'
bitflags_compat.evr = '0.9.8-1'
bitflags_compat.arch = 'noarch'
bitflags_compat.add_deparray(solv.SOLVABLE_PROVIDES, pool.Dep('crate(bitflags)').Rel(solv.REL_EQ, pool.Dep('0.9.8')))
bitflags = repo.add_solvable()
bitflags.name = 'rust-bitflags-devel'
bitflags.evr = '1.1.1-1'
bitflags.arch = 'noarch'
bitflags.add_deparray(solv.SOLVABLE_PROVIDES, pool.Dep('crate(bitflags)').Rel(solv.REL_EQ, pool.Dep('1.1.1')))
clap = repo.add_solvable()
clap.name = 'rust-clap-devel'
clap.evr = '1-1'
clap.arch = 'noarch'
clap.add_deparray(solv.SOLVABLE_REQUIRES, pool.Dep('crate(bitflags)').Rel(solv.REL_GT | solv.REL_EQ, pool.Dep('1.0.0')).Rel(solv.REL_WITH, pool.Dep('crate(bitflags)').Rel(solv.REL_LT, pool.Dep('2.0.0'))))
openssl = repo.add_solvable()
openssl.name = 'rust-openssl-devel'
openssl.evr = '1-1'
openssl.arch = 'noarch'
openssl.add_deparray(solv.SOLVABLE_REQUIRES, pool.Dep('crate(bitflags)').Rel(solv.REL_GT | solv.REL_EQ, pool.Dep('0.9.0')).Rel(solv.REL_WITH, pool.Dep('crate(bitflags)').Rel(solv.REL_LT, pool.Dep('1.0.0'))))

pool.createwhatprovides()

for pkg in (clap, openssl):
    for dep in pkg.lookup_deparray(solv.SOLVABLE_REQUIRES):
        print(f'{dep}: {pool.whatprovides(dep)}')


(crate(bitflags) >= 1.0.0 with crate(bitflags) < 2.0.0): [<Solvable #3 rust-bitflags-devel-1.1.1-1.noarch>]
(crate(bitflags) >= 0.9.0 with crate(bitflags) < 1.0.0): [<Solvable #2 rust-bitflags0.9-devel-0.9.8-1.noarch>]


DNF must be doing something wrong. I'm not sure what exactly.

Comment 7 Igor Raits 2018-10-02 12:07:09 UTC
> /usr/lib/python3.7/site-packages/dnf/cli/commands/repoquery.py(317)_get_recursive_deps_query()
    316         t = t.union(query_in.filter(requires=set_requires))

Bug happens here.

ipdb> [str(x) for x in set_requires if not isinstance(x, str)]
['crate(bitflags) = 0.9.1', 'crate(bitflags/default) = 0.9.1', 'crate(bitflags/example_generated) = 0.9.1', 'crate(bitflags/unstable_testing) = 0.9.1', 'rust-bitflags0.9-devel = 0.9.1-4.fc29']

But query_in.filter(requires=…) is broken here.

For instance,
>>> query_in.filter(requires=hawkey.Reldep(self.base.sack, 'crate(bitflags) = 2.0.0')).run()

Roughly looking at libdnf's code, seems that depSplitter works fine, but probably filtering doesn't.

I'm not profecient in C++ and I don't have time to waste, so please just fix this bug.

---

You can still reproduce this bug on rawhide. Just run `dnf repoquery --whatrequires rust-bitflags0.9-devel` which would return things like rust-clap-devel, but that one depends on rust-bitflags-devel (not the rust-bitflags0.9-devel).

Comment 8 Igor Raits 2018-10-02 12:11:32 UTC
void
Query::Impl::filterRcoReldep(const Filter & f, Map *m)
{
    assert(f.getMatchType() == _HY_RELDEP);

    Pool *pool = dnf_sack_get_pool(sack);
    Id rco_key = reldep_keyname2id(f.getKeyname());
    Queue rco;
    auto resultPset = result.get();

    queue_init(&rco);
    Id resultId = -1;
    while ((resultId = resultPset->next(resultId)) != -1) {
        Solvable *s = pool_id2solvable(pool, resultId );
        for (auto match : f.getMatches()) {
            Id reldepFilterId = match.reldep;

            queue_empty(&rco);
            solvable_lookup_idarray(s, rco_key, &rco);
            for (int j = 0; j < rco.count; ++j) {
                Id reldepIdFromSolvable = rco.elements[j];

                if (pool_match_dep(pool, reldepFilterId, reldepIdFromSolvable )) {
                    MAPSET(m, resultId );
                    goto nextId;
                }
            }
        }
        nextId:;
    }
    queue_free(&rco);
}


I think bug would be in this function, because `pool_match_dep()` returns true in pessimistic case (aka some part of reldep is matching). You should not us this function for this case.

Comment 9 Igor Raits 2018-10-02 12:14:49 UTC
Michael, correct me if I'm wrong.

Comment 10 Michael Schröder 2018-10-04 10:04:29 UTC
Well, you're somewhat wrong ;)

Actually libsolv has a 'solvable_matchesdep()' function that pretty much looks like the above code.

But let's talk a bit of what to expect from a 'whatrequires' query. I want to start with the argument being a capability and not a package name, i.e. what rpm and zypper do.
Basically the question is "If this capability is provided, which packages require it?". (As provides can't use rich deps you also can't use a rich dep as whatprovides query argument.)

Now say you have got kernel packages that provide different flavors, and you have a provide for every flavor. Some kernel module has: "Requires: (kernel with flavor = default)".
You then do a whatrequires match for "kernel". Clearly you want to see the kernel module in the output.

So this is what libsolv implements. It returns the package even if only the first part of the "with" statement is true. (And it can't check the second part, because there was no way to specify the flavor capability.)

This does not work well if the "with" op is used to specify an interval, like in this bug report. I could somewhat special case this, but I'm not really convinced that it makes sense because things will get pretty inconsistent.

But lets step back a bit: I'm somewhat confused about the repoquery in this bug: it looks to me like you specified a package and then dnf does a dependency match for all the provides of the package? In that case there would be a way out: libsolv would need to provide a dependency match where not just one capability, but a set of capabilities is provided as argument. In that case we could implement the with/without rich ops in a correct way.

Comment 11 Igor Raits 2018-10-04 10:20:21 UTC
> But lets step back a bit: I'm somewhat confused about the repoquery in this bug: it looks to me like you specified a package and then dnf does a dependency match for all the provides of the package? In that case there would be a way out: libsolv would need to provide a dependency match where not just one capability, but a set of capabilities is provided as argument. In that case we could implement the with/without rich ops in a correct way.

Yes. It goes through all provides of a package and tries to find what requires them.

For things like and/or, I would say that behavior is correct, but with/without case I think should be special.

Comment 12 Igor Raits 2018-10-31 12:46:56 UTC
So Michael implemented pool_whatmatchessolvable(). And now it is available in rawhide.

Can you fix implementation now?

Comment 13 Neal Gompa 2019-02-02 23:59:45 UTC
This is a seriously big problem as we keep using rich dependencies. Even dnfdragora uses rich dependencies now, and we have the entire Rust ecosystem relying on it. If we can't repoquery them reasonably well, this is going to cause major headaches. And we still need this working for spam-o-matic and other aspects of releng check scripts.

Daniel, can you please look at this and prioritize it?

Comment 14 Daniel Mach 2019-03-20 11:34:55 UTC
I believe the bug is still valid on F30, moving release accordingly.
I also replaced 'depends on' with 'blocks' dnf-community tracker.

Comment 15 Lukáš Hrázký 2019-04-10 13:26:45 UTC
According to the documentation (well, the short comment above the function definition), as well as looking at the code, the pool_whatmatchessolvable() function works only for installable packages, which AFAIU is a problem. We want repoquery to work on all packages, even those that are not installable in the current state, and that is what it does now. So presumably by using the new function we would solve OP problem, but introduce a regression by no longer listing non-installable packages.

Michael, am I right? Why doesn't the function work for non-installable packages?

Comment 16 Michael Schröder 2019-04-10 13:44:06 UTC
Well, it matches what the other pool_whatmatches... functions do. "installable" here means that the
package can be on the "whatprovides" hash, i.e. it has a architecture that is compatible with
the host's architecture. It's not about if all dependencies can be met or not.

There's actually two places where installability matters:

1) the comment above the function definition is about the input solvid, where currently only
   an installable package will work (all other will return an empty list)

2) the returned list will also contain only installable packages, which is similar to the
other whatmatches functions.

I'm not sure what repoquery did in regard of package architecture.

Comment 17 Michael Schröder 2019-07-15 12:05:53 UTC
(See https://github.com/openSUSE/libsolv/issues/334 about one part of not-installable packages)

Comment 19 Lukáš Hrázký 2019-11-08 14:11:24 UTC
PRs have been around for a while:
https://github.com/rpm-software-management/dnf/pull/1432
https://github.com/rpm-software-management/libdnf/pull/762
https://github.com/rpm-software-management/ci-dnf-stack/pull/577

The solution (in its current form) implements correct resolution of rich dependencies by resolving the repoquery NEVRA arguments to packages and then using the pool_whatmatchessolvable() libsolv function, which can resolve the rich dependencies of the packages.

Note it only really works for rich dependencies in the form of "(require >= 1.0.0 with require < 2.0.0)", perhaps with some small variations. But the libsolv code is special-casing this and doesn't provide a generic solution. This is all we can do on DNF side though, if any improvements happen on libsolv side, they should start working in DNF transparently.

Comment 20 Ben Cotton 2020-04-30 20:12:00 UTC
This message is a reminder that Fedora 30 is nearing its end of life.
Fedora will stop maintaining and issuing updates for Fedora 30 on 2020-05-26.
It is Fedora's policy to close all bug reports from releases that are no longer
maintained. At that time this bug will be closed as EOL if it remains open with a
Fedora 'version' of '30'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 30 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 21 Lukáš Hrázký 2020-05-04 12:03:41 UTC
The fix was released in Fedora already.


Note You need to log in before you can comment on or make changes to this bug.