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.

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

index ec98e03..5aedc88 100644
@@ -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):

index d9a908d..0d7f88c 100644
@@ -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

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.

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

index ec98e03..5aedc88 100644
@@ -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):

index d9a908d..0d7f88c 100644
@@ -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

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: [email protected] [mailto:[email protected]] On Behalf Of Klauer, Daniel
Sent: mardi 17 mai 2016 14:58
To: [email protected]
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.

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

b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch index ec98e03..5aedc88 100644

@@ -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 <[email protected]>
+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 <[email protected]>

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

b/meta/recipes-devtools/python/python-smartpm_git.bb index d9a908d..0d7f88c 100644

@@ -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
[email protected]
http://lists.openembedded.org/mailman/listinfo/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 <[email protected]>
Sent: Tuesday, June 7, 2016 15:17
To: Klauer, Daniel; [email protected]
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: [email protected] [mailto:[email protected]] On Behalf Of Klauer, Daniel
Sent: mardi 17 mai 2016 14:58
To: [email protected]
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.

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

b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch index ec98e03..5aedc88 100644

@@ -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 <[email protected]>
+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 <[email protected]>

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

b/meta/recipes-devtools/python/python-smartpm_git.bb index d9a908d..0d7f88c 100644

@@ -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
[email protected]
http://lists.openembedded.org/mailman/listinfo/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:[email protected]]
Sent: mercredi 8 juin 2016 12:24
To: Herve Jourdain <[email protected]>; [email protected]
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 <[email protected]>
Sent: Tuesday, June 7, 2016 15:17
To: Klauer, Daniel; [email protected]
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: [email protected] [mailto:[email protected]] On Behalf Of Klauer, Daniel
Sent: mardi 17 mai 2016 14:58
To: [email protected]
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.

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

b/meta/recipes-devtools/python/python-smartpm/smart-attempt.patch index ec98e03..5aedc88 100644

@@ -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 <[email protected]>
+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 <[email protected]>

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

b/meta/recipes-devtools/python/python-smartpm_git.bb index d9a908d..0d7f88c 100644

@@ -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
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

=

Hello,

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.

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

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

Thanks,
Daniel

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:[email protected]]
Sent: mercredi 8 juin 2016 17:43
To: Herve Jourdain <[email protected]>; [email protected]
Subject: Re: [OE-core] [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems

Hello,

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.

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

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

Thanks,
Daniel
=

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

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é

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

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:[email protected]]
Sent: mercredi 8 juin 2016 18:47
To: Herve Jourdain <[email protected]>
Cc: [email protected]
Subject: ***SPAM*** Re: [OE-core] [PATCH] smartpm: Don't ignore error if RPM transaction fails without problems

Daniel's fix did trigger a retry, which failed because the packages were already installed.

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.

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

me.

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