Re: [gentoo-portage-dev] [PATCH gentoolkit 2/2] eclean: Add option to delete binpkgs with changed deps
On Mon, Mar 2, 2020 at 9:15 PM Zac Medico wrote:
>
> On 3/2/20 1:11 PM, Matt Turner wrote:
> > On Sun, Mar 1, 2020 at 10:39 PM Zac Medico 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'])[0].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= - don't delete files modified since
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.
Re: [gentoo-portage-dev] [PATCH gentoolkit 2/2] eclean: Add option to delete binpkgs with changed deps
On 3/2/20 1:11 PM, Matt Turner wrote:
> On Sun, Mar 1, 2020 at 10:39 PM Zac Medico 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'])[0].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.
--
Thanks,
Zac
signature.asc
Description: OpenPGP digital signature
Re: [gentoo-portage-dev] [PATCH gentoolkit 2/2] eclean: Add option to delete binpkgs with changed deps
On 3/2/20 1:02 PM, Matt Turner wrote:
> On Mon, Mar 2, 2020 at 12:40 PM Matt Turner wrote:
>>
>> On Sun, Mar 1, 2020 at 10:32 PM Zac Medico wrote:
>>>
>>> On 2/20/20 9:29 PM, Matt Turner wrote:
+
def findPackages(
options,
exclude=None,
@@ -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'])[0].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 destructive and var_dbapi.cpv_exists(cpv):
# Exclude if an instance of the package is installed due to
>>>
>>> The aux_get calls are expensive, so it's more efficient to get multiple
>>> values with each call like:
>>> keys = ('RDEPEND', 'PDEPEND')
>>> binpkg_deps = dict(zip(keys, bin_dbapi.aux_get(cpv, keys))
>>> ebuild_deps = dict(zip(keys, port_dbapi.aux_get(cpv, keys))
>>>
>>> Otherwise, looks good.
>>
>> Thanks, that makes the code a lot nicer too.
>
> Actually, use_reduce wants a list (it calls .split). Wrapping those in
> list() looks like it works, but I suspect that's not as you intended.
> What does the zip add over just doing this?
>
> binpkg_deps = bin_dbapi.aux_get(cpv, keys)
> ebuild_deps = port_dbapi.aux_get(cpv, keys)
Using dict(zip(keys, port_dbapi.aux_get(cpv, keys)) is only useful if
you want to use a dictionary to access the values. However, if lists are
good enough then you might just use those instead. You could even join
the values together like this:
ebuild_deps = ' '.join(port_dbapi.aux_get(cpv, keys))
--
Thanks,
Zac
signature.asc
Description: OpenPGP digital signature
Re: [gentoo-portage-dev] [PATCH gentoolkit 2/2] eclean: Add option to delete binpkgs with changed deps
On Sun, Mar 1, 2020 at 10:39 PM Zac Medico 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'])[0].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.
With your other suggestion in place, the code looks like this, which
is hopefully clearer.
# Exclude if binpkg exists in the porttree and not --deep
if not destructive and port_dbapi.cpv_exists(cpv):
if not options['changed-deps']:
continue
keys = ('RDEPEND', 'PDEPEND')
binpkg_deps = bin_dbapi.aux_get(cpv, keys)
ebuild_deps = port_dbapi.aux_get(cpv, keys)
uselist = bin_dbapi.aux_get(cpv, ['USE'])[0].split()
if _deps_equal(binpkg_deps, ebuild_deps, cpv.eapi, uselist):
continue
Unfortunately I don't have any packages with changed-deps at the
moment for testing :(
Re: [gentoo-portage-dev] [PATCH gentoolkit 2/2] eclean: Add option to delete binpkgs with changed deps
On Mon, Mar 2, 2020 at 12:40 PM Matt Turner wrote:
>
> On Sun, Mar 1, 2020 at 10:32 PM Zac Medico wrote:
> >
> > On 2/20/20 9:29 PM, Matt Turner wrote:
> > > +
> > > def findPackages(
> > > options,
> > > exclude=None,
> > > @@ -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'])[0].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 destructive and var_dbapi.cpv_exists(cpv):
> > > # Exclude if an instance of the package is installed due to
> > >
> >
> > The aux_get calls are expensive, so it's more efficient to get multiple
> > values with each call like:
> > keys = ('RDEPEND', 'PDEPEND')
> > binpkg_deps = dict(zip(keys, bin_dbapi.aux_get(cpv, keys))
> > ebuild_deps = dict(zip(keys, port_dbapi.aux_get(cpv, keys))
> >
> > Otherwise, looks good.
>
> Thanks, that makes the code a lot nicer too.
Actually, use_reduce wants a list (it calls .split). Wrapping those in
list() looks like it works, but I suspect that's not as you intended.
What does the zip add over just doing this?
binpkg_deps = bin_dbapi.aux_get(cpv, keys)
ebuild_deps = port_dbapi.aux_get(cpv, keys)
Re: [gentoo-portage-dev] [PATCH gentoolkit 2/2] eclean: Add option to delete binpkgs with changed deps
On Sun, Mar 1, 2020 at 10:32 PM Zac Medico wrote:
>
> On 2/20/20 9:29 PM, Matt Turner wrote:
> > +
> > def findPackages(
> > options,
> > exclude=None,
> > @@ -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'])[0].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 destructive and var_dbapi.cpv_exists(cpv):
> > # Exclude if an instance of the package is installed due to
> >
>
> The aux_get calls are expensive, so it's more efficient to get multiple
> values with each call like:
> keys = ('RDEPEND', 'PDEPEND')
> binpkg_deps = dict(zip(keys, bin_dbapi.aux_get(cpv, keys))
> ebuild_deps = dict(zip(keys, port_dbapi.aux_get(cpv, keys))
>
> Otherwise, looks good.
Thanks, that makes the code a lot nicer too.
Re: [gentoo-portage-dev] [PATCH gentoolkit 2/2] eclean: Add option to delete binpkgs with changed deps
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'])[0].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?
--
Thanks,
Zac
signature.asc
Description: OpenPGP digital signature
Re: [gentoo-portage-dev] [PATCH gentoolkit 2/2] eclean: Add option to delete binpkgs with changed deps
On 2/20/20 9:29 PM, Matt Turner wrote:
> +
> def findPackages(
> options,
> exclude=None,
> @@ -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'])[0].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 destructive and var_dbapi.cpv_exists(cpv):
> # Exclude if an instance of the package is installed due to
>
The aux_get calls are expensive, so it's more efficient to get multiple
values with each call like:
keys = ('RDEPEND', 'PDEPEND')
binpkg_deps = dict(zip(keys, bin_dbapi.aux_get(cpv, keys))
ebuild_deps = dict(zip(keys, port_dbapi.aux_get(cpv, keys))
Otherwise, looks good.
--
Thanks,
Zac
signature.asc
Description: OpenPGP digital signature
