Merge lp:~abentley/launchpad/process-upload-no-cwd into lp:launchpad

[Aaron Bentley]

= Summary =
Fix bug #595957: archive uploader tries to move the changelog to the current
working directory

== Proposed fix ==
Stop specifying where to create the temp directory.

== Pre-implementation notes ==
None

== Implementation details ==
In the process, I had to create a way to create DSCFiles. This meant using a
temp directory in the LaunchpadObjectFactory, which does not have addCleanup.
Therefore, I extracted a temp_dir context manager and useContext from
useTempDir.

== Tests ==
bin/test -t test_ReadOnlyCWD

== Demo and Q/A ==
None

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/archiveuploader/dscfile.py
  lib/lp/testing/factory.py
  lib/lp/testing/__init__.py
  lib/lp/archiveuploader/tests/test_dscfile.py

[Paul Hummer (rockstar)]

Looks good. Just so you know, I went through this and followed the code in my head. I'm not letting you off easy, you just write good code.

Also, I now have it on my TODO list to read up on contextlib.

review: Approve (code)

=== modified file 'lib/lp/archiveuploader/dscfile.py'
--- lib/lp/archiveuploader/dscfile.py	2010-03-23 14:00:34 +0000
+++ lib/lp/archiveuploader/dscfile.py	2010-06-28 20:53:25 +0000
@@ -483,7 +483,7 @@
             "Verifying uploaded source package by unpacking it.")
 
         # Get a temporary dir together.
-        tmpdir = tempfile.mkdtemp(dir=self.dirname)
+        tmpdir = tempfile.mkdtemp()
 
         # chdir into it
         cwd = os.getcwd()

=== modified file 'lib/lp/archiveuploader/tests/test_dscfile.py'
--- lib/lp/archiveuploader/tests/test_dscfile.py	2010-04-12 20:17:59 +0000
+++ lib/lp/archiveuploader/tests/test_dscfile.py	2010-06-28 20:53:25 +0000
@@ -8,10 +8,12 @@
 import os
 import unittest
 
-from lp.archiveuploader.dscfile import findAndMoveChangelog, findCopyright
+from canonical.testing.layers import LaunchpadZopelessLayer
+from lp.archiveuploader.dscfile import (
+    findAndMoveChangelog, findCopyright)
 from lp.archiveuploader.nascentuploadfile import UploadError
 from lp.archiveuploader.tests import mock_logger_quiet
-from lp.testing import TestCase
+from lp.testing import TestCase, TestCaseWithFactory
 
 
 class TestDscFile(TestCase):
@@ -114,5 +116,22 @@
             errors[0].args[0],
             "debian/changelog file too large, 10MiB max")
 
+
+class TestDscFileLibrarian(TestCaseWithFactory):
+    """Tests for DscFile that may use the Librarian."""
+
+    layer = LaunchpadZopelessLayer
+
+    def test_ReadOnlyCWD(self):
+        """Processing a file should work when cwd is read-only."""
+        tempdir = self.useTempDir()
+        dsc_file = self.factory.makeDscFile(tempdir)
+        os.chmod(tempdir, 0555)
+        try:
+            list(dsc_file.unpackAndCheckSource())
+        finally:
+            os.chmod(tempdir, 0755)
+
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2010-06-15 18:24:04 +0000
+++ lib/lp/testing/__init__.py	2010-06-28 20:53:25 +0000
@@ -253,11 +253,18 @@
         """
         return self.id()
 
+    def useContext(self, context):
+        """Use the supplied context in this test.
+
+        The context will be cleaned via addCleanup.
+        """
+        retval = context.__enter__()
+        self.addCleanup(context.__exit__, None, None, None)
+        return retval
+
     def makeTemporaryDirectory(self):
         """Create a temporary directory, and return its path."""
-        tempdir = tempfile.mkdtemp()
-        self.addCleanup(shutil.rmtree, tempdir)
-        return tempdir
+        return self.useContext(temp_dir())
 
     def assertProvides(self, obj, interface):
         """Assert 'obj' correctly provides 'interface'."""
@@ -433,6 +440,7 @@
         cwd = os.getcwd()
         os.chdir(tempdir)
         self.addCleanup(os.chdir, cwd)
+        return tempdir
 
     def _unfoldEmailHeader(self, header):
         """Unfold a multiline e-mail header."""
@@ -997,6 +1005,15 @@
         obj_url.replace('http://api.launchpad.dev/',
         str(launchpad._root_uri)))
 
+
+@contextmanager
+def temp_dir():
+    """Provide a temporary directory as a ContextManager."""
+    tempdir = tempfile.mkdtemp()
+    yield tempdir
+    shutil.rmtree(tempdir)
+
+
 def unlink_source_packages(product):
     """Remove all links between the product and source packages.
 

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-06-21 07:30:23 +0000
+++ lib/lp/testing/factory.py	2010-06-28 20:53:25 +0000
@@ -15,6 +15,7 @@
     'ObjectFactory',
     ]
 
+from contextlib import nested
 from datetime import datetime, timedelta
 from email.encoders import encode_base64
 from email.utils import make_msgid, formatdate
@@ -25,6 +26,7 @@
 import os.path
 from random import randint
 from StringIO import StringIO
+from textwrap import dedent
 from threading import local
 
 import pytz
@@ -55,10 +57,13 @@
 from canonical.launchpad.interfaces.temporaryblobstorage import (
     ITemporaryStorageManager)
 from canonical.launchpad.ftests._sqlobject import syncUpdate
+from canonical.launchpad.scripts.logger import QuietFakeLogger
 from canonical.launchpad.webapp.dbpolicy import MasterDatabasePolicy
 from canonical.launchpad.webapp.interfaces import (
     IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
 
+from lp.archiveuploader.dscfile import DSCFile
+from lp.archiveuploader.uploadpolicy import BuildDaemonUploadPolicy
 from lp.blueprints.interfaces.specification import (
     ISpecificationSet, SpecificationDefinitionStatus)
 from lp.blueprints.interfaces.sprint import ISprintSet
@@ -137,7 +142,7 @@
 from lp.soyuz.model.processor import ProcessorFamily, ProcessorFamilySet
 from lp.soyuz.model.publishing import SourcePackagePublishingHistory
 
-from lp.testing import run_with_login, time_counter, login, logout
+from lp.testing import run_with_login, time_counter, login, logout, temp_dir
 
 from lp.translations.interfaces.potemplate import IPOTemplateSet
 from lp.translations.interfaces.translationgroup import (
@@ -1846,6 +1851,47 @@
         store.add(bq)
         return bq
 
+    def makeDscFile(self, tempdir_path=None):
+        """Make a DscFile.
+
+        :param tempdir_path: Path to a temporary directory to use.  If not
+            supplied, a temp directory will be created.
+        """
+        filename = 'ed_0.2-20.dsc'
+        contexts = []
+        if tempdir_path is None:
+            contexts.append(temp_dir())
+        # Use nested so temp_dir is an optional context.
+        with nested(*contexts) as result:
+            if tempdir_path is None:
+                tempdir_path = result[0]
+            fullpath = os.path.join(tempdir_path, filename)
+            with open(fullpath, 'w') as dsc_file:
+                dsc_file.write(dedent("""\
+                Format: 1.0
+                Source: ed
+                Version: 0.2-20
+                Binary: ed
+                Maintainer: James Troup <[e-mail]>
+                Architecture: any
+                Standards-Version: 3.5.8.0
+                Build-Depends: dpatch
+                Files:
+                 ddd57463774cae9b50e70cd51221281b 185913 ed_0.2.orig.tar.gz
+                 f9e1e5f13725f581919e9bfd62272a05 8506 ed_0.2-20.diff.gz
+                """))
+            class Changes:
+                architectures = ['source']
+            logger = QuietFakeLogger()
+            policy = BuildDaemonUploadPolicy()
+            policy.distroseries = self.makeDistroSeries()
+            policy.archive = self.makeArchive()
+            policy.distro = policy.distroseries.distribution
+            dsc_file = DSCFile(fullpath, 'digest', 0, 'main/editors',
+                'priority', 'package', 'version', Changes, policy, logger)
+            list(dsc_file.verify())
+        return dsc_file
+
     def makeTranslationTemplatesBuildJob(self, branch=None):
         """Make a new `TranslationTemplatesBuildJob`.