1 |
On Mon, Mar 2, 2020 at 9:15 PM Zac Medico <zmedico@g.o> wrote: |
2 |
> |
3 |
> On 3/2/20 1:11 PM, Matt Turner wrote: |
4 |
> > On Sun, Mar 1, 2020 at 10:39 PM Zac Medico <zmedico@g.o> wrote: |
5 |
> >> |
6 |
> >> On 2/20/20 9:29 PM, Matt Turner wrote: |
7 |
> >>> @@ -564,7 +577,22 @@ def findPackages( |
8 |
> >>> |
9 |
> >>> # Exclude if binpkg exists in the porttree and not --deep |
10 |
> >>> if not destructive and port_dbapi.cpv_exists(cpv): |
11 |
> >>> - continue |
12 |
> >>> + if not options['changed-deps']: |
13 |
> >>> + continue |
14 |
> >>> + |
15 |
> >>> + uselist = bin_dbapi.aux_get(cpv, ['USE'])[0].split() |
16 |
> >>> + all_equal = True |
17 |
> >>> + |
18 |
> >>> + for k in ('RDEPEND', 'PDEPEND'): |
19 |
> >>> + binpkg_deps = bin_dbapi.aux_get(cpv, [k]) |
20 |
> >>> + ebuild_deps = port_dbapi.aux_get(cpv, [k]) |
21 |
> >>> + |
22 |
> >>> + if not _deps_equal(binpkg_deps, ebuild_deps, cpv.eapi, uselist): |
23 |
> >>> + all_equal = False |
24 |
> >>> + break |
25 |
> >>> + |
26 |
> >>> + if all_equal: |
27 |
> >>> + continue |
28 |
> >> |
29 |
> >> If all_equal is True, then none of the other filters have an opportunity |
30 |
> >> to add this package to the dead_binpkgs set. That's not good is it? |
31 |
> > |
32 |
> > There are four cases we skip including packages: 1) exclude list, 2) |
33 |
> > time limit, 3) non-destructive and package still exists (and now |
34 |
> > optionally runtime deps haven't changed), 4) destructive and package |
35 |
> > is installed. Cases (3) and (4) are non-overlapping. |
36 |
> > |
37 |
> > If none of those cases are true, then we add the package to the |
38 |
> > dead_binpkgs set. The logic looks right to me. |
39 |
> > |
40 |
> > Maybe I'm not understanding. |
41 |
> |
42 |
> What I imagine is that you could have some old packages that you |
43 |
> probably want to delete because they're so old, even though their deps |
44 |
> have not changed. Meanwhile you have some packages that are |
45 |
> relatively recent and you'd like to delete them if they have changed deps. |
46 |
> |
47 |
> Given the current logic, I guess you'd have to do separate passes, one |
48 |
> to delete packages based on age and another to delete packages based on |
49 |
> changed deps. Maybe it's fine to require separate passes for this kind |
50 |
> of thing. I supposed the alternative would be to add an --or flag that |
51 |
> would allow you to say that you want to delete packages if they are at |
52 |
> least a certain age or they have changed deps. |
53 |
|
54 |
Oh, I think I understand now. |
55 |
|
56 |
I was confused about this. I expected that --time-limit=2w to mean |
57 |
that eclean should delete everything older than two weeks, but it's |
58 |
actually the opposite, more or less. It actually means to *keep* |
59 |
everything with less than two weeks old. Surprised me... |
60 |
|
61 |
> -t, --time-limit=<time> - don't delete files modified since <time> |
62 |
|
63 |
So, with that surprising behavior I think my patch is doing the right |
64 |
thing, but with the wrong comment. I'll fix those and send v2 patches |
65 |
with the tabs restored, etc. |
66 |
|
67 |
Thanks a bunch for the review. |