Merge lp:~mvo/software-center/hopefully-fix-lp1008229 into lp:software-center

[Michael Vogt]

Trivial branch to merge some vars to ensure that they are defined, even if the file is not there.
From reviewing the code it seems like its the only way to trigger the bug in #1008229.
A possible test for this would be:
=== modified file 'tests/test_debfileapplication.py'
--- tests/test_debfileapplication.py 2012-05-30 18:39:55 +0000
+++ tests/test_debfileapplication.py 2012-07-02 14:53:20 +0000
@@ -32,6 +32,12 @@
     def setUp(self):
         self.db = get_test_db()

+ def test_regression_lp1008229(self):
+ debfileapplication = DebFileApplication(DEBFILE_PATH)
+ debfileapplication.request += "xxx"
+ debfiledetails = debfileapplication.get_details(self.db)
+ self.assertEqual(debfiledetails.version, None)
+
     def test_get_name(self):
         debfileapplication = DebFileApplication(DEBFILE_PATH)
         debfiledetails = debfileapplication.get_details(self.db)

but its really a hack as I couldn't find a better way to trigger it.

[Michael Vogt (mvo)]

We should cherry pick this to 5.2 too

[Gary Lasker (gary-lasker)]

Yep, looking at the code it seems that this would only happen on an error opening the deb file, so this fix should take care of it. I wonder why so many people are hitting this though. Anyway, defining the variable early seems the right thing to do. Thanks mvo!

review: Approve

[Gary Lasker (gary-lasker)]

I merged this to trunk and to the 5.2 branch as well.

[Michael Vogt (mvo)]

On Wed, Jul 11, 2012 at 01:19:19AM -0000, Gary Lasker wrote:
> Review: Approve

Thanks for the review.

> Yep, looking at the code it seems that this would only happen on an
> error opening the deb file, so this fix should take care of it. I
> wonder why so many people are hitting this though.
[..]

Yes, its a bit puzzling as I did not find a way to reproduce it yet.

Cheers,
 MIchael

[Michael Vogt (mvo)]

On Wed, Jul 11, 2012 at 01:19:19AM -0000, Gary Lasker wrote:
> Review: Approve
>
> Yep, looking at the code it seems that this would only happen on an error opening the deb file, so this fix should take care of it. I wonder why so many people are hitting this though. Anyway, defining the variable early seems the right thing to do. Thanks mvo!

I just noticed that this got merged in trunk into the debian/changelog
for the already released 5.3.4 version changelog entry. As this
version is already released a new changelog entry for 5.3.5 needs to
get started in this case. Usually the best way to tell is if the
header contains UNRELEASED or not (or using rmadison software-center
to check, but that is usually not needed). I fixed this in trunk now.

Cheers,
 Michael

[Gary Lasker (gary-lasker)]

Oh, thank you for noticing that! Yes, I usually (every time before this) notice the UNRELEASED and if it's not there just do dch -i and add the UNRELEASED. I just missed it this time. Thanks again for finding it and taking care of it.

However, I did *not* know about rmadison. I usually use LP itself to get the versions, this is way easier! So thanks for that too!

[Gary Lasker (gary-lasker)]

Oh, and I also could not manage to figure out how to trigger this bug. I would love to know how people are hitting this. Hopefully, as per your branch title, this fix will take care of it. By inspection, it seems the correct fix and it has no downside. In any case, errors.ubuntu.com will (eventually) tell the tale. :)

=== modified file 'softwarecenter/db/debfile.py'
--- softwarecenter/db/debfile.py	2012-05-16 15:52:07 +0000
+++ softwarecenter/db/debfile.py	2012-07-02 14:56:19 +0000
@@ -69,6 +69,8 @@
             raise DebFileOpenError("AppDetailsDebFile: doc must be None.")
 
         self._error = None
+        self._deb = None
+
         # check errors before creating the DebPackage
         if not os.path.exists(self._app.request):
             self._error = _("Not found")
@@ -88,7 +90,6 @@
                 # but that is no longer the case with the latest apt
                 self._deb = DebPackage(self._app.request, self._cache._cache)
         except:
-            self._deb = None
             # deb files which are corrupt
             self._error = _("Internal Error")
             self._error_not_found = utf8(_(u"The file \u201c%s\u201d "