void-linux / xbps

The X Binary Package System (XBPS)
https://voidlinux.org/xbps/
Other
786 stars 125 forks source link

xbps-remove -R does not remove all dependencies when removing multiple packages #377

Open gergesh opened 3 years ago

gergesh commented 3 years ago

The following commands were ran after each other, nothing in between:

λ ~ sudo xbps-remove -R gupnp
gupnp-1.2.4_1 in transaction breaks installed pkg `gupnp-tools-0.10.0_2'
Transaction aborted due to unresolved dependencies.
λ ~ sudo xbps-remove -R gupnp gupnp-tools

Name           Action    Version           New version            Download size
gupnp          remove    1.2.4_1           -                      -
gupnp-tools    remove    0.10.0_2          -                      -
gtksourceview4 remove    4.8.0_1           -                      -
gupnp-av       remove    0.12.11_4         -                      -

Size freed on disk:           4067KB
Space available on disk:       213GB

Do you want to continue? [Y/n] ^C
λ ~ sudo xbps-remove -R gupnp-tools

Name           Action    Version           New version            Download size
gupnp-tools    remove    0.10.0_2          -                      -
gtksourceview4 remove    4.8.0_1           -                      -
gupnp-av       remove    0.12.11_4         -                      -

Size freed on disk:           3837KB
Space available on disk:       213GB

Do you want to continue? [Y/n]
Removing `gupnp-tools-0.10.0_2' ...
Updating GTK+ icon cache for /usr/share/icons/hicolor...
Updating MIME database...
Removed `gupnp-tools-0.10.0_2' successfully.
Removing `gtksourceview4-4.8.0_1' ...
Removed `gtksourceview4-4.8.0_1' successfully.
Removing `gupnp-av-0.12.11_4' ...
Removed `gupnp-av-0.12.11_4' successfully.

0 downloaded, 0 installed, 0 updated, 0 configured, 3 removed.
λ ~ sudo xbps-remove -R gupnp

Name                      Action    Version           New version            Download size
gupnp                     remove    1.2.4_1           -                      -
gssdp                     remove    1.2.3_1           -                      -
libsoup                   remove    2.72.0_1          -                      -
brotli                    remove    1.0.9_2           -                      -
libpsl                    remove    0.21.1_1          -                      -
glib-networking           remove    2.66.0_1          -                      -
libproxy                  remove    0.4.15_1          -                      -
gsettings-desktop-schemas remove    3.38.0_1          -                      -

Size freed on disk:           8219KB
Space available on disk:       213GB

Do you want to continue? [Y/n]
Removing `gupnp-1.2.4_1' ...
Removed `gupnp-1.2.4_1' successfully.
Removing `gssdp-1.2.3_1' ...
Removed `gssdp-1.2.3_1' successfully.
Removing `libsoup-2.72.0_1' ...
Removed `libsoup-2.72.0_1' successfully.
Removing `brotli-1.0.9_2' ...
Removed `brotli-1.0.9_2' successfully.
Removing `libpsl-0.21.1_1' ...
Removed `libpsl-0.21.1_1' successfully.
Removing `glib-networking-2.66.0_1' ...
Updating GLib GIO modules cache...
Removed `glib-networking-2.66.0_1' successfully.
Removing `libproxy-0.4.15_1' ...
Removed `libproxy-0.4.15_1' successfully.
Removing `gsettings-desktop-schemas-3.38.0_1' ...
Refreshing GSettings database from usr/share/glib-2.0/schemas... done.
Removed `gsettings-desktop-schemas-3.38.0_1' successfully.

0 downloaded, 0 installed, 0 updated, 0 configured, 8 removed.

As for my two cents, if I had to guess I'd say it's due to them having shared dependencies which are not considered for removal (checking if the dependencies are orphaned for each of them separately, not taking into consideration that both are removed).

ericonr commented 3 years ago

This isn't terrible, because you can just run with -o for orphans later, but it can be a pain if you have other orphans you wish to keep, and shouldn't be necessary.

It's probably a limitation of the current solver, or maybe it just checks for new orphans in the wrong place. Will try to investigate.

AlexDvorak commented 2 years ago

Hi, I took a look around to see where it might come from after reproducing the steps above with the exact same results. In bin/xbps-remove/main.c, in the loop where it removes the packages, it doesn't seem to do anything special when doing it recursively so it just passes each individual package to remove_pkg with a flag to remove recursively or not.

bin/xbps-remove/main.c
@@ -290,10 +290,10 @@ main(int argc, char **argv)
   for (int i = optind; i < argc; i++) {
        rv = remove_pkg(&xh, argv[i], recursive);
        if (rv == ENOENT) {
            missing++;
            continue;
        } else if (rv != 0) {
            xbps_end(&xh);
            exit(rv);
        }
    }

And remove_pkg seems to just pass it through to xbps_transaction_remove_pkg if it exists. which does check for orphans, but it can only check for orphans while treating one package at a time as removed.

lib/transaction_ops.c
@@ -479,13 +479,13 @@ xbps_transaction_remove_pkg(struct xbps_handle *xhp,
    xbps_array_set_cstring_nocopy(orphans_pkg, 0, pkgname);
    orphans = xbps_find_pkg_orphans(xhp, orphans_pkg);
    xbps_object_release(orphans_pkg);
    if (xbps_object_type(orphans) != XBPS_TYPE_ARRAY)
        return EINVAL;

    for (unsigned int i = 0; i < xbps_array_count(orphans); i++) {
        obj = xbps_array_get(orphans, i);
        xbps_transaction_pkg_type_set(obj, XBPS_TRANS_REMOVE);
        if (!xbps_transaction_store(xhp, pkgs, obj, false)) {
            return EINVAL;
        }
    }

xbps_find_pkg_orphans does have the ability to accept multiple package names for those to be treated as already removed it seems.

xbps_array_t xbps_find_pkg_orphans(struct xbps_handle *xhp, xbps_array_t orphans);

It seems then, that a potential solution would be to have a seperate function in lib/transaction_ops.c that would remove multiple packages recursively (xbps_transaction_remove_pkgs_recursive(struct xbps_handle *xhp, xbps_array_t pkgs))? and in bin/xbps-remove/main.c if the recursive flag is passed, then call that function instead? Have I understood this correctly?

ericonr commented 2 years ago

I ended up not looking into this part of the code at all, but that sounds reasonable.

AlexDvorak commented 2 years ago

Is it worth fixing? (I’d be willing to submit a PR if it is)