launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/upload-file-conflict-bug-663562 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #663562 duplicate orig for "linux" package in hardy
  https://bugs.launchpad.net/bugs/663562


= Summary =
When the upload processor looking up files, make sure we don't just look up 
ones that are not marked as removed.

== Proposed fix ==
Currently the upload processor is only looking up file ancestry where the file 
has not been removed from disk, by virtue of using the SQL view 
SourcePackageFilePublishing.  This should not be allowed to happen because 
once a file is uploaded with certain contents, those contents should never 
change.

== Implementation details ==
The call to IDistribution.getFileByName is replaced with 
IArchive.getFileByName which does the right thing.  The context archive can be 
None, so that's taken care of too.

== Tests ==
bin/test -cvv test_ppauploadprocessor test_conflicting_deleted_orig_file
-- 
https://code.launchpad.net/~julian-edwards/launchpad/upload-file-conflict-bug-663562/+merge/42264
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/upload-file-conflict-bug-663562 into lp:launchpad.

=== modified file 'lib/lp/archiveuploader/dscfile.py'
--- lib/lp/archiveuploader/dscfile.py	2010-11-05 14:17:11 +0000
+++ lib/lp/archiveuploader/dscfile.py	2010-11-30 16:16:46 +0000
@@ -430,11 +430,12 @@
         else:
             archives = [self.policy.archive]
 
+        archives = [archive for archive in archives if archive is not None]
+
         library_file = None
         for archive in archives:
             try:
-                library_file = self.policy.distro.getFileByName(
-                    filename, source=True, binary=False, archive=archive)
+                library_file = archive.getFileByName(filename)
                 self.logger.debug(
                     "%s found in %s" % (filename, archive.displayname))
                 return library_file, archive

=== modified file 'lib/lp/archiveuploader/tests/test_ppauploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_ppauploadprocessor.py	2010-11-10 23:42:37 +0000
+++ lib/lp/archiveuploader/tests/test_ppauploadprocessor.py	2010-11-30 16:16:46 +0000
@@ -18,6 +18,7 @@
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.config import config
+from canonical.database.constants import UTC_NOW
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
 from canonical.launchpad.testing.fakepackager import FakePackager
@@ -1113,6 +1114,31 @@
             self.uploadprocessor.last_processed_upload.queue_root.status,
             PackageUploadStatus.DONE)
 
+    def test_conflicting_deleted_orig_file(self):
+        # Uploading a conflicting orig file should be disallowed even if
+        # the existing one was deleted from disk.
+        upload_dir = self.queueUpload("bar_1.0-1-ppa-orig", "~name16/ubuntu")
+        self.processUpload(self.uploadprocessor, upload_dir)
+        self.assertEqual(
+            self.uploadprocessor.last_processed_upload.queue_root.status,
+            PackageUploadStatus.DONE)
+
+        # Delete the published file.
+        [bar_src] = self.name16.archive.getPublishedSources(name="bar")
+        bar_src.requestDeletion(self.name16)
+        bar_src.dateremoved = UTC_NOW
+        self.layer.txn.commit()
+
+        # bar_1.0-3 contains an orig file of the same version with
+        # different contents than the one we previously uploaded.
+        upload_dir = self.queueUpload("bar_1.0-3", "~name16/ubuntu")
+        self.processUpload(self.uploadprocessor, upload_dir)
+        self.assertTrue(
+            self.uploadprocessor.last_processed_upload.is_rejected)
+        self.assertIn(
+            'File bar_1.0.orig.tar.gz already exists in ',
+            self.uploadprocessor.last_processed_upload.rejection_message)
+
     def test30QuiltMultipleReusedOrigs(self):
         """Official orig*.tar.* can be reused for PPA uploads.