depgraph: Do not allow slotted deps to be satisfied by wrong slots

[zhuyifei1999]

This may be part of what caused the "perl rebuild bug". The "priority" of perl, when seen by perl modules, may have "satisfied" set to an installed perl of a wrong slot. Compounding this factor with bug #756199 where find_smallest_cycle would select a single node, in this case because the dependency of perl is satisfied and the priority then gets ignored, the "cycle" becomes a perl module alone and gets rebuilt early.

I also updated the test from the previous patch to account for this change. No other tests seems affected.

For a larger scale test, I reproduced this initially with a stage3 chroot from a Jan 1 2022 stage3 snapshot, and testing in an equivalent dockerfile would work too:

FROM gentoo/stage3:amd64-openrc-20220101
RUN emerge-webrsync
COPY . /portage

Before this patch (USE flags omitted):

# cd /portage && bin/emerge -puDN @world 2>&1 | grep -i perl
[ebuild     U  ] app-admin/perl-cleaner-2.30-r1 [2.30]
[ebuild  rR    ] virtual/perl-File-Temp-0.231.100
[ebuild  rR    ] dev-perl/Locale-gettext-1.70.0-r1
[ebuild  rR    ] dev-perl/MIME-Charset-1.12.2-r1
[ebuild  rR    ] dev-perl/Module-Build-0.423.100
[ebuild  rR    ] dev-perl/Text-CharWidth-0.40.0-r2
[ebuild     U  ] dev-lang/perl-5.36.0-r2 [5.34.0-r3]
[ebuild  N     ] virtual/perl-CPAN-2.330.0
[ebuild     U  ] virtual/perl-ExtUtils-MakeMaker-7.640.0 [7.620.0]
[ebuild     U  ] virtual/perl-File-Spec-3.840.0 [3.800.0]
[...]

After this patch:

# cd /portage && bin/emerge -puDN @world 2>&1 | grep -i perl
[ebuild     U  ] app-admin/perl-cleaner-2.30-r1 [2.30]
[ebuild     U  ] dev-lang/perl-5.36.0-r2:0/5.36 [5.34.0-r3:0/5.34]
[ebuild  N     ] virtual/perl-CPAN-2.330.0
[ebuild     U  ] virtual/perl-ExtUtils-MakeMaker-7.640.0 [7.620.0]
[ebuild     U  ] virtual/perl-Data-Dumper-2.184.0 [2.179.0]
[ebuild     U  ] virtual/perl-File-Spec-3.840.0 [3.800.0]
[ebuild     U  ] virtual/perl-Test-Harness-3.440.0-r1 [3.430.0]
[ebuild  rR    ] dev-perl/Pod-Parser-1.630.0-r1
[ebuild  rR    ] dev-perl/Text-CharWidth-0.40.0-r2
[ebuild  rR    ] dev-perl/Text-WrapI18N-0.60.0-r2
[...]

Bug: https://bugs.gentoo.org/463976
Bug: https://bugs.gentoo.org/592880
Bug: https://bugs.gentoo.org/596664
Bug: https://bugs.gentoo.org/631490
Bug: https://bugs.gentoo.org/764365
Bug: https://bugs.gentoo.org/793992
Signed-off-by: YiFei Zhu zhuyifei1999@gmail.com

[zhuyifei1999]

I don't know if anything needs to be done around this part of the code:

            if not dep_priority.ignored or self._dynamic_config._traverse_ignored_deps:
                inst_pkgs = [
                    inst_pkg
                    for inst_pkg in reversed(vardb.match_pkgs(virt_dep.atom))
                    if not reinstall_atoms.findAtomForPackage(
                        inst_pkg, modified_use=self._pkg_use_enabled(inst_pkg)
                    )
                ]
                if inst_pkgs:
                    for inst_pkg in inst_pkgs:
                        if self._pkg_visibility_check(inst_pkg):
                            # highest visible
                            virt_dep.priority.satisfied = inst_pkg
                            break
                    if not virt_dep.priority.satisfied:
                        # none visible, so use highest
                        virt_dep.priority.satisfied = inst_pkgs[0]

                if not self._add_pkg(virt_pkg, virt_dep):
                    return 0

It's too complicated to get a full picture right now.

[zhuyifei1999]

An actual test case is added :)

[akhuettel]

Not portage dev but perl maintainer here- this looks very good!

As far as I'm concerned it would be great to see this in ~arch portage before Perl 5.38 is unmasked, then we have a real-world test! :)

[thesamesam]

I don't know if anything needs to be done around this part of the code:

I think we're okay on this part, as it's specifically to do with handling of virtuals. It could do with some more comments (unrelated to your change) in general though. We should also clean up the "old-style" virtual support too, it's long overdue.

It looks like b7fdd57 caters to a similarish problem for virtuals (not quite the same, but an interesting example or where the pair property wasn't handled at first).

After looking at this for a while, I think it looks good and we can merge it, but I'd still like @zmedico's input when he gets time. But let's see what happens with it in master.

Many thanks for tackling a truly notorious bug which has led to a lot of frustration over the years!