On Mon, Mar 2, 2020 at 9:15 PM Zac Medico <zmed...@gentoo.org> wrote: > > On 3/2/20 1:11 PM, Matt Turner wrote: > > On Sun, Mar 1, 2020 at 10:39 PM Zac Medico <zmed...@gentoo.org> wrote: > >> > >> On 2/20/20 9:29 PM, Matt Turner wrote: > >>> @@ -564,7 +577,22 @@ def findPackages( > >>> > >>> # Exclude if binpkg exists in the porttree and not --deep > >>> if not destructive and port_dbapi.cpv_exists(cpv): > >>> - continue > >>> + if not options['changed-deps']: > >>> + continue > >>> + > >>> + uselist = bin_dbapi.aux_get(cpv, ['USE']).split() > >>> + all_equal = True > >>> + > >>> + for k in ('RDEPEND', 'PDEPEND'): > >>> + binpkg_deps = bin_dbapi.aux_get(cpv, [k]) > >>> + ebuild_deps = port_dbapi.aux_get(cpv, [k]) > >>> + > >>> + if not _deps_equal(binpkg_deps, ebuild_deps, cpv.eapi, > >>> uselist): > >>> + all_equal = False > >>> + break > >>> + > >>> + if all_equal: > >>> + continue > >> > >> If all_equal is True, then none of the other filters have an opportunity > >> to add this package to the dead_binpkgs set. That's not good is it? > > > > There are four cases we skip including packages: 1) exclude list, 2) > > time limit, 3) non-destructive and package still exists (and now > > optionally runtime deps haven't changed), 4) destructive and package > > is installed. Cases (3) and (4) are non-overlapping. > > > > If none of those cases are true, then we add the package to the > > dead_binpkgs set. The logic looks right to me. > > > > Maybe I'm not understanding. > > What I imagine is that you could have some old packages that you > probably want to delete because they're so old, even though their deps > have not changed. Meanwhile you have some packages that are > relatively recent and you'd like to delete them if they have changed deps. > > Given the current logic, I guess you'd have to do separate passes, one > to delete packages based on age and another to delete packages based on > changed deps. Maybe it's fine to require separate passes for this kind > of thing. I supposed the alternative would be to add an --or flag that > would allow you to say that you want to delete packages if they are at > least a certain age or they have changed deps.
Oh, I think I understand now. I was confused about this. I expected that --time-limit=2w to mean that eclean should delete everything older than two weeks, but it's actually the opposite, more or less. It actually means to *keep* everything with less than two weeks old. Surprised me... > -t, --time-limit=<time> - don't delete files modified since <time> So, with that surprising behavior I think my patch is doing the right thing, but with the wrong comment. I'll fix those and send v2 patches with the tabs restored, etc. Thanks a bunch for the review.