All of lore.kernel.org
 help / color / mirror / Atom feed
* smartpm: Don't ignore error if RPM transaction fails without problems
@ 2016-05-17 12:54 Klauer, Daniel
  2016-05-17 12:58 ` [PATCH] " Klauer, Daniel
  0 siblings, 1 reply; 15+ messages in thread
From: Klauer, Daniel @ 2016-05-17 12:54 UTC (permalink / raw)
  To: openembedded-core

SmartPM could misinterpret RPM transaction error as success,
if ts.run() (RPM Python API) returns an empty problems list.

This could happen for example if the RPM database is partially corrupted
such that the transaction does not have any problems like conflicts or
missing dependencies, but still can't be committed.

The added patch fixes the problem in the upstream sources;
one of the existing patches has to be adjusted to still apply.

Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
---
 .../python/python-smartpm/smart-attempt.patch      |  6 +--
 .../smart-rpm-transaction-failure-check.patch      | 57 ++++++++++++++++++++++
 meta/recipes-devtools/python/python-smartpm_git.bb |  1 +
 3 files changed, 61 insertions(+), 3 deletions(-)
 create mode 100644 meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-check.patch

diff --git a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
index ec98e03..5aedc88 100644
--- a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
+++ b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
@@ -36,7 +36,7 @@ index 9bbd952..ba6405a 100644
          finally:
              del getTS.ts
              cb.grabOutput(False)
-+            if probs and sysconf.has("attempt-install", soft=True):
++            if (probs is not None) and sysconf.has("attempt-install", soft=True):
 +                def remove_conflict(pkgNEVR):
 +                    for key in changeset.keys():
 +                        if pkgNEVR == str(key):
@@ -67,8 +67,8 @@ index 9bbd952..ba6405a 100644
 +                        retry = 0
 +
              prog.setDone()
--            if probs:
-+            if probs and (not retry):
+-            if probs is not None:
++            if (probs is not None) and (not retry):
                  raise Error, "\n".join([x[0] for x in probs])
              prog.stop()
 +            if retry and len(changeset):
diff --git a/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-check.patch b/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-check.patch
new file mode 100644
index 0000000..a740ddd
--- /dev/null
+++ b/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-check.patch
@@ -0,0 +1,57 @@
+From 0c55d7e18f40465e95e8e4bf22af01f5d4477d3c Mon Sep 17 00:00:00 2001
+From: Daniel Klauer <daniel.klauer@gin.de>
+Date: Wed, 11 May 2016 17:22:49 +0200
+Subject: [PATCH] rpm: Don't ignore transaction error with empty problems list
+
+SmartPM could misinterpret RPM transaction error as success,
+if ts.run() (RPM Python API) returns an empty problems list,
+because of incorrect check for None which treated empty list
+to be the same as None when it has different meaning.
+
+ts.run() returns:
+* None in case of success
+* problems list in case of error, may be empty
+(look at rpmts_Run() in rpm-5.4.14/python/rpmts-py.c [1])
+
+"if mylist" is not good enough to check for error here, because it will
+treat an empty list as "false" because its len() == 0 [2].
+
+ts.check() seems to be different (it's ok for it to return an empty list),
+but for consistency it should be made clear that it can return either None,
+an empty list or a non-empty list.
+
+[1] http://rpm5.org/cvs/fileview?f=rpm/python/rpmts-py.c&v=1.111.2.3
+[2] https://docs.python.org/2/library/stdtypes.html#truth-value-testing
+
+Upstream-Status: Pending
+
+Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
+---
+ smart/backends/rpm/pm.py | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/smart/backends/rpm/pm.py b/smart/backends/rpm/pm.py
+index 9bbd952..635f726 100644
+--- a/smart/backends/rpm/pm.py
++++ b/smart/backends/rpm/pm.py
+@@ -208,7 +208,7 @@ class RPMPackageManager(PackageManager):
+         force = sysconf.get("rpm-force", False)
+         if not force:
+             probs = ts.check()
+-            if probs:
++            if (probs is not None) and (len(probs) != 0):
+                 problines = []
+                 for prob in probs:
+                     name1 = "%s-%s-%s" % prob[0]
+@@ -247,7 +247,7 @@ class RPMPackageManager(PackageManager):
+             del getTS.ts
+             cb.grabOutput(False)
+             prog.setDone()
+-            if probs:
++            if probs is not None:
+                 raise Error, "\n".join([x[0] for x in probs])
+             prog.stop()
+ 
+-- 
+1.9.1
+
diff --git a/meta/recipes-devtools/python/python-smartpm_git.bb b/meta/recipes-devtools/python/python-smartpm_git.bb
index d9a908d..0d7f88c 100644
--- a/meta/recipes-devtools/python/python-smartpm_git.bb
+++ b/meta/recipes-devtools/python/python-smartpm_git.bb
@@ -17,6 +17,7 @@ SRC_URI = "\
           file://smart-recommends.patch \
           file://smart-improve-error-reporting.patch \
           file://smart-channelsdir.patch \
+          file://smart-rpm-transaction-failure-check.patch \
           file://smart-attempt.patch \
           file://smart-attempt-fix.patch \
           file://smart-add-for-rpm-ignoresize-check.patch \
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
  2016-05-17 12:54 smartpm: Don't ignore error if RPM transaction fails without problems Klauer, Daniel
@ 2016-05-17 12:58 ` Klauer, Daniel
  2016-06-07 13:17   ` Herve Jourdain
  0 siblings, 1 reply; 15+ messages in thread
From: Klauer, Daniel @ 2016-05-17 12:58 UTC (permalink / raw)
  To: openembedded-core

SmartPM could misinterpret RPM transaction error as success,
if ts.run() (RPM Python API) returns an empty problems list.

This could happen for example if the RPM database is partially corrupted
such that the transaction does not have any problems like conflicts or
missing dependencies, but still can't be committed.

The added patch fixes the problem in the upstream sources;
one of the existing patches has to be adjusted to still apply.

Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
---
 .../python/python-smartpm/smart-attempt.patch      |  6 +--
 .../smart-rpm-transaction-failure-check.patch      | 57 ++++++++++++++++++++++
 meta/recipes-devtools/python/python-smartpm_git.bb |  1 +
 3 files changed, 61 insertions(+), 3 deletions(-)
 create mode 100644 meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-check.patch

diff --git a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
index ec98e03..5aedc88 100644
--- a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
+++ b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
@@ -36,7 +36,7 @@ index 9bbd952..ba6405a 100644
          finally:
              del getTS.ts
              cb.grabOutput(False)
-+            if probs and sysconf.has("attempt-install", soft=True):
++            if (probs is not None) and sysconf.has("attempt-install", soft=True):
 +                def remove_conflict(pkgNEVR):
 +                    for key in changeset.keys():
 +                        if pkgNEVR == str(key):
@@ -67,8 +67,8 @@ index 9bbd952..ba6405a 100644
 +                        retry = 0
 +
              prog.setDone()
--            if probs:
-+            if probs and (not retry):
+-            if probs is not None:
++            if (probs is not None) and (not retry):
                  raise Error, "\n".join([x[0] for x in probs])
              prog.stop()
 +            if retry and len(changeset):
diff --git a/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-check.patch b/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-check.patch
new file mode 100644
index 0000000..a740ddd
--- /dev/null
+++ b/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-check.patch
@@ -0,0 +1,57 @@
+From 0c55d7e18f40465e95e8e4bf22af01f5d4477d3c Mon Sep 17 00:00:00 2001
+From: Daniel Klauer <daniel.klauer@gin.de>
+Date: Wed, 11 May 2016 17:22:49 +0200
+Subject: [PATCH] rpm: Don't ignore transaction error with empty problems list
+
+SmartPM could misinterpret RPM transaction error as success,
+if ts.run() (RPM Python API) returns an empty problems list,
+because of incorrect check for None which treated empty list
+to be the same as None when it has different meaning.
+
+ts.run() returns:
+* None in case of success
+* problems list in case of error, may be empty
+(look at rpmts_Run() in rpm-5.4.14/python/rpmts-py.c [1])
+
+"if mylist" is not good enough to check for error here, because it will
+treat an empty list as "false" because its len() == 0 [2].
+
+ts.check() seems to be different (it's ok for it to return an empty list),
+but for consistency it should be made clear that it can return either None,
+an empty list or a non-empty list.
+
+[1] http://rpm5.org/cvs/fileview?f=rpm/python/rpmts-py.c&v=1.111.2.3
+[2] https://docs.python.org/2/library/stdtypes.html#truth-value-testing
+
+Upstream-Status: Pending
+
+Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
+---
+ smart/backends/rpm/pm.py | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/smart/backends/rpm/pm.py b/smart/backends/rpm/pm.py
+index 9bbd952..635f726 100644
+--- a/smart/backends/rpm/pm.py
++++ b/smart/backends/rpm/pm.py
+@@ -208,7 +208,7 @@ class RPMPackageManager(PackageManager):
+         force = sysconf.get("rpm-force", False)
+         if not force:
+             probs = ts.check()
+-            if probs:
++            if (probs is not None) and (len(probs) != 0):
+                 problines = []
+                 for prob in probs:
+                     name1 = "%s-%s-%s" % prob[0]
+@@ -247,7 +247,7 @@ class RPMPackageManager(PackageManager):
+             del getTS.ts
+             cb.grabOutput(False)
+             prog.setDone()
+-            if probs:
++            if probs is not None:
+                 raise Error, "\n".join([x[0] for x in probs])
+             prog.stop()
+
+--
+1.9.1
+
diff --git a/meta/recipes-devtools/python/python-smartpm_git.bb b/meta/recipes-devtools/python/python-smartpm_git.bb
index d9a908d..0d7f88c 100644
--- a/meta/recipes-devtools/python/python-smartpm_git.bb
+++ b/meta/recipes-devtools/python/python-smartpm_git.bb
@@ -17,6 +17,7 @@ SRC_URI = "\
           file://smart-recommends.patch \
           file://smart-improve-error-reporting.patch \
           file://smart-channelsdir.patch \
+          file://smart-rpm-transaction-failure-check.patch \
           file://smart-attempt.patch \
           file://smart-attempt-fix.patch \
           file://smart-add-for-rpm-ignoresize-check.patch \
--
1.9.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
  2016-05-17 12:58 ` [PATCH] " Klauer, Daniel
@ 2016-06-07 13:17   ` Herve Jourdain
  2016-06-08 11:35     ` Klauer, Daniel
       [not found]     ` <1465381435600.39214@gin.de>
  0 siblings, 2 replies; 15+ messages in thread
From: Herve Jourdain @ 2016-06-07 13:17 UTC (permalink / raw)
  To: 'Klauer, Daniel', openembedded-core

Hi Daniel,

I've just met a problem linked to that patch...
Indeed, it appears that your modifications make the original patch work as
intended: check if there are problems like RPMPROB_NEW_FILE_CONFLICT or
RPMPROB_FILE_CONFLICT, then remove the packages from the list of packages to
apply (because it's attempt-install), then try the same transaction again...

The problem is: if not all the packages have problems, then the packages
that didn't have problems installing in the first place will now get an
error: "package already installed"!!!
And since it's not one of the problems that are handled in pm.py, it will
just fail...

What do you think would be the best way to solve this: add a check for all
the codes linked to package already installed in the "attempt-only" check in
pm.py?
Or try to cancel the whole previous transaction - and then keep on
committing the new one?

I can generate a new patch if you want, after your guidance, or let you fix
it, like you prefer.
I do have an environment where I can reproduce this problem easily enough.

Herve

-----Original Message-----
From: openembedded-core-bounces@lists.openembedded.org
[mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
Klauer, Daniel
Sent: mardi 17 mai 2016 14:58
To: openembedded-core@lists.openembedded.org
Subject: [OE-core] [PATCH] smartpm: Don't ignore error if RPM transaction
fails without problems

SmartPM could misinterpret RPM transaction error as success, if ts.run()
(RPM Python API) returns an empty problems list.

This could happen for example if the RPM database is partially corrupted
such that the transaction does not have any problems like conflicts or
missing dependencies, but still can't be committed.

The added patch fixes the problem in the upstream sources; one of the
existing patches has to be adjusted to still apply.

Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
---
 .../python/python-smartpm/smart-attempt.patch      |  6 +--
 .../smart-rpm-transaction-failure-check.patch      | 57
++++++++++++++++++++++
 meta/recipes-devtools/python/python-smartpm_git.bb |  1 +
 3 files changed, 61 insertions(+), 3 deletions(-)  create mode 100644
meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-ch
eck.patch

diff --git a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
index ec98e03..5aedc88 100644
--- a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
+++ b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
@@ -36,7 +36,7 @@ index 9bbd952..ba6405a 100644
          finally:
              del getTS.ts
              cb.grabOutput(False)
-+            if probs and sysconf.has("attempt-install", soft=True):
++            if (probs is not None) and sysconf.has("attempt-install",
soft=True):
 +                def remove_conflict(pkgNEVR):
 +                    for key in changeset.keys():
 +                        if pkgNEVR == str(key):
@@ -67,8 +67,8 @@ index 9bbd952..ba6405a 100644
 +                        retry = 0
 +
              prog.setDone()
--            if probs:
-+            if probs and (not retry):
+-            if probs is not None:
++            if (probs is not None) and (not retry):
                  raise Error, "\n".join([x[0] for x in probs])
              prog.stop()
 +            if retry and len(changeset):
diff --git
a/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-
check.patch
b/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-
check.patch
new file mode 100644
index 0000000..a740ddd
--- /dev/null
+++ b/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-
+++ failure-check.patch
@@ -0,0 +1,57 @@
+From 0c55d7e18f40465e95e8e4bf22af01f5d4477d3c Mon Sep 17 00:00:00 2001
+From: Daniel Klauer <daniel.klauer@gin.de>
+Date: Wed, 11 May 2016 17:22:49 +0200
+Subject: [PATCH] rpm: Don't ignore transaction error with empty 
+problems list
+
+SmartPM could misinterpret RPM transaction error as success, if 
+ts.run() (RPM Python API) returns an empty problems list, because of 
+incorrect check for None which treated empty list to be the same as 
+None when it has different meaning.
+
+ts.run() returns:
+* None in case of success
+* problems list in case of error, may be empty (look at rpmts_Run() in 
+rpm-5.4.14/python/rpmts-py.c [1])
+
+"if mylist" is not good enough to check for error here, because it will 
+treat an empty list as "false" because its len() == 0 [2].
+
+ts.check() seems to be different (it's ok for it to return an empty 
+list), but for consistency it should be made clear that it can return 
+either None, an empty list or a non-empty list.
+
+[1] http://rpm5.org/cvs/fileview?f=rpm/python/rpmts-py.c&v=1.111.2.3
+[2] https://docs.python.org/2/library/stdtypes.html#truth-value-testing
+
+Upstream-Status: Pending
+
+Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
+---
+ smart/backends/rpm/pm.py | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/smart/backends/rpm/pm.py b/smart/backends/rpm/pm.py index 
+9bbd952..635f726 100644
+--- a/smart/backends/rpm/pm.py
++++ b/smart/backends/rpm/pm.py
+@@ -208,7 +208,7 @@ class RPMPackageManager(PackageManager):
+         force = sysconf.get("rpm-force", False)
+         if not force:
+             probs = ts.check()
+-            if probs:
++            if (probs is not None) and (len(probs) != 0):
+                 problines = []
+                 for prob in probs:
+                     name1 = "%s-%s-%s" % prob[0] @@ -247,7 +247,7 @@ 
+class RPMPackageManager(PackageManager):
+             del getTS.ts
+             cb.grabOutput(False)
+             prog.setDone()
+-            if probs:
++            if probs is not None:
+                 raise Error, "\n".join([x[0] for x in probs])
+             prog.stop()
+
+--
+1.9.1
+
diff --git a/meta/recipes-devtools/python/python-smartpm_git.bb
b/meta/recipes-devtools/python/python-smartpm_git.bb
index d9a908d..0d7f88c 100644
--- a/meta/recipes-devtools/python/python-smartpm_git.bb
+++ b/meta/recipes-devtools/python/python-smartpm_git.bb
@@ -17,6 +17,7 @@ SRC_URI = "\
           file://smart-recommends.patch \
           file://smart-improve-error-reporting.patch \
           file://smart-channelsdir.patch \
+          file://smart-rpm-transaction-failure-check.patch \
           file://smart-attempt.patch \
           file://smart-attempt-fix.patch \
           file://smart-add-for-rpm-ignoresize-check.patch \
--
1.9.1

--
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
  2016-06-07 13:17   ` Herve Jourdain
@ 2016-06-08 11:35     ` Klauer, Daniel
       [not found]     ` <1465381435600.39214@gin.de>
  1 sibling, 0 replies; 15+ messages in thread
From: Klauer, Daniel @ 2016-06-08 11:35 UTC (permalink / raw)
  To: openembedded-core

Hello,

I don't know the details of the attempt-install feature, but it looks like my patch did change the retry behaviour, due to this change in smart/backends/rpm/pm.py (in smart-attempt.patch):
-            if probs and sysconf.has("attempt-install", soft=True):
+            if (probs is not None) and sysconf.has("attempt-install", soft=True):

Unlike before, this if block now sets retry = 1 if len(probs) == 0. It looks to me like the attempt-install feature only wants to retry in case it found file conflicts, and there aren't any other problems - but unfortunately retry is only set back to 0 if it finds something other than file conflicts, the case of len(probs) == 0 is not handled inside the if block.

Is this the problem you're seeing? I think it could be fixed by reverting that one change, maybe like this:
-            if (probs is not None) and sysconf.has("attempt-install", soft=True):
+            # If there are file conflicts, remove conflicting packages from transaction and retry,
+            # unless there are other problems too.
+            if (probs is not None) and (len(probs) != 0) and sysconf.has("attempt-install", soft=True):

Daniel
________________________________________
From: Herve Jourdain <herve.jourdain@neuf.fr>
Sent: Tuesday, June 7, 2016 15:17
To: Klauer, Daniel; openembedded-core@lists.openembedded.org
Subject: RE: [OE-core] [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems

Hi Daniel,

I've just met a problem linked to that patch...
Indeed, it appears that your modifications make the original patch work as
intended: check if there are problems like RPMPROB_NEW_FILE_CONFLICT or
RPMPROB_FILE_CONFLICT, then remove the packages from the list of packages to
apply (because it's attempt-install), then try the same transaction again...

The problem is: if not all the packages have problems, then the packages
that didn't have problems installing in the first place will now get an
error: "package already installed"!!!
And since it's not one of the problems that are handled in pm.py, it will
just fail...

What do you think would be the best way to solve this: add a check for all
the codes linked to package already installed in the "attempt-only" check in
pm.py?
Or try to cancel the whole previous transaction - and then keep on
committing the new one?

I can generate a new patch if you want, after your guidance, or let you fix
it, like you prefer.
I do have an environment where I can reproduce this problem easily enough.

Herve

-----Original Message-----
From: openembedded-core-bounces@lists.openembedded.org
[mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
Klauer, Daniel
Sent: mardi 17 mai 2016 14:58
To: openembedded-core@lists.openembedded.org
Subject: [OE-core] [PATCH] smartpm: Don't ignore error if RPM transaction
fails without problems

SmartPM could misinterpret RPM transaction error as success, if ts.run()
(RPM Python API) returns an empty problems list.

This could happen for example if the RPM database is partially corrupted
such that the transaction does not have any problems like conflicts or
missing dependencies, but still can't be committed.

The added patch fixes the problem in the upstream sources; one of the
existing patches has to be adjusted to still apply.

Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
---
 .../python/python-smartpm/smart-attempt.patch      |  6 +--
 .../smart-rpm-transaction-failure-check.patch      | 57
++++++++++++++++++++++
 meta/recipes-devtools/python/python-smartpm_git.bb |  1 +
 3 files changed, 61 insertions(+), 3 deletions(-)  create mode 100644
meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-ch
eck.patch

diff --git a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
index ec98e03..5aedc88 100644
--- a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
+++ b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
@@ -36,7 +36,7 @@ index 9bbd952..ba6405a 100644
          finally:
              del getTS.ts
              cb.grabOutput(False)
-+            if probs and sysconf.has("attempt-install", soft=True):
++            if (probs is not None) and sysconf.has("attempt-install",
soft=True):
 +                def remove_conflict(pkgNEVR):
 +                    for key in changeset.keys():
 +                        if pkgNEVR == str(key):
@@ -67,8 +67,8 @@ index 9bbd952..ba6405a 100644
 +                        retry = 0
 +
              prog.setDone()
--            if probs:
-+            if probs and (not retry):
+-            if probs is not None:
++            if (probs is not None) and (not retry):
                  raise Error, "\n".join([x[0] for x in probs])
              prog.stop()
 +            if retry and len(changeset):
diff --git
a/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-
check.patch
b/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-
check.patch
new file mode 100644
index 0000000..a740ddd
--- /dev/null
+++ b/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-
+++ failure-check.patch
@@ -0,0 +1,57 @@
+From 0c55d7e18f40465e95e8e4bf22af01f5d4477d3c Mon Sep 17 00:00:00 2001
+From: Daniel Klauer <daniel.klauer@gin.de>
+Date: Wed, 11 May 2016 17:22:49 +0200
+Subject: [PATCH] rpm: Don't ignore transaction error with empty
+problems list
+
+SmartPM could misinterpret RPM transaction error as success, if
+ts.run() (RPM Python API) returns an empty problems list, because of
+incorrect check for None which treated empty list to be the same as
+None when it has different meaning.
+
+ts.run() returns:
+* None in case of success
+* problems list in case of error, may be empty (look at rpmts_Run() in
+rpm-5.4.14/python/rpmts-py.c [1])
+
+"if mylist" is not good enough to check for error here, because it will
+treat an empty list as "false" because its len() == 0 [2].
+
+ts.check() seems to be different (it's ok for it to return an empty
+list), but for consistency it should be made clear that it can return
+either None, an empty list or a non-empty list.
+
+[1] http://rpm5.org/cvs/fileview?f=rpm/python/rpmts-py.c&v=1.111.2.3
+[2] https://docs.python.org/2/library/stdtypes.html#truth-value-testing
+
+Upstream-Status: Pending
+
+Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
+---
+ smart/backends/rpm/pm.py | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/smart/backends/rpm/pm.py b/smart/backends/rpm/pm.py index
+9bbd952..635f726 100644
+--- a/smart/backends/rpm/pm.py
++++ b/smart/backends/rpm/pm.py
+@@ -208,7 +208,7 @@ class RPMPackageManager(PackageManager):
+         force = sysconf.get("rpm-force", False)
+         if not force:
+             probs = ts.check()
+-            if probs:
++            if (probs is not None) and (len(probs) != 0):
+                 problines = []
+                 for prob in probs:
+                     name1 = "%s-%s-%s" % prob[0] @@ -247,7 +247,7 @@
+class RPMPackageManager(PackageManager):
+             del getTS.ts
+             cb.grabOutput(False)
+             prog.setDone()
+-            if probs:
++            if probs is not None:
+                 raise Error, "\n".join([x[0] for x in probs])
+             prog.stop()
+
+--
+1.9.1
+
diff --git a/meta/recipes-devtools/python/python-smartpm_git.bb
b/meta/recipes-devtools/python/python-smartpm_git.bb
index d9a908d..0d7f88c 100644
--- a/meta/recipes-devtools/python/python-smartpm_git.bb
+++ b/meta/recipes-devtools/python/python-smartpm_git.bb
@@ -17,6 +17,7 @@ SRC_URI = "\
           file://smart-recommends.patch \
           file://smart-improve-error-reporting.patch \
           file://smart-channelsdir.patch \
+          file://smart-rpm-transaction-failure-check.patch \
           file://smart-attempt.patch \
           file://smart-attempt-fix.patch \
           file://smart-add-for-rpm-ignoresize-check.patch \
--
1.9.1

--
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core






^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
       [not found]     ` <1465381435600.39214@gin.de>
@ 2016-06-08 12:52       ` Herve Jourdain
  2016-06-08 15:43         ` Klauer, Daniel
  0 siblings, 1 reply; 15+ messages in thread
From: Herve Jourdain @ 2016-06-08 12:52 UTC (permalink / raw)
  To: 'Klauer, Daniel', openembedded-core

Hi,

Yes, that's what I'm seeing... And setting the check with (len(probs)!=0) in
case of retry makes it go away (I tested that exact same thing yesterday),
provided another small modification is added.
You also need to add another check just before raising the error, or you
would end up getting an "unknown error" raised there.
I basically replaced:
-        if (probs is not None) and (not retry):
+       if (probs is not None) and ((len(probs) != 0) or not
sysconf.has("attempt-install", soft=True)) and (not retry):

BUT reflecting on the whole scheme, I'm wondering how it will work in case
of file found conflict, since the problem package gets removed from the
list, but the list is committed again, with most packages already
installed...
I therefore wonder that there could be the same error that I got in the end,
i.e failing with package already installed - which should not fail for
attempt only.

Regarding the "empty" probs list issue when in attempted mode, do you want
to issue the patch, or do you prefer I do it?

Herve

-----Original Message-----
From: Klauer, Daniel [mailto:Daniel.Klauer@gin.de] 
Sent: mercredi 8 juin 2016 12:24
To: Herve Jourdain <herve.jourdain@neuf.fr>;
openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] smartpm: Don't ignore error if RPM
transaction fails without problems

Hello,

I don't know the details of the attempt-install feature, but it looks like
my patch did change the retry behaviour, due to this change in
smart/backends/rpm/pm.py (in smart-attempt.patch):
-            if probs and sysconf.has("attempt-install", soft=True):
+            if (probs is not None) and sysconf.has("attempt-install",
soft=True):

Unlike before, this if block now sets retry = 1 if len(probs) == 0. It looks
to me like the attempt-install feature only wants to retry in case it found
file conflicts, and there aren't any other problems - but unfortunately
retry is only set back to 0 if it finds something other than file conflicts,
the case of len(probs) == 0 is not handled inside the if block.

Is this the problem you're seeing? I think it could be fixed by reverting
that one change, maybe like this:
-            if (probs is not None) and sysconf.has("attempt-install",
soft=True):
+            # If there are file conflicts, remove conflicting packages from
transaction and retry,
+            # unless there are other problems too.
+            if (probs is not None) and (len(probs) != 0) and
sysconf.has("attempt-install", soft=True):

Daniel

________________________________________
From: Herve Jourdain <herve.jourdain@neuf.fr>
Sent: Tuesday, June 7, 2016 15:17
To: Klauer, Daniel; openembedded-core@lists.openembedded.org
Subject: RE: [OE-core] [PATCH] smartpm: Don't ignore error if RPM
transaction fails without problems

Hi Daniel,

I've just met a problem linked to that patch...
Indeed, it appears that your modifications make the original patch work as
intended: check if there are problems like RPMPROB_NEW_FILE_CONFLICT or
RPMPROB_FILE_CONFLICT, then remove the packages from the list of packages to
apply (because it's attempt-install), then try the same transaction again...

The problem is: if not all the packages have problems, then the packages
that didn't have problems installing in the first place will now get an
error: "package already installed"!!!
And since it's not one of the problems that are handled in pm.py, it will
just fail...

What do you think would be the best way to solve this: add a check for all
the codes linked to package already installed in the "attempt-only" check in
pm.py?
Or try to cancel the whole previous transaction - and then keep on
committing the new one?

I can generate a new patch if you want, after your guidance, or let you fix
it, like you prefer.
I do have an environment where I can reproduce this problem easily enough.

Herve

-----Original Message-----
From: openembedded-core-bounces@lists.openembedded.org
[mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
Klauer, Daniel
Sent: mardi 17 mai 2016 14:58
To: openembedded-core@lists.openembedded.org
Subject: [OE-core] [PATCH] smartpm: Don't ignore error if RPM transaction
fails without problems

SmartPM could misinterpret RPM transaction error as success, if ts.run()
(RPM Python API) returns an empty problems list.

This could happen for example if the RPM database is partially corrupted
such that the transaction does not have any problems like conflicts or
missing dependencies, but still can't be committed.

The added patch fixes the problem in the upstream sources; one of the
existing patches has to be adjusted to still apply.

Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
---
 .../python/python-smartpm/smart-attempt.patch      |  6 +--
 .../smart-rpm-transaction-failure-check.patch      | 57
++++++++++++++++++++++
 meta/recipes-devtools/python/python-smartpm_git.bb |  1 +
 3 files changed, 61 insertions(+), 3 deletions(-)  create mode 100644
meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-ch
eck.patch

diff --git a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
index ec98e03..5aedc88 100644
--- a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
+++ b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
@@ -36,7 +36,7 @@ index 9bbd952..ba6405a 100644
          finally:
              del getTS.ts
              cb.grabOutput(False)
-+            if probs and sysconf.has("attempt-install", soft=True):
++            if (probs is not None) and sysconf.has("attempt-install",
soft=True):
 +                def remove_conflict(pkgNEVR):
 +                    for key in changeset.keys():
 +                        if pkgNEVR == str(key):
@@ -67,8 +67,8 @@ index 9bbd952..ba6405a 100644
 +                        retry = 0
 +
              prog.setDone()
--            if probs:
-+            if probs and (not retry):
+-            if probs is not None:
++            if (probs is not None) and (not retry):
                  raise Error, "\n".join([x[0] for x in probs])
              prog.stop()
 +            if retry and len(changeset):
diff --git
a/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-
check.patch
b/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-failure-
check.patch
new file mode 100644
index 0000000..a740ddd
--- /dev/null
+++ b/meta/recipes-devtools/python/python-smartpm/smart-rpm-transaction-
+++ failure-check.patch
@@ -0,0 +1,57 @@
+From 0c55d7e18f40465e95e8e4bf22af01f5d4477d3c Mon Sep 17 00:00:00 2001
+From: Daniel Klauer <daniel.klauer@gin.de>
+Date: Wed, 11 May 2016 17:22:49 +0200
+Subject: [PATCH] rpm: Don't ignore transaction error with empty 
+problems list
+
+SmartPM could misinterpret RPM transaction error as success, if
+ts.run() (RPM Python API) returns an empty problems list, because of 
+incorrect check for None which treated empty list to be the same as 
+None when it has different meaning.
+
+ts.run() returns:
+* None in case of success
+* problems list in case of error, may be empty (look at rpmts_Run() in 
+rpm-5.4.14/python/rpmts-py.c [1])
+
+"if mylist" is not good enough to check for error here, because it will 
+treat an empty list as "false" because its len() == 0 [2].
+
+ts.check() seems to be different (it's ok for it to return an empty 
+list), but for consistency it should be made clear that it can return 
+either None, an empty list or a non-empty list.
+
+[1] http://rpm5.org/cvs/fileview?f=rpm/python/rpmts-py.c&v=1.111.2.3
+[2] https://docs.python.org/2/library/stdtypes.html#truth-value-testing
+
+Upstream-Status: Pending
+
+Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
+---
+ smart/backends/rpm/pm.py | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/smart/backends/rpm/pm.py b/smart/backends/rpm/pm.py index
+9bbd952..635f726 100644
+--- a/smart/backends/rpm/pm.py
++++ b/smart/backends/rpm/pm.py
+@@ -208,7 +208,7 @@ class RPMPackageManager(PackageManager):
+         force = sysconf.get("rpm-force", False)
+         if not force:
+             probs = ts.check()
+-            if probs:
++            if (probs is not None) and (len(probs) != 0):
+                 problines = []
+                 for prob in probs:
+                     name1 = "%s-%s-%s" % prob[0] @@ -247,7 +247,7 @@ 
+class RPMPackageManager(PackageManager):
+             del getTS.ts
+             cb.grabOutput(False)
+             prog.setDone()
+-            if probs:
++            if probs is not None:
+                 raise Error, "\n".join([x[0] for x in probs])
+             prog.stop()
+
+--
+1.9.1
+
diff --git a/meta/recipes-devtools/python/python-smartpm_git.bb
b/meta/recipes-devtools/python/python-smartpm_git.bb
index d9a908d..0d7f88c 100644
--- a/meta/recipes-devtools/python/python-smartpm_git.bb
+++ b/meta/recipes-devtools/python/python-smartpm_git.bb
@@ -17,6 +17,7 @@ SRC_URI = "\
           file://smart-recommends.patch \
           file://smart-improve-error-reporting.patch \
           file://smart-channelsdir.patch \
+          file://smart-rpm-transaction-failure-check.patch \
           file://smart-attempt.patch \
           file://smart-attempt-fix.patch \
           file://smart-add-for-rpm-ignoresize-check.patch \
--
1.9.1

--
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core




=



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
  2016-06-08 12:52       ` Herve Jourdain
@ 2016-06-08 15:43         ` Klauer, Daniel
  2016-06-08 16:00           ` Herve Jourdain
  2016-06-08 16:05           ` Mark Hatle
  0 siblings, 2 replies; 15+ messages in thread
From: Klauer, Daniel @ 2016-06-08 15:43 UTC (permalink / raw)
  To: Herve Jourdain, openembedded-core

Hello,

> You also need to add another check just before raising the error, or you
> would end up getting an "unknown error" raised there.
> I basically replaced:
> -        if (probs is not None) and (not retry):
> +       if (probs is not None) and ((len(probs) != 0) or not
> sysconf.has("attempt-install", soft=True)) and (not retry):

Hmm, it sounds like the attempt mode wants to ignore installation failures
(empty problems list) like before the patch, which makes sense to me. Afterall,
attempt mode wants to try installation and ignore failures. So it seems good to
fix this regression too.

However, I wonder why it never ignored a non-empty problems list, which would
also trigger an error. Maybe that case just never happens in practice, because
it's always just file conflicts. Those trigger a retry, which prevents the error
from being raised.

> BUT reflecting on the whole scheme, I'm wondering how it will work in case
> of file found conflict, since the problem package gets removed from the
> list, but the list is committed again, with most packages already
> installed...
> I therefore wonder that there could be the same error that I got in the end,
> i.e failing with package already installed - which should not fail for
> attempt only.

Indeed, I'm curious about that too...

If you could put together the patch, that would be great and fine with me.

Thanks,
Daniel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
  2016-06-08 15:43         ` Klauer, Daniel
@ 2016-06-08 16:00           ` Herve Jourdain
  2016-06-08 16:05           ` Mark Hatle
  1 sibling, 0 replies; 15+ messages in thread
From: Herve Jourdain @ 2016-06-08 16:00 UTC (permalink / raw)
  To: 'Klauer, Daniel', openembedded-core

Hi Daniel,

OK, I'll send what I've done to fix this for me tomorrow - it's basically
what we have discussed here, modification of 2 lines.

Regarding the possibility that the whole scheme might fail because of
"package already installed" during a retry, I won't try anything yet,
because I'd like to understand why it was done this way to begin with -
there may be some good reasons that I just can't find, so I'd rather be on
the safe side.
This said, the way things are done would work if all packages would fail
installation, because then none would become "already installed". But since
I'm not sure what exact kind of use cases / tests are supposed to be working
or not here, I'd rather postpone "fixing" that part.

If anyone on the list knows the history on that part, though, I'd be
interested to learn the background on the design, and the rationale of the
implementation.

Herve

-----Original Message-----
From: Klauer, Daniel [mailto:Daniel.Klauer@gin.de] 
Sent: mercredi 8 juin 2016 17:43
To: Herve Jourdain <herve.jourdain@neuf.fr>;
openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] smartpm: Don't ignore error if RPM
transaction fails without problems

Hello,

> You also need to add another check just before raising the error, or 
> you would end up getting an "unknown error" raised there.
> I basically replaced:
> -        if (probs is not None) and (not retry):
> +       if (probs is not None) and ((len(probs) != 0) or not
> sysconf.has("attempt-install", soft=True)) and (not retry):

Hmm, it sounds like the attempt mode wants to ignore installation failures
(empty problems list) like before the patch, which makes sense to me.
Afterall, attempt mode wants to try installation and ignore failures. So it
seems good to fix this regression too.

However, I wonder why it never ignored a non-empty problems list, which
would also trigger an error. Maybe that case just never happens in practice,
because it's always just file conflicts. Those trigger a retry, which
prevents the error from being raised.

> BUT reflecting on the whole scheme, I'm wondering how it will work in 
> case of file found conflict, since the problem package gets removed 
> from the list, but the list is committed again, with most packages 
> already installed...
> I therefore wonder that there could be the same error that I got in 
> the end, i.e failing with package already installed - which should not 
> fail for attempt only.

Indeed, I'm curious about that too...

If you could put together the patch, that would be great and fine with me.

Thanks,
Daniel
=



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
  2016-06-08 15:43         ` Klauer, Daniel
  2016-06-08 16:00           ` Herve Jourdain
@ 2016-06-08 16:05           ` Mark Hatle
  2016-06-08 16:17             ` Herve Jourdain
  1 sibling, 1 reply; 15+ messages in thread
From: Mark Hatle @ 2016-06-08 16:05 UTC (permalink / raw)
  To: openembedded-core

On 6/8/16 10:43 AM, Klauer, Daniel wrote:
> Hello,
> 
>> You also need to add another check just before raising the error, or you
>> would end up getting an "unknown error" raised there.
>> I basically replaced:
>> -        if (probs is not None) and (not retry):
>> +       if (probs is not None) and ((len(probs) != 0) or not
>> sysconf.has("attempt-install", soft=True)) and (not retry):
> 
> Hmm, it sounds like the attempt mode wants to ignore installation failures
> (empty problems list) like before the patch, which makes sense to me. Afterall,
> attempt mode wants to try installation and ignore failures. So it seems good to
> fix this regression too.
> 
> However, I wonder why it never ignored a non-empty problems list, which would
> also trigger an error. Maybe that case just never happens in practice, because
> it's always just file conflicts. Those trigger a retry, which prevents the error
> from being raised.
> 
>> BUT reflecting on the whole scheme, I'm wondering how it will work in case
>> of file found conflict, since the problem package gets removed from the
>> list, but the list is committed again, with most packages already
>> installed...

File conflicts are discovered prior to the transaction being committed
(installation time).  Problems reported during installation are 'different' (and
generally do not happen).  I'm not sure if a pre/post install failure, bad
package (signature or otherwise) or whatever would do here.  I'm not sure it's
been tested.

(The items above can't generally happen based on the way the system is designed..)

--Mark

>> I therefore wonder that there could be the same error that I got in the end,
>> i.e failing with package already installed - which should not fail for
>> attempt only.
> 
> Indeed, I'm curious about that too...
> 
> If you could put together the patch, that would be great and fine with me.
> 
> Thanks,
> Daniel
> 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
  2016-06-08 16:05           ` Mark Hatle
@ 2016-06-08 16:17             ` Herve Jourdain
  2016-06-08 16:47               ` Mark Hatle
  0 siblings, 1 reply; 15+ messages in thread
From: Herve Jourdain @ 2016-06-08 16:17 UTC (permalink / raw)
  To: Mark Hatle; +Cc: openembedded-core

Hi Mark,

In my case, the issue was due to an empty problem list, which after Daniel's fix did trigger a retry, which failed because the packages were already installed.
But the code for a non-empty problem list does also trigger a retry, and I suppose it would run also in the same "package already installed" issue, if it were to be triggered - not sure though, since I never had that case yet.
Do you think we need to check and potentially fix that case? Or shall we remove the retry feature altogether instead?

Hervé

> Le 9 juin 2016 à 00:05, Mark Hatle <mark.hatle@windriver.com> a écrit :
> 
>> On 6/8/16 10:43 AM, Klauer, Daniel wrote:
>> Hello,
>> 
>>> You also need to add another check just before raising the error, or you
>>> would end up getting an "unknown error" raised there.
>>> I basically replaced:
>>> -        if (probs is not None) and (not retry):
>>> +       if (probs is not None) and ((len(probs) != 0) or not
>>> sysconf.has("attempt-install", soft=True)) and (not retry):
>> 
>> Hmm, it sounds like the attempt mode wants to ignore installation failures
>> (empty problems list) like before the patch, which makes sense to me. Afterall,
>> attempt mode wants to try installation and ignore failures. So it seems good to
>> fix this regression too.
>> 
>> However, I wonder why it never ignored a non-empty problems list, which would
>> also trigger an error. Maybe that case just never happens in practice, because
>> it's always just file conflicts. Those trigger a retry, which prevents the error
>> from being raised.
>> 
>>> BUT reflecting on the whole scheme, I'm wondering how it will work in case
>>> of file found conflict, since the problem package gets removed from the
>>> list, but the list is committed again, with most packages already
>>> installed...
> 
> File conflicts are discovered prior to the transaction being committed
> (installation time).  Problems reported during installation are 'different' (and
> generally do not happen).  I'm not sure if a pre/post install failure, bad
> package (signature or otherwise) or whatever would do here.  I'm not sure it's
> been tested.
> 
> (The items above can't generally happen based on the way the system is designed..)
> 
> --Mark
> 
>>> I therefore wonder that there could be the same error that I got in the end,
>>> i.e failing with package already installed - which should not fail for
>>> attempt only.
>> 
>> Indeed, I'm curious about that too...
>> 
>> If you could put together the patch, that would be great and fine with me.
>> 
>> Thanks,
>> Daniel
>> 
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
  2016-06-08 16:17             ` Herve Jourdain
@ 2016-06-08 16:47               ` Mark Hatle
  2016-06-09  7:47                 ` ***SPAM*** " Herve Jourdain
  2016-06-09  8:19                 ` Klauer, Daniel
  0 siblings, 2 replies; 15+ messages in thread
From: Mark Hatle @ 2016-06-08 16:47 UTC (permalink / raw)
  To: Herve Jourdain; +Cc: openembedded-core

On 6/8/16 11:17 AM, Herve Jourdain wrote:
> Hi Mark,
> 
> In my case, the issue was due to an empty problem list, which after Daniel's fix did trigger a retry, which failed because the packages were already installed.
> But the code for a non-empty problem list does also trigger a retry, and I suppose it would run also in the same "package already installed" issue, if it were to be triggered - not sure though, since I never had that case yet.
> Do you think we need to check and potentially fix that case? Or shall we remove the retry feature altogether instead?

We should definitely get the fix suggested here in place and resolve the
immediate issue.

I'm not sure if we can intentionally trigger the faults or not.  If we can, then
we can add a test case to the existing harness.

The normal way to test this is inject a pre/post install script fault -- but the
cross install process protects against that already.

I can't think of any way to trigger the fault in an automated fashion (cause a
failure during install vs during transaction config)

--Mark

> Hervé
> 
>> Le 9 juin 2016 à 00:05, Mark Hatle <mark.hatle@windriver.com> a écrit :
>>
>>> On 6/8/16 10:43 AM, Klauer, Daniel wrote:
>>> Hello,
>>>
>>>> You also need to add another check just before raising the error, or you
>>>> would end up getting an "unknown error" raised there.
>>>> I basically replaced:
>>>> -        if (probs is not None) and (not retry):
>>>> +       if (probs is not None) and ((len(probs) != 0) or not
>>>> sysconf.has("attempt-install", soft=True)) and (not retry):
>>>
>>> Hmm, it sounds like the attempt mode wants to ignore installation failures
>>> (empty problems list) like before the patch, which makes sense to me. Afterall,
>>> attempt mode wants to try installation and ignore failures. So it seems good to
>>> fix this regression too.
>>>
>>> However, I wonder why it never ignored a non-empty problems list, which would
>>> also trigger an error. Maybe that case just never happens in practice, because
>>> it's always just file conflicts. Those trigger a retry, which prevents the error
>>> from being raised.
>>>
>>>> BUT reflecting on the whole scheme, I'm wondering how it will work in case
>>>> of file found conflict, since the problem package gets removed from the
>>>> list, but the list is committed again, with most packages already
>>>> installed...
>>
>> File conflicts are discovered prior to the transaction being committed
>> (installation time).  Problems reported during installation are 'different' (and
>> generally do not happen).  I'm not sure if a pre/post install failure, bad
>> package (signature or otherwise) or whatever would do here.  I'm not sure it's
>> been tested.
>>
>> (The items above can't generally happen based on the way the system is designed..)
>>
>> --Mark
>>
>>>> I therefore wonder that there could be the same error that I got in the end,
>>>> i.e failing with package already installed - which should not fail for
>>>> attempt only.
>>>
>>> Indeed, I'm curious about that too...
>>>
>>> If you could put together the patch, that would be great and fine with me.
>>>
>>> Thanks,
>>> Daniel
>>>
>>
>> -- 
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: ***SPAM*** Re: [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
  2016-06-08 16:47               ` Mark Hatle
@ 2016-06-09  7:47                 ` Herve Jourdain
  2016-06-09  8:19                 ` Klauer, Daniel
  1 sibling, 0 replies; 15+ messages in thread
From: Herve Jourdain @ 2016-06-09  7:47 UTC (permalink / raw)
  To: 'Mark Hatle'; +Cc: openembedded-core

Hi Mark,

OK, I've just sent the patch for the immediate issue to the list.

For the remaining potential issue, I believe one easy way to fix it would be
to handle the RPMPROB_PKG_INSTALLED error in addition to the existing error
types, but without generating a retry (if attempting to install a package,
and the package is already installed, then it should be just fine)...
But without a way to test the fix, I'm not sure if we want to do that...

Herve

-----Original Message-----
From: Mark Hatle [mailto:mark.hatle@windriver.com] 
Sent: mercredi 8 juin 2016 18:47
To: Herve Jourdain <herve.jourdain@neuf.fr>
Cc: openembedded-core@lists.openembedded.org
Subject: ***SPAM*** Re: [OE-core] [PATCH] smartpm: Don't ignore error if RPM
transaction fails without problems

On 6/8/16 11:17 AM, Herve Jourdain wrote:
> Hi Mark,
> 
> In my case, the issue was due to an empty problem list, which after
Daniel's fix did trigger a retry, which failed because the packages were
already installed.
> But the code for a non-empty problem list does also trigger a retry, and I
suppose it would run also in the same "package already installed" issue, if
it were to be triggered - not sure though, since I never had that case yet.
> Do you think we need to check and potentially fix that case? Or shall we
remove the retry feature altogether instead?

We should definitely get the fix suggested here in place and resolve the
immediate issue.

I'm not sure if we can intentionally trigger the faults or not.  If we can,
then we can add a test case to the existing harness.

The normal way to test this is inject a pre/post install script fault -- but
the cross install process protects against that already.

I can't think of any way to trigger the fault in an automated fashion (cause
a failure during install vs during transaction config)

--Mark

> Hervé
> 
>> Le 9 juin 2016 à 00:05, Mark Hatle <mark.hatle@windriver.com> a écrit :
>>
>>> On 6/8/16 10:43 AM, Klauer, Daniel wrote:
>>> Hello,
>>>
>>>> You also need to add another check just before raising the error, 
>>>> or you would end up getting an "unknown error" raised there.
>>>> I basically replaced:
>>>> -        if (probs is not None) and (not retry):
>>>> +       if (probs is not None) and ((len(probs) != 0) or not
>>>> sysconf.has("attempt-install", soft=True)) and (not retry):
>>>
>>> Hmm, it sounds like the attempt mode wants to ignore installation 
>>> failures (empty problems list) like before the patch, which makes 
>>> sense to me. Afterall, attempt mode wants to try installation and 
>>> ignore failures. So it seems good to fix this regression too.
>>>
>>> However, I wonder why it never ignored a non-empty problems list, 
>>> which would also trigger an error. Maybe that case just never 
>>> happens in practice, because it's always just file conflicts. Those 
>>> trigger a retry, which prevents the error from being raised.
>>>
>>>> BUT reflecting on the whole scheme, I'm wondering how it will work 
>>>> in case of file found conflict, since the problem package gets 
>>>> removed from the list, but the list is committed again, with most 
>>>> packages already installed...
>>
>> File conflicts are discovered prior to the transaction being 
>> committed (installation time).  Problems reported during installation 
>> are 'different' (and generally do not happen).  I'm not sure if a 
>> pre/post install failure, bad package (signature or otherwise) or 
>> whatever would do here.  I'm not sure it's been tested.
>>
>> (The items above can't generally happen based on the way the system 
>> is designed..)
>>
>> --Mark
>>
>>>> I therefore wonder that there could be the same error that I got in 
>>>> the end, i.e failing with package already installed - which should 
>>>> not fail for attempt only.
>>>
>>> Indeed, I'm curious about that too...
>>>
>>> If you could put together the patch, that would be great and fine with
me.
>>>
>>> Thanks,
>>> Daniel
>>>
>>
>> --
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
  2016-06-08 16:47               ` Mark Hatle
  2016-06-09  7:47                 ` ***SPAM*** " Herve Jourdain
@ 2016-06-09  8:19                 ` Klauer, Daniel
  1 sibling, 0 replies; 15+ messages in thread
From: Klauer, Daniel @ 2016-06-09  8:19 UTC (permalink / raw)
  To: Mark Hatle, Herve Jourdain; +Cc: openembedded-core

Hello,

the case I encountered the original issue with was a partially broken RPM database, which allowed the "Computing transaction" to work, but failed at "Committing transaction" when installing a package.

To reproduce it, the RPM database can be broken intentionally:
# echo foo > /var/lib/rpm/Arch

Then try to install some package...
# smart install|reinstall|upgrade <package>


SmartPM output & exit code before the fix (commit 1dc5f5d5c844585eec114be9480e0e4d8e60d09c):


Loading cache...
...
Computing transaction...
...
Committing transaction...
Preparing...                    ######################################## [  0%]
   1:Installing <package>       ######################################## [100%]
Output from <package>-<version>@<arch>:
rpmdb: BDB0004 fop_read_meta: /var/lib/rpm/Arch: unexpected file type or format
error: cannot open Arch(1022) index: Invalid argument(22)
	DB: Berkeley DB 6.0.30: (January 23, 2014)

Saving cache...

# echo $?
0


And after the fix:


Loading cache...
...
Computing transaction...
...
Committing transaction...
Preparing...                    ######################################## [  0%]
   1:Installing <package>       ######################################## [100%]
Output from <package>-<version>@<arch>:
rpmdb: BDB0004 fop_read_meta: /var/lib/rpm/Arch: unexpected file type or format
error: cannot open Arch(1022) index: Invalid argument(22)
	DB: Berkeley DB 6.0.30: (January 23, 2014)
error: Unknown error

# echo $?
1


I've only ever tested this with breaking the Arch index database; perhaps the behaviour will be different when breaking another index, and most likely RPM will fail earlier when the main /var/lib/rpm/Packages database is broken. Either way, perhaps this can be used for a test case.

Regards,
Daniel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
  2016-05-17 10:47 ` Burton, Ross
@ 2016-05-25  9:55   ` Klauer, Daniel
  0 siblings, 0 replies; 15+ messages in thread
From: Klauer, Daniel @ 2016-05-25  9:55 UTC (permalink / raw)
  To: Burton, Ross; +Cc: poky

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

Hello,


thanks for the hint, I've sent this (and 2 more smartpm patches) to oe-core.


Best regards,

Daniel


________________________________
From: Burton, Ross <ross.burton@intel.com>
Sent: Tuesday, May 17, 2016 12:47
To: Klauer, Daniel
Cc: poky@yoctoproject.org
Subject: Re: [poky] [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems

Good catch but patches that are actually for openembedded-core should go to openembedded-core@lists.openembedded.org<mailto:openembedded-core@lists.openembedded.org>, can you resend it?

Cheers,
Ross

On 12 May 2016 at 09:01, Klauer, Daniel <Daniel.Klauer@gin.de<mailto:Daniel.Klauer@gin.de>> wrote:
SmartPM could misinterpret RPM transaction error as success,
if ts.run() (RPM Python API) returns an empty problems list.

This could happen for example if the RPM database is partially corrupted
such that the transaction does not have any problems like conflicts or
missing dependencies, but still can't be committed.

The added patch fixes the problem in the upstream sources;
one of the existing patches has to be adjusted to still apply.

Signed-off-by: Daniel Klauer <daniel.klauer@gin.de<mailto:daniel.klauer@gin.de>>
---
 ...gnore-transaction-error-with-empty-proble.patch | 57 ++++++++++++++++++++++
 .../python/python-smartpm/smart-attempt.patch      |  6 +--
 meta/recipes-devtools/python/python-smartpm_git.bb<http://python-smartpm_git.bb> |  1 +
 3 files changed, 61 insertions(+), 3 deletions(-)
 create mode 100644 meta/recipes-devtools/python/python-smartpm/0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch

diff --git a/meta/recipes-devtools/python/python-smartpm/0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch b/meta/recipes-devtools/python/python-smartpm/0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch
new file mode 100644
index 0000000..a740ddd
--- /dev/null
+++ b/meta/recipes-devtools/python/python-smartpm/0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch
@@ -0,0 +1,57 @@
+From 0c55d7e18f40465e95e8e4bf22af01f5d4477d3c Mon Sep 17 00:00:00 2001
+From: Daniel Klauer <daniel.klauer@gin.de<mailto:daniel.klauer@gin.de>>
+Date: Wed, 11 May 2016 17:22:49 +0200
+Subject: [PATCH] rpm: Don't ignore transaction error with empty problems list
+
+SmartPM could misinterpret RPM transaction error as success,
+if ts.run() (RPM Python API) returns an empty problems list,
+because of incorrect check for None which treated empty list
+to be the same as None when it has different meaning.
+
+ts.run() returns:
+* None in case of success
+* problems list in case of error, may be empty
+(look at rpmts_Run() in rpm-5.4.14/python/rpmts-py.c [1])
+
+"if mylist" is not good enough to check for error here, because it will
+treat an empty list as "false" because its len() == 0 [2].
+
+ts.check() seems to be different (it's ok for it to return an empty list),
+but for consistency it should be made clear that it can return either None,
+an empty list or a non-empty list.
+
+[1] http://rpm5.org/cvs/fileview?f=rpm/python/rpmts-py.c&v=1.111.2.3
+[2] https://docs.python.org/2/library/stdtypes.html#truth-value-testing
+
+Upstream-Status: Pending
+
+Signed-off-by: Daniel Klauer <daniel.klauer@gin.de<mailto:daniel.klauer@gin.de>>
+---
+ smart/backends/rpm/pm.py | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/smart/backends/rpm/pm.py b/smart/backends/rpm/pm.py
+index 9bbd952..635f726 100644
+--- a/smart/backends/rpm/pm.py
++++ b/smart/backends/rpm/pm.py
+@@ -208,7 +208,7 @@ class RPMPackageManager(PackageManager):
+         force = sysconf.get("rpm-force", False)
+         if not force:
+             probs = ts.check()
+-            if probs:
++            if (probs is not None) and (len(probs) != 0):
+                 problines = []
+                 for prob in probs:
+                     name1 = "%s-%s-%s" % prob[0]
+@@ -247,7 +247,7 @@ class RPMPackageManager(PackageManager):
+             del getTS.ts
+             cb.grabOutput(False)
+             prog.setDone()
+-            if probs:
++            if probs is not None:
+                 raise Error, "\n".join([x[0] for x in probs])
+             prog.stop()
+
+--
+1.9.1
+
diff --git a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
index ec98e03..5aedc88 100644
--- a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
+++ b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
@@ -36,7 +36,7 @@ index 9bbd952..ba6405a 100644
          finally:
              del getTS.ts
              cb.grabOutput(False)
-+            if probs and sysconf.has("attempt-install", soft=True):
++            if (probs is not None) and sysconf.has("attempt-install", soft=True):
 +                def remove_conflict(pkgNEVR):
 +                    for key in changeset.keys():
 +                        if pkgNEVR == str(key):
@@ -67,8 +67,8 @@ index 9bbd952..ba6405a 100644
 +                        retry = 0
 +
              prog.setDone()
--            if probs:
-+            if probs and (not retry):
+-            if probs is not None:
++            if (probs is not None) and (not retry):
                  raise Error, "\n".join([x[0] for x in probs])
              prog.stop()
 +            if retry and len(changeset):
diff --git a/meta/recipes-devtools/python/python-smartpm_git.bb<http://python-smartpm_git.bb> b/meta/recipes-devtools/python/python-smartpm_git.bb<http://python-smartpm_git.bb>
index d6c378b..95b7e09 100644
--- a/meta/recipes-devtools/python/python-smartpm_git.bb<http://python-smartpm_git.bb>
+++ b/meta/recipes-devtools/python/python-smartpm_git.bb<http://python-smartpm_git.bb>
@@ -17,6 +17,7 @@ SRC_URI = "\
           file://smart-recommends.patch \
           file://smart-improve-error-reporting.patch \
           file://smart-channelsdir.patch \
+          file://0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch \
           file://smart-attempt.patch \
           file://smart-attempt-fix.patch \
           file://smart-rpm4-fixes.patch \
--
1.9.1
--
_______________________________________________
poky mailing list
poky@yoctoproject.org<mailto:poky@yoctoproject.org>
https://lists.yoctoproject.org/listinfo/poky


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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
  2016-05-12  8:01 Klauer, Daniel
@ 2016-05-17 10:47 ` Burton, Ross
  2016-05-25  9:55   ` Klauer, Daniel
  0 siblings, 1 reply; 15+ messages in thread
From: Burton, Ross @ 2016-05-17 10:47 UTC (permalink / raw)
  To: Klauer, Daniel; +Cc: poky

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

Good catch but patches that are actually for openembedded-core should go to
openembedded-core@lists.openembedded.org, can you resend it?

Cheers,
Ross

On 12 May 2016 at 09:01, Klauer, Daniel <Daniel.Klauer@gin.de> wrote:

> SmartPM could misinterpret RPM transaction error as success,
> if ts.run() (RPM Python API) returns an empty problems list.
>
> This could happen for example if the RPM database is partially corrupted
> such that the transaction does not have any problems like conflicts or
> missing dependencies, but still can't be committed.
>
> The added patch fixes the problem in the upstream sources;
> one of the existing patches has to be adjusted to still apply.
>
> Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
> ---
>  ...gnore-transaction-error-with-empty-proble.patch | 57
> ++++++++++++++++++++++
>  .../python/python-smartpm/smart-attempt.patch      |  6 +--
>  meta/recipes-devtools/python/python-smartpm_git.bb |  1 +
>  3 files changed, 61 insertions(+), 3 deletions(-)
>  create mode 100644
> meta/recipes-devtools/python/python-smartpm/0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch
>
> diff --git
> a/meta/recipes-devtools/python/python-smartpm/0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch
> b/meta/recipes-devtools/python/python-smartpm/0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch
> new file mode 100644
> index 0000000..a740ddd
> --- /dev/null
> +++
> b/meta/recipes-devtools/python/python-smartpm/0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch
> @@ -0,0 +1,57 @@
> +From 0c55d7e18f40465e95e8e4bf22af01f5d4477d3c Mon Sep 17 00:00:00 2001
> +From: Daniel Klauer <daniel.klauer@gin.de>
> +Date: Wed, 11 May 2016 17:22:49 +0200
> +Subject: [PATCH] rpm: Don't ignore transaction error with empty problems
> list
> +
> +SmartPM could misinterpret RPM transaction error as success,
> +if ts.run() (RPM Python API) returns an empty problems list,
> +because of incorrect check for None which treated empty list
> +to be the same as None when it has different meaning.
> +
> +ts.run() returns:
> +* None in case of success
> +* problems list in case of error, may be empty
> +(look at rpmts_Run() in rpm-5.4.14/python/rpmts-py.c [1])
> +
> +"if mylist" is not good enough to check for error here, because it will
> +treat an empty list as "false" because its len() == 0 [2].
> +
> +ts.check() seems to be different (it's ok for it to return an empty list),
> +but for consistency it should be made clear that it can return either
> None,
> +an empty list or a non-empty list.
> +
> +[1] http://rpm5.org/cvs/fileview?f=rpm/python/rpmts-py.c&v=1.111.2.3
> +[2] https://docs.python.org/2/library/stdtypes.html#truth-value-testing
> +
> +Upstream-Status: Pending
> +
> +Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
> +---
> + smart/backends/rpm/pm.py | 4 ++--
> + 1 file changed, 2 insertions(+), 2 deletions(-)
> +
> +diff --git a/smart/backends/rpm/pm.py b/smart/backends/rpm/pm.py
> +index 9bbd952..635f726 100644
> +--- a/smart/backends/rpm/pm.py
> ++++ b/smart/backends/rpm/pm.py
> +@@ -208,7 +208,7 @@ class RPMPackageManager(PackageManager):
> +         force = sysconf.get("rpm-force", False)
> +         if not force:
> +             probs = ts.check()
> +-            if probs:
> ++            if (probs is not None) and (len(probs) != 0):
> +                 problines = []
> +                 for prob in probs:
> +                     name1 = "%s-%s-%s" % prob[0]
> +@@ -247,7 +247,7 @@ class RPMPackageManager(PackageManager):
> +             del getTS.ts
> +             cb.grabOutput(False)
> +             prog.setDone()
> +-            if probs:
> ++            if probs is not None:
> +                 raise Error, "\n".join([x[0] for x in probs])
> +             prog.stop()
> +
> +--
> +1.9.1
> +
> diff --git
> a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
> b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
> index ec98e03..5aedc88 100644
> --- a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
> +++ b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
> @@ -36,7 +36,7 @@ index 9bbd952..ba6405a 100644
>           finally:
>               del getTS.ts
>               cb.grabOutput(False)
> -+            if probs and sysconf.has("attempt-install", soft=True):
> ++            if (probs is not None) and sysconf.has("attempt-install",
> soft=True):
>  +                def remove_conflict(pkgNEVR):
>  +                    for key in changeset.keys():
>  +                        if pkgNEVR == str(key):
> @@ -67,8 +67,8 @@ index 9bbd952..ba6405a 100644
>  +                        retry = 0
>  +
>               prog.setDone()
> --            if probs:
> -+            if probs and (not retry):
> +-            if probs is not None:
> ++            if (probs is not None) and (not retry):
>                   raise Error, "\n".join([x[0] for x in probs])
>               prog.stop()
>  +            if retry and len(changeset):
> diff --git a/meta/recipes-devtools/python/python-smartpm_git.bb
> b/meta/recipes-devtools/python/python-smartpm_git.bb
> index d6c378b..95b7e09 100644
> --- a/meta/recipes-devtools/python/python-smartpm_git.bb
> +++ b/meta/recipes-devtools/python/python-smartpm_git.bb
> @@ -17,6 +17,7 @@ SRC_URI = "\
>            file://smart-recommends.patch \
>            file://smart-improve-error-reporting.patch \
>            file://smart-channelsdir.patch \
> +
> file://0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch \
>            file://smart-attempt.patch \
>            file://smart-attempt-fix.patch \
>            file://smart-rpm4-fixes.patch \
> --
> 1.9.1
> --
> _______________________________________________
> poky mailing list
> poky@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/poky
>

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems
@ 2016-05-12  8:01 Klauer, Daniel
  2016-05-17 10:47 ` Burton, Ross
  0 siblings, 1 reply; 15+ messages in thread
From: Klauer, Daniel @ 2016-05-12  8:01 UTC (permalink / raw)
  To: poky

SmartPM could misinterpret RPM transaction error as success,
if ts.run() (RPM Python API) returns an empty problems list.

This could happen for example if the RPM database is partially corrupted
such that the transaction does not have any problems like conflicts or
missing dependencies, but still can't be committed.

The added patch fixes the problem in the upstream sources;
one of the existing patches has to be adjusted to still apply.

Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
---
 ...gnore-transaction-error-with-empty-proble.patch | 57 ++++++++++++++++++++++
 .../python/python-smartpm/smart-attempt.patch      |  6 +--
 meta/recipes-devtools/python/python-smartpm_git.bb |  1 +
 3 files changed, 61 insertions(+), 3 deletions(-)
 create mode 100644 meta/recipes-devtools/python/python-smartpm/0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch

diff --git a/meta/recipes-devtools/python/python-smartpm/0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch b/meta/recipes-devtools/python/python-smartpm/0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch
new file mode 100644
index 0000000..a740ddd
--- /dev/null
+++ b/meta/recipes-devtools/python/python-smartpm/0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch
@@ -0,0 +1,57 @@
+From 0c55d7e18f40465e95e8e4bf22af01f5d4477d3c Mon Sep 17 00:00:00 2001
+From: Daniel Klauer <daniel.klauer@gin.de>
+Date: Wed, 11 May 2016 17:22:49 +0200
+Subject: [PATCH] rpm: Don't ignore transaction error with empty problems list
+
+SmartPM could misinterpret RPM transaction error as success,
+if ts.run() (RPM Python API) returns an empty problems list,
+because of incorrect check for None which treated empty list
+to be the same as None when it has different meaning.
+
+ts.run() returns:
+* None in case of success
+* problems list in case of error, may be empty
+(look at rpmts_Run() in rpm-5.4.14/python/rpmts-py.c [1])
+
+"if mylist" is not good enough to check for error here, because it will
+treat an empty list as "false" because its len() == 0 [2].
+
+ts.check() seems to be different (it's ok for it to return an empty list),
+but for consistency it should be made clear that it can return either None,
+an empty list or a non-empty list.
+
+[1] http://rpm5.org/cvs/fileview?f=rpm/python/rpmts-py.c&v=1.111.2.3
+[2] https://docs.python.org/2/library/stdtypes.html#truth-value-testing
+
+Upstream-Status: Pending
+
+Signed-off-by: Daniel Klauer <daniel.klauer@gin.de>
+---
+ smart/backends/rpm/pm.py | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/smart/backends/rpm/pm.py b/smart/backends/rpm/pm.py
+index 9bbd952..635f726 100644
+--- a/smart/backends/rpm/pm.py
++++ b/smart/backends/rpm/pm.py
+@@ -208,7 +208,7 @@ class RPMPackageManager(PackageManager):
+         force = sysconf.get("rpm-force", False)
+         if not force:
+             probs = ts.check()
+-            if probs:
++            if (probs is not None) and (len(probs) != 0):
+                 problines = []
+                 for prob in probs:
+                     name1 = "%s-%s-%s" % prob[0]
+@@ -247,7 +247,7 @@ class RPMPackageManager(PackageManager):
+             del getTS.ts
+             cb.grabOutput(False)
+             prog.setDone()
+-            if probs:
++            if probs is not None:
+                 raise Error, "\n".join([x[0] for x in probs])
+             prog.stop()
+ 
+-- 
+1.9.1
+
diff --git a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
index ec98e03..5aedc88 100644
--- a/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
+++ b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch
@@ -36,7 +36,7 @@ index 9bbd952..ba6405a 100644
          finally:
              del getTS.ts
              cb.grabOutput(False)
-+            if probs and sysconf.has("attempt-install", soft=True):
++            if (probs is not None) and sysconf.has("attempt-install", soft=True):
 +                def remove_conflict(pkgNEVR):
 +                    for key in changeset.keys():
 +                        if pkgNEVR == str(key):
@@ -67,8 +67,8 @@ index 9bbd952..ba6405a 100644
 +                        retry = 0
 +
              prog.setDone()
--            if probs:
-+            if probs and (not retry):
+-            if probs is not None:
++            if (probs is not None) and (not retry):
                  raise Error, "\n".join([x[0] for x in probs])
              prog.stop()
 +            if retry and len(changeset):
diff --git a/meta/recipes-devtools/python/python-smartpm_git.bb b/meta/recipes-devtools/python/python-smartpm_git.bb
index d6c378b..95b7e09 100644
--- a/meta/recipes-devtools/python/python-smartpm_git.bb
+++ b/meta/recipes-devtools/python/python-smartpm_git.bb
@@ -17,6 +17,7 @@ SRC_URI = "\
           file://smart-recommends.patch \
           file://smart-improve-error-reporting.patch \
           file://smart-channelsdir.patch \
+          file://0001-rpm-Don-t-ignore-transaction-error-with-empty-proble.patch \
           file://smart-attempt.patch \
           file://smart-attempt-fix.patch \
           file://smart-rpm4-fixes.patch \
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-06-09  8:44 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-17 12:54 smartpm: Don't ignore error if RPM transaction fails without problems Klauer, Daniel
2016-05-17 12:58 ` [PATCH] " Klauer, Daniel
2016-06-07 13:17   ` Herve Jourdain
2016-06-08 11:35     ` Klauer, Daniel
     [not found]     ` <1465381435600.39214@gin.de>
2016-06-08 12:52       ` Herve Jourdain
2016-06-08 15:43         ` Klauer, Daniel
2016-06-08 16:00           ` Herve Jourdain
2016-06-08 16:05           ` Mark Hatle
2016-06-08 16:17             ` Herve Jourdain
2016-06-08 16:47               ` Mark Hatle
2016-06-09  7:47                 ` ***SPAM*** " Herve Jourdain
2016-06-09  8:19                 ` Klauer, Daniel
  -- strict thread matches above, loose matches on Subject: below --
2016-05-12  8:01 Klauer, Daniel
2016-05-17 10:47 ` Burton, Ross
2016-05-25  9:55   ` Klauer, Daniel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.