[PATCH 0/1] opkg: fix remove pkg with --force-removal-of-dependent-packages failed

[PATCH 0/1] opkg: fix remove pkg with --force-removal-of-dependent-packages failed

[Hongxu Jia]

1. vim local.conf
...
PACKAGE_CLASSES ?= "package_ipk"
IMAGE_INSTALL_append = " perl-modules"
IMAGE_FEATURES += "package-management ssh-server-dropbear"
...

2. bitbake core-image-minimal

3. runqemu qemux86

4. ssh target
opkg remove perl --force-removal-of-dependent-packages

Expected result:
perl removed successfully

//Hongxu

The following changes since commit 505a6b696ae990442de720e5e1135a3e44d5f75c:

  bitbake: fetcher: fix BB_STRICT_CHECKSUM datatype check (2014-10-06 16:09:53 +0100)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib hongxu/fix-opkg
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=hongxu/fix-opkg

Hongxu Jia (1):
  opkg: fix remove pkg with --force-removal-of-dependent-packages failed

 ...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

-- 
1.9.1

[PATCH 1/1] opkg: fix remove pkg with --force-removal-of-dependent-packages failed

[Hongxu Jia]

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 <hongxu.jia@windriver.com>
---
 ...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

diff --git a/meta/recipes-devtools/opkg/opkg/libopkg-opkg_remove.c-avoid-remove-pkg-repeatly-with.patch b/meta/recipes-devtools/opkg/opkg/libopkg-opkg_remove.c-avoid-remove-pkg-repeatly-with.patch
new file mode 100644
index 0000000..08433a9
--- /dev/null
+++ b/meta/recipes-devtools/opkg/opkg/libopkg-opkg_remove.c-avoid-remove-pkg-repeatly-with.patch
@@ -0,0 +1,39 @@
+From 41425d67d3589b1912416a17f740d6407c7834f2 Mon Sep 17 00:00:00 2001
+From: Hongxu Jia <hongxu.jia@windriver.com>
+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 <hongxu.jia@windriver.com>
+---
+ 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 &&
++             pkg->state_flag & SF_FILELIST_CHANGED &&
++             pkg->state_status == SS_NOT_INSTALLED)
++         return 0;
++
+      /* only attempt to remove dependent installed packages if
+       * force_depends is not specified or the package is being
+       * replaced.
+-- 
+1.9.1
+
diff --git a/meta/recipes-devtools/opkg/opkg_0.2.2.bb b/meta/recipes-devtools/opkg/opkg_0.2.2.bb
index 3dd7489..867ff7a 100644
--- a/meta/recipes-devtools/opkg/opkg_0.2.2.bb
+++ b/meta/recipes-devtools/opkg/opkg_0.2.2.bb
@@ -4,6 +4,7 @@ SRC_URI = "http://downloads.yoctoproject.org/releases/${BPN}/${BPN}-${PV}.tar.gz
            file://no-install-recommends.patch \
            file://add-exclude.patch \
            file://opkg-configure.service \
+           file://libopkg-opkg_remove.c-avoid-remove-pkg-repeatly-with.patch \
 "
 
 S = "${WORKDIR}/${BPN}-${PV}"
-- 
1.9.1

Re: [PATCH 1/1] opkg: fix remove pkg with --force-removal-of-dependent-packages failed

[Paul Barker]

On 8 October 2014 13:16, Hongxu Jia <hongxu.jia@windriver.com> wrote:
> 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 <hongxu.jia@windriver.com>
> ---
>  ...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
>
> diff --git a/meta/recipes-devtools/opkg/opkg/libopkg-opkg_remove.c-avoid-remove-pkg-repeatly-with.patch b/meta/recipes-devtools/opkg/opkg/libopkg-opkg_remove.c-avoid-remove-pkg-repeatly-with.patch
> new file mode 100644
> index 0000000..08433a9
> --- /dev/null
> +++ b/meta/recipes-devtools/opkg/opkg/libopkg-opkg_remove.c-avoid-remove-pkg-repeatly-with.patch
> @@ -0,0 +1,39 @@
> +From 41425d67d3589b1912416a17f740d6407c7834f2 Mon Sep 17 00:00:00 2001
> +From: Hongxu Jia <hongxu.jia@windriver.com>
> +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 <hongxu.jia@windriver.com>
> +---
> + 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 &&
> ++             pkg->state_flag & SF_FILELIST_CHANGED &&
> ++             pkg->state_status == SS_NOT_INSTALLED)
> ++         return 0;
> ++
> +      /* only attempt to remove dependent installed packages if
> +       * force_depends is not specified or the package is being
> +       * replaced.
> +--
> +1.9.1
> +

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 to opkg-devel@googlegroups.com
and I'll merge it upstream. If you're too busy I can have a look at
this myself next week.

Cheers,

-- 
Paul Barker

Email: paul@paulbarker.me.uk
http://www.paulbarker.me.uk

Re: [PATCH 1/1] opkg: fix remove pkg with --force-removal-of-dependent-packages failed

[Hongxu Jia]

On 10/11/2014 05:31 PM, Paul Barker wrote:
> 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 toopkg-devel@googlegroups.com
> and I'll merge it upstream. If you're too busy I can have a look at
> this myself next week.

OK, I will try the alternative solution.

//Hongxu

Re: [PATCH 1/1] opkg: fix remove pkg with --force-removal-of-dependent-packages failed

[Hongxu Jia]

On 10/11/2014 05:31 PM, Paul Barker wrote:
> 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

> If you've got time to try this alternative solution could you let me
> know if it works. If so, send the patch toopkg-devel@googlegroups.com
> and I'll merge it upstream. If you're too busy I can have a look at
> this myself next week.

Re: [PATCH 1/1] opkg: fix remove pkg with --force-removal-of-dependent-packages failed

[Hongxu Jia]

On 10/11/2014 06:17 PM, Hongxu Jia wrote:
> On 10/11/2014 05:31 PM, Paul Barker wrote:
>> 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.
>

s/will/will not/

//Hongxu

> //Hongxu
>
>> If you've got time to try this alternative solution could you let me
>> know if it works. If so, send the patch toopkg-devel@googlegroups.com
>> and I'll merge it upstream. If you're too busy I can have a look at
>> this myself next week.
>

Re: [PATCH 1/1] opkg: fix remove pkg with --force-removal-of-dependent-packages failed

[Paul Barker]

[-- Attachment #1: Type: text/plain, Size: 916 bytes --]

On 11 October 2014 11:17, Hongxu Jia <hongxu.jia@windriver.com> wrote:
> On 10/11/2014 05:31 PM, Paul Barker wrote:
>>
>> 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.
>

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

Thanks,

-- 
Paul Barker

Email: paul@paulbarker.me.uk
http://www.paulbarker.me.uk

[-- Attachment #2: 0001-WIP-Attempt-to-fix-duplicate-removal-of-dependent-pa.patch --]
[-- Type: text/x-patch, Size: 1123 bytes --]

From b663a030db08fee1da4976436b58a7f1a19a551d Mon Sep 17 00:00:00 2001
From: Paul Barker <paul@paulbarker.me.uk>
Date: Sat, 11 Oct 2014 10:23:45 +0000
Subject: [PATCH] WIP: Attempt to fix duplicate removal of dependent packages
To: opkg-devel@googlegroups.com

Signed-off-by: Paul Barker <paul@paulbarker.me.uk>
---
 libopkg/opkg_remove.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libopkg/opkg_remove.c b/libopkg/opkg_remove.c
index 34f9154..fb8ab52 100644
--- a/libopkg/opkg_remove.c
+++ b/libopkg/opkg_remove.c
@@ -117,7 +117,8 @@ opkg_remove_dependent_pkgs(pkg_t *pkg, abstract_pkg_t **dependents)
         if (dep_ab_pkg->state_status == SS_INSTALLED) {
             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) {
+                if (dep_pkg->state_status == SS_INSTALLED
+                        && !pkg_vec_contains(dependent_pkgs, dep_pkg)) {
                     pkg_vec_insert(dependent_pkgs, dep_pkg);
                     count++;
                 }
-- 
2.1.2

Re: [PATCH 1/1] opkg: fix remove pkg with --force-removal-of-dependent-packages failed

[Hongxu Jia]

On 10/11/2014 06:27 PM, Paul Barker wrote:
> On 11 October 2014 11:17, Hongxu Jia <hongxu.jia@windriver.com> wrote:
>> On 10/11/2014 05:31 PM, Paul Barker wrote:
>>> 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.
>>
> 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:

--- a/libopkg/opkg_remove.c
+++ b/libopkg/opkg_remove.c
@@ -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++;
+                    }

//Hongxu

Re: [PATCH 1/1] opkg: fix remove pkg with --force-removal-of-dependent-packages failed

[Paul Barker]

On 11 October 2014 11:33, Hongxu Jia <hongxu.jia@windriver.com> wrote:
>
> Here is my patch, and it could not work, I think we are doing
> the same thing:
>
> --- a/libopkg/opkg_remove.c
> +++ b/libopkg/opkg_remove.c
> @@ -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++;
> +                    }
>
> //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.

Thanks,

-- 
Paul Barker

Email: paul@paulbarker.me.uk
http://www.paulbarker.me.uk

Re: [PATCH 1/1] opkg: fix remove pkg with --force-removal-of-dependent-packages failed

[Paul Barker]

On 11 October 2014 11:37, Paul Barker <paul@paulbarker.me.uk> wrote:
> On 11 October 2014 11:33, Hongxu Jia <hongxu.jia@windriver.com> wrote:
>>
>> Here is my patch, and it could not work, I think we are doing
>> the same thing:
>>
>> --- a/libopkg/opkg_remove.c
>> +++ b/libopkg/opkg_remove.c
>> @@ -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++;
>> +                    }
>>
>> //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 <paul@paulbarker.me.uk>

Thanks,

-- 
Paul Barker

Email: paul@paulbarker.me.uk
http://www.paulbarker.me.uk

Re: [PATCH 1/1] opkg: fix remove pkg with --force-removal-of-dependent-packages failed

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 370 bytes --]

On 21 October 2014 17:17, Paul Barker <paul@paulbarker.me.uk> wrote:

> 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 <paul@paulbarker.me.uk>
>

Thanks Paul, tag added and merged to my MUT.

Ross

[-- Attachment #2: Type: text/html, Size: 817 bytes --]