opkg: fix remove pkg with --force-removal-of-dependent-packages failed

opkg remove perl --force-removal-of-dependent-packages
...
Removing package perl-module-extutils-mm-dos from root...
...
Removing package perl-module-extutils-mm-dos from root...
You can force removal of packages with failed prerm scripts with the option:
--force-remove
No packages removed.
Collected errors:
* pkg_run_script: Internal error: perl-module-extutils-mm-dos has a
NULL tmp_unpack_dir.
* opkg_remove_pkg: not removing package "perl-module-extutils-mm-dos",
prerm script failed
...

While remove pkg with '--force-removal-of-dependent-packages', pkg may be added to remove list multiple times, add status check to make sure pkg only be removed once.

[YOCTO #6819]

Signed-off-by Hongxu Jia <[email protected]>

---
 ...g_remove.c-avoid-remove-pkg-repeatly-with.patch | 39 ++++++++++++++++++++++
 meta/recipes-devtools/opkg/opkg_0.2.2.bb           |  1 +
 2 files changed, 40 insertions(+)
 create mode 100644 meta/recipes-devtools/opkg/opkg/libopkg-opkg_remove.c-avoid-remove-pkg-repeatly-with.patch

meta/recipes-devtools/opkg/opkg/libopkg-opkg_remove.c-avoid-remove-pkg-repeatly-with.patch new file +29 -0

new file mode 100644
index 0000000..08433a9
@@ -0,0 +1,39 @@
+From 41425d67d3589b1912416a17f740d6407c7834f2 Mon Sep 17 00:00:00 2001
+From: Hongxu Jia <[email protected]>
+Date: Wed, 8 Oct 2014 19:53:13 +0800
+Subject: [PATCH] libopkg/opkg_remove.c: avoid remove pkg repeatly with option
+ --force-removal-of-dependent-packages
+
+While remove pkg with '--force-removal-of-dependent-packages',
+pkg may be added to pkgs remove list multiple times, add status
+check to make sure pkg only be removed once.
+
+Upstream-Status: Pending
+
+Signed-off-by: Hongxu Jia <[email protected]>
+---
+ libopkg/opkg_remove.c | 8 ++++++++
+ 1 file changed, 8 insertions(+)
+
+diff --git a/libopkg/opkg_remove.c b/libopkg/opkg_remove.c
+index 34f9154..a225e41 100644
+--- a/libopkg/opkg_remove.c
++++ b/libopkg/opkg_remove.c
+@@ -250,6 +250,14 @@ opkg_remove_pkg(pkg_t *pkg, int from_upgrade)
+      if ((parent_pkg = pkg->parent) == NULL)
+         return 0;
+
++     /* While remove pkg with '--force-removal-of-dependent-packages',
++        pkg may be added to remove list multiple times, add status
++        check to make sure pkg only be removed once. */
++     if (conf->force_removal_of_dependent_packages &&

Your solution looks fine but I'd prefer it to be solved a different way upstream.

It should be possible to ensure that duplicates don't enter the removal queue in the first place by checking the return of pkg_vec_contains(dependent_pkgs, dep_pkg) before calling pkg_vec_insert(dependent_pkgs, dep_pkg) in opkg_remove_dependent_pkgs. This would keep the logic in opkg_remove_pkg clean.

If you've got time to try this alternative solution could you let me know if it works. If so, send the patch [email protected] and I'll merge it upstream. If you're too busy I can have a look at this myself next week.

Your solution looks fine but I'd prefer it to be solved a different way upstream.

It should be possible to ensure that duplicates don't enter the removal queue in the first place by checking the return of pkg_vec_contains(dependent_pkgs, dep_pkg) before calling pkg_vec_insert(dependent_pkgs, dep_pkg) in opkg_remove_dependent_pkgs. This would keep the logic in opkg_remove_pkg clean.

If you've got time to try this alternative solution could you let me know if it works. If so, send the patch [email protected] and I'll merge it upstream. If you're too busy I can have a look at this myself next week.

Quoted 8 lines · Paul Barker

Your solution looks fine but I'd prefer it to be solved a different way upstream.

It should be possible to ensure that duplicates don't enter the removal queue in the first place by checking the return of pkg_vec_contains(dependent_pkgs, dep_pkg) before calling pkg_vec_insert(dependent_pkgs, dep_pkg) in opkg_remove_dependent_pkgs. This would keep the logic in opkg_remove_pkg clean.

The pkg_vec_contains(dependent_pkgs, dep_pkg) could not work here. Because 'dependent_pkgs' is not global which contains all removed pkgs.

The checking will filter duplicated pkg.

//Hongxu

Quoted 4 lines

If you've got time to try this alternative solution could you let me know if it works. If so, send the patch [email protected] and I'll merge it upstream. If you're too busy I can have a look at this myself next week.

Quoted 9 lines · Paul Barker

Your solution looks fine but I'd prefer it to be solved a different way upstream.

It should be possible to ensure that duplicates don't enter the removal queue in the first place by checking the return of pkg_vec_contains(dependent_pkgs, dep_pkg) before calling pkg_vec_insert(dependent_pkgs, dep_pkg) in opkg_remove_dependent_pkgs. This would keep the logic in opkg_remove_pkg clean.

The pkg_vec_contains(dependent_pkgs, dep_pkg) could not work here. Because 'dependent_pkgs' is not global which contains all removed pkgs.

The checking will filter duplicated pkg.

Quoted 10 lines · Hongxu Jia

Quoted 8 lines · Paul Barker

Your solution looks fine but I'd prefer it to be solved a different way upstream.

It should be possible to ensure that duplicates don't enter the removal queue in the first place by checking the return of pkg_vec_contains(dependent_pkgs, dep_pkg) before calling pkg_vec_insert(dependent_pkgs, dep_pkg) in opkg_remove_dependent_pkgs. This would keep the logic in opkg_remove_pkg clean.

Quoted 5 lines

The pkg_vec_contains(dependent_pkgs, dep_pkg) could not work here. Because 'dependent_pkgs' is not global which contains all removed pkgs.

The checking will filter duplicated pkg.

I've attached a patch which should apply to opkg v0.2.2 to show what I intended.

Thanks,

Here is my patch, and it could not work, I think we are doing the same thing:

libopkg/opkg_remove.c +0 -0

@@ -118,8 +118,11 @@ opkg_remove_dependent_pkgs(pkg_t *pkg, abstract_pkg_t
 **dependents)
             for (a = 0; a < dep_ab_pkg->pkgs->len; a++) {
                 pkg_t *dep_pkg = dep_ab_pkg->pkgs->pkgs[a];

Quoted 19 lines · Hongxu Jia

Here is my patch, and it could not work, I think we are doing the same thing:

libopkg/opkg_remove.c +5 -2

@@ -118,8 +118,11 @@ opkg_remove_dependent_pkgs(pkg_t *pkg, abstract_pkg_t
 **dependents)
             for (a = 0; a < dep_ab_pkg->pkgs->len; a++) {
                 pkg_t *dep_pkg = dep_ab_pkg->pkgs->pkgs[a];
                 if (dep_pkg->state_status == SS_INSTALLED) {
-                    pkg_vec_insert(dependent_pkgs, dep_pkg);
-                    count++;
+                    if (!pkg_vec_contains(dependent_pkgs, dep_pkg))
+                    {
+                        pkg_vec_insert(dependent_pkgs, dep_pkg);
+                        count++;
+                    }
 

libopkg/opkg_remove.c opkg_remove_dependent_pkgs(pkg_t *pkg, abstract_pkg_t · line 128

Quoted 1 line

libopkg/opkg_remove.c +0 -0

 //Hongxu

My assumption was that duplicates were only being added during a single call to opkg_remove_dependent_pkgs, as on subsequent calls a package will no longer have state_status == SS_INSTALLED if it has already been removed. If my assumption is wrong, your original patch looks to be the best solution.

Hongxu's original patch applies correctly upstream and passes the opkg test suite. I'll merge this onto the opkg-0.2.x branch and so we can change Upstream-status to 'Backport'.

Acked-by Paul Barker <[email protected]>