someone gave me a patch and I shall bring it to you gentoolkit-eclean-package-names-now-really-protects-all-versions.patch --- a/pym/gentoolkit/eclean/search.py 2020-05-02 13:30:12.958615228 +0200 +++ b/pym/gentoolkit/eclean/search.py 2020-05-02 13:33:31.019597359 +0200 @@ -586,16 +586,17 @@ if _deps_equal(binpkg_deps, ebuild_deps, cpv.eapi, uselist): continue - if destructive and var_dbapi.cpv_exists(cpv): + if destructive and port_dbapi.cpv_exists(cpv): # Exclude if an instance of the package is installed due to # the --package-names option. if cp in installed and port_dbapi.cpv_exists(cpv): continue - # Exclude if BUILD_TIME of binpkg is same as vartree - buildtime = var_dbapi.aux_get(cpv, ['BUILD_TIME'])[0] - if buildtime == bin_dbapi.aux_get(cpv, ['BUILD_TIME'])[0]: - continue + if var_dbapi.cpv_exists(cpv): + # Exclude if BUILD_TIME of binpkg is same as vartree + buildtime = var_dbapi.aux_get(cpv, ['BUILD_TIME'])[0] + if buildtime == bin_dbapi.aux_get(cpv, ['BUILD_TIME'])[0]: + continue binpkg_path = bin_dbapi.bintree.getname(cpv) dead_binpkgs.setdefault(cpv, []).append(binpkg_path) Reproducible: Always
(In reply to Alexander Kern from comment #0) > someone gave me a patch and I shall bring it to you > gentoolkit-eclean-package-names-now-really-protects-all-versions.patch If you want to protect all versions. Then don't use the -d --destruvtive option. Then the following code changes would not even be run. > > --- a/pym/gentoolkit/eclean/search.py 2020-05-02 13:30:12.958615228 +0200 > +++ b/pym/gentoolkit/eclean/search.py 2020-05-02 13:33:31.019597359 +0200 > @@ -586,16 +586,17 @@ > if _deps_equal(binpkg_deps, ebuild_deps, cpv.eapi, > uselist): > continue > > - if destructive and var_dbapi.cpv_exists(cpv): > + if destructive and port_dbapi.cpv_exists(cpv): This seems wrong This was checking to see if the package is installed. This change makes it see if it exists in the current repo trees. See the comment below in the next 2 lines. > # Exclude if an instance of the package is installed > due to > # the --package-names option. > if cp in installed and port_dbapi.cpv_exists(cpv): > continue > This if statement now duplicates the if statement changed above, so port_dbapi.cpv_exists(cpv) is always True at this point. > - # Exclude if BUILD_TIME of binpkg is same as vartree > - buildtime = var_dbapi.aux_get(cpv, ['BUILD_TIME'])[0] > - if buildtime == bin_dbapi.aux_get(cpv, > ['BUILD_TIME'])[0]: > - continue > + if var_dbapi.cpv_exists(cpv): > + # Exclude if BUILD_TIME of binpkg is same as > vartree > + buildtime = var_dbapi.aux_get(cpv, > ['BUILD_TIME'])[0] > + if buildtime == bin_dbapi.aux_get(cpv, > ['BUILD_TIME'])[0]: > + continue > > binpkg_path = bin_dbapi.bintree.getname(cpv) > dead_binpkgs.setdefault(cpv, []).append(binpkg_path) > > Reproducible: Always Overall, I see that the patch was reversing the order of checking both port_dbapi and var_dbapi. Can you explain the reasoning more?
I think we need and a sign-off from whoever wrote the patch before anything could be merged here.
https://pastebin.com/9mjiFzA4 Why we need the patch? Because without, it eclean -dn will remove packages which have another version still installed, and all of the listed version below are currently valid in portage tree, so no reason to delete them (need them for quick downgrading for example) # eclean-pkg -dnp * Building file list for packages cleaning... * Here are the binary packages that would be deleted: [ 1.8 M ] app-arch/rpm-4.14.1 [ 739.2 K ] app-arch/zstd-1.3.7-r1 [ 1.3 M ] app-cdr/cdrtools-3.02_alpha09 [ 3.4 M ] app-crypt/gnupg-2.2.19 [ 141.7 M ] app-office/libreoffice-bin-6.3.4.2-r1 [ 1.4 M ] app-office/libreoffice-l10n-6.3.4.2 [ 633.0 K ] app-portage/eix-0.33.9-r1 [ 160.0 K ] app-portage/portage-utils-0.80 [ 1.7 M ] app-shells/bash-4.4_p23-r1 [ 947.5 K ] app-text/libetonyek-0.1.9 [ 2.7 M ] app-text/libmwaw-0.3.15 [ 949.9 K ] app-text/libstaroffice-0.0.6 [ 994.7 K ] app-text/libwps-0.4.10 [ 1.8 M ] app-text/poppler-0.85.0 [ 15.5 M ] dev-lang/python-2.7.17-r2 [ 24.5 M ] dev-lang/python-3.8.2-r1 [ 89.8 K ] dev-libs/check-0.14.0 [ 12.4 M ] dev-libs/icu-65.1-r1 [ 79.1 K ] dev-libs/json-c-0.13.1-r1 [ 431.2 K ] dev-libs/libburn-1.5.0 [ 459.4 K ] dev-libs/libevent-2.1.8 [ 307.4 K ] dev-libs/libisofs-1.5.0 [ 342.1 K ] dev-libs/libixion-0.14.1 [ 816.5 K ] dev-libs/liborcus-0.14.1-r1 [ 901.7 K ] dev-libs/libpcre-8.42 [ 171.3 K ] dev-libs/libuv-1.35.0 [ 337.3 K ] dev-libs/nettle-3.4.1 [ 1.6 M ] dev-libs/nss-3.51.1 [ 1.7 M ] dev-libs/nss-3.52 [ 5.1 M ] dev-python/pip-19.3.1-r2 [ 2.2 M ] dev-python/setuptools-44.0.0 [ 117.4 K ] dev-util/mdds-1.4.3 [ 135.8 K ] media-gfx/graphite2-1.3.13 [ 476.6 K ] media-libs/alsa-lib-1.2.1.2 [ 102.0 K ] media-libs/gexiv2-0.10.10-r1 [ 1.7 M ] media-libs/harfbuzz-2.6.4 [ 469.7 K ] media-libs/libcdr-0.1.5 [ 414.7 K ] media-libs/libsndfile-1.0.28-r4 [ 4.1 M ] net-misc/anydesk-5.5.1 [ 2.1 M ] net-misc/ntp-4.2.8_p13 [ 859.6 K ] net-misc/wget-1.20.3-r1 [ 7.3 M ] net-print/cups-2.2.13 [ 3.0 M ] sys-apps/portage-2.3.89-r3 [ 3.8 M ] sys-apps/util-linux-2.33.2 [ 46.1 M ] sys-devel/gcc-9.2.0-r2 [ 629.6 K ] sys-fs/ntfs3g-2017.3.23-r2 [ 19.2 M ] sys-libs/glibc-2.29-r700 [ 19.1 M ] sys-libs/glibc-2.30-r8 [ 1.5 M ] sys-libs/ncurses-6.1_p20190609 [ 1.0 M ] sys-libs/readline-7.0_p5-r1 [ 309.0 K ] sys-libs/timezone-data-2019c [ 66.0 M ] www-client/firefox-bin-76.0 [ 653.1 K ] x11-terms/xfce4-terminal-0.8.9.1 =========== [ 406.1 M ] Total space from 53 files would be freed in the packages directory and WITH the patch, non of those packages are listed by that command (it still works the way it should, tested several times) So ignoring my patch is IMHO like totally removing the option --package-names. Now to your questions: --package-names only makes sense with --deep as I just tried again: # eclean-pkg -np !!! --package-names only makes sense in --deep mode. Next you wrote: > - if destructive and var_dbapi.cpv_exists(cpv): > + if destructive and port_dbapi.cpv_exists(cpv): "This seems wrong This was checking to see if the package is installed. This change makes it see if it exists in the current repo trees." That is exaclty why I changed that: I want to prevent deleting if the packageversion still exists in portage. I know that old but valid versions are NOT installed, but that's not the point. LATER I had to re-insert that check towards installed (var...) to avoid an exception when it checks the BUILD_TIME Next you wrote "This if statement now duplicates the if statement changed above, so port_dbapi.cpv_exists(cpv) is always True at this point." That is correct. Don't know if I left it in purpose to reduce nr of lines changed or if I didn't see it. I removed it in the next version of the patch: https://pastebin.com/fW6rKdSY Thanks for having a look at it, I appreciate your questions as only the code writer knows what the code writer thought about it, my fault to not giving the reasons of it all in the first place!
It has been years since I did the big re-write of the eclean code. I'll review it more in the next week or so. It'll take a while to get fully familiar with it again. But I keep getting the feeling that you are missing the point of the -d, --deep option. If I remember correctly, the -d was originally named --destructive which is what my mind keeps in its understanding of eclean. This mode was to remove all but the minimum number of pkgs to re-install with. If you wanted to keep all versions of an installed pkg, then don't pass it the -d option. Thanks for fixing that missed duplication of cpv_exists()
(In reply to Alexander Kern from comment #0) > someone gave me a patch and I shall bring it to you > gentoolkit-eclean-package-names-now-really-protects-all-versions.patch > > --- a/pym/gentoolkit/eclean/search.py 2020-05-02 13:30:12.958615228 +0200 > +++ b/pym/gentoolkit/eclean/search.py 2020-05-02 13:33:31.019597359 +0200 > @@ -586,16 +586,17 @@ > if _deps_equal(binpkg_deps, ebuild_deps, cpv.eapi, > uselist): > continue > > - if destructive and var_dbapi.cpv_exists(cpv): > + if destructive and port_dbapi.cpv_exists(cpv): > # Exclude if an instance of the package is installed > due to > # the --package-names option. When I run `eclean-pkg -d`, I expect it to preserve the binary package corresponding to an installed package even if the corresponding ebuild is not available. The new port_dbapi.cpv_exists(cpv) logic here would cause my binary package to get deleted if the corresponding ebuild is missing for some reason, and I think that would be an unpleasant surprise for myself and others.
it is he, not me: You are right, Brian, concerning --deep on it's own. But --package-names just DOESN'T work without --deep! Let me summarize: 1. I want to keep all valid versions from all packages installed, YES but 2. I want to remove every version of any package that I just removed from installtion! So I want to have a command that after an emerge --depclean also removes related (hence now unneeded) binaries. Without --deep only invalid binaries are deleted (invalid according to portage tree, which is the place to tell if it's valid or not) The man page sais I can! "--package-names option ... will protect files corresponding to all existing versions of installed packages" This feature currently just does not work as told. Now to you, Zac: You have a point there, BUT eclean in the current form does NOT what you want either! Look at the code (or my first patch): We go into that branch in the current version but we only keep the binary (=continue) if it's found in portage tree. In my last patch I changed - if cp in installed and port_dbapi.cpv_exists(cpv): + if cp in installed: because it was doubled due to my change. But it already existed before: Only "valid" (see above) packages are kept. => Only binaries you are ABLE to install are kept! You cannot re-install binaries without the ebuild, so if you are sure you want to keep packages of vanished versions you HAVE to make a copy of the ebuilds (and related files!) into your personal "overlay" in /usr/local/portage.
Created attachment 892238 [details, diff] pastebin content
