Merge pull request #650 from jkloetzke/fix-gunzip-extract

Fix gunzip extract

Author: jkloetzke

pym/bob/scm/url.py

@@ -175,7 +175,14 @@ async def _extract(self, cmds, invoker, dirCreated, stdout=None):
         if dirCreated or isYounger(destination, canary):
             for cmd in cmds:
                 if shutil.which(cmd[0]) is None: continue
-                await invoker.checkCommand(cmd, cwd=self.dir, stdout=stdout)
+                if stdout:
+                    # Extracts to stdout. We must open the destination only
+                    # after we have confirmed that we run the extractor!
+                    # Otherwise we truncate the file.
+                    with open(stdout, 'wb') as f:
+                        await invoker.checkCommand(cmd, cwd=self.dir, stdout=f)
+                else:
+                    await invoker.checkCommand(cmd, cwd=self.dir, stdout=stdout)
                 invoker.trace("<touch>", canary)
                 with open(canary, "wb") as f:
                     pass
@@ -246,9 +253,8 @@ async def extract(self, invoker, dirCreated):
         else:
             raise BuildError("unkown suffix")
 
-        with open(dst, 'wb') as f:
-            cmd = [self.CMD, "-c", src]
-            await self._extract([cmd], invoker, dirCreated, f)
+        cmd = [self.CMD, "-c", src]
+        await self._extract([cmd], invoker, dirCreated, dst)
 
         shutil.copystat(src, dst)
 
@@ -527,7 +533,7 @@ def _download(self, url, destination, mode):
 
         return True, None
 
-    async def _fetch(self, invoker, url, workspaceFile, destination, mode):
+    async def _fetch(self, invoker, url, destination, mode):
         if url.scheme in ['', 'file']:
             # Verify that host name is empty or "localhost"
             if url.netloc not in ['', 'localhost']:
@@ -537,7 +543,7 @@ async def _fetch(self, invoker, url, workspaceFile, destination, mode):
                 if isYounger(url.path, destination):
                     if os.path.isdir(destination):
                         invoker.fail("Destination", destination, "is an existing directory!")
-                    invoker.trace("<cp>", url.path, workspaceFile)
+                    invoker.trace("<cp>", url.path, destination)
                     with tempfile.TemporaryDirectory(dir=os.path.dirname(destination)) as tmpDir:
                         tmpFile = os.path.join(tmpDir, self.__fn)
                         # Keep mtime when copying. Otherwise we would update
@@ -553,8 +559,7 @@ async def _fetch(self, invoker, url, workspaceFile, destination, mode):
         elif url.scheme in ["http", "https", "ftp"]:
             retries = self.__retries
             while True:
-                invoker.trace("<wget>", url.geturl(), ">",
-                        workspaceFile, "retires:", retries)
+                invoker.trace("<wget>", url.geturl(), ">", destination, "retires:", retries)
                 try:
                     updated, err = await invoker.runInExecutor(UrlScm._download, self, url, destination, mode)
                     if err:
@@ -613,7 +618,7 @@ def _upload(self, source, url):
 
         return None
 
-    async def _put(self, invoker, workspaceFile, source, url):
+    async def _put(self, invoker, source, url):
         if url.scheme in ['', 'file']:
             # Verify that host name is empty or "localhost"
             if url.netloc not in ['', 'localhost']:
@@ -624,7 +629,7 @@ async def _put(self, invoker, workspaceFile, source, url):
             if isYounger(source, url.path):
                 if os.path.isdir(url.path):
                     invoker.fail("Destination", url.path, "is an existing directory!")
-                invoker.trace("<cp>", workspaceFile, url.path)
+                invoker.trace("<cp>", source, url.path)
                 destDir = os.path.dirname(url.path)
                 os.makedirs(destDir, exist_ok=True)
                 with tempfile.TemporaryDirectory(dir=destDir) as tmpDir:
@@ -634,7 +639,7 @@ async def _put(self, invoker, workspaceFile, source, url):
         elif url.scheme in ["http", "https"]:
             retries = self.__retries
             while True:
-                invoker.trace("<wput>", workspaceFile, ">", url.geturl(), "retires:", retries)
+                invoker.trace("<wput>", source, ">", url.geturl(), "retires:", retries)
                 try:
                     err = await invoker.runInExecutor(UrlScm._upload, self, source, url)
                     if err:
@@ -689,7 +694,6 @@ async def invoke(self, invoker, workspaceCreated):
         if not os.path.isdir(invoker.joinPath(self.__dir)):
             os.makedirs(invoker.joinPath(self.__dir), exist_ok=True)
             workspaceCreated = True
-        workspaceFile = os.path.join(self.__dir, self.__fn)
         extractor = self.__getExtractor()
 
         destination = invoker.joinPath(self.__dir, self.__fn)
@@ -714,7 +718,7 @@ async def invoke(self, invoker, workspaceCreated):
                 if err:
                     # Output previously failed download attempt as warning
                     invoker.warn(err)
-                downloaded, err = await self._fetch(invoker, url, workspaceFile, destination, mode)
+                downloaded, err = await self._fetch(invoker, url, destination, mode)
                 if err is None:
                     break
             else:
@@ -725,17 +729,17 @@ async def invoke(self, invoker, workspaceCreated):
 
         # Always verify file hashes
         if self.__digestSha1:
-            invoker.trace("<sha1sum>", workspaceFile)
+            invoker.trace("<sha1sum>", destination)
             d = hashFile(destination, hashlib.sha1).hex()
             if d != self.__digestSha1:
                 invoker.fail("SHA1 digest did not match! expected:", self.__digestSha1, "got:", d)
         if self.__digestSha256:
-            invoker.trace("<sha256sum>", workspaceFile)
+            invoker.trace("<sha256sum>", destination)
             d = hashFile(destination, hashlib.sha256).hex()
             if d != self.__digestSha256:
                 invoker.fail("SHA256 digest did not match! expected:", self.__digestSha256, "got:", d)
         if self.__digestSha512:
-            invoker.trace("<sha512sum>", workspaceFile)
+            invoker.trace("<sha512sum>", destination)
             d = hashFile(destination, hashlib.sha512).hex()
             if d != self.__digestSha512:
                 invoker.fail("SHA512 digest did not match! expected:", self.__digestSha512, "got:", d)
@@ -745,7 +749,7 @@ async def invoke(self, invoker, workspaceCreated):
         if downloaded:
             for url, upload in urls:
                 if upload:
-                    await self._put(invoker, workspaceFile, destination, url)
+                    await self._put(invoker, destination, url)
 
         # Run optional extractors
         if extractor is not None:

test/unit/test_input_urlscm.py

@@ -341,7 +341,7 @@ def testNoResponse(self):
                 with self.assertRaises(InvocationError):
                     self.invokeScm(workspace, scm)
 
-class TestExtraction:
+class TestExtraction(TestCase):
 
     @classmethod
     def setUpClass(cls):
@@ -355,7 +355,7 @@ def setUpClass(cls):
             f.write("Hello world!")
 
         cls.tarGzFile = os.path.join(cls.dir, "test.tar.gz")
-        subprocess.run(["tar", "-zcf", cls.tarGzFile, src],
+        subprocess.run(["tar", "-C", cls.dir, "-zcf", "test.tar.gz", "src"],
             cwd=cls.dir, check=True)
         with open(cls.tarGzFile, "rb") as f:
             cls.tarGzDigestSha1 = hashlib.sha1(f.read()).digest().hex()
@@ -437,14 +437,43 @@ def testGz(self):
             self.assertExists(os.path.join(workspace, "test.txt.gz"))
             self.assertExists(os.path.join(workspace, "test.txt"))
 
+    def testGzDoubleInvocation(self):
+        """Invoking the SCM twice does not touch the extracted file"""
+        scm = self.createUrlScm({
+            "url" : self.gzFile,
+            "digestSHA256" : self.gzDigestSha256,
+        })
+        with TemporaryWorkspace() as workspace:
+            self.invokeScm(workspace, scm)
+
+            test_txt_gz = os.path.join(workspace, "test.txt.gz")
+            test_txt = os.path.join(workspace, "test.txt")
+            with open(test_txt_gz, "rb") as f:
+                self.assertEqual(self.gzDigestSha256,
+                                 hashlib.sha256(f.read()).digest().hex())
+            with open(test_txt, "r") as f:
+                self.assertEqual("Hello world!", f.read())
+            first_txt_gz_ts = os.stat(test_txt_gz).st_mtime_ns
+            first_txt_ts = os.stat(test_txt).st_mtime_ns
+
+            self.invokeScm(workspace, scm)
+            with open(test_txt_gz, "rb") as f:
+                self.assertEqual(self.gzDigestSha256,
+                                 hashlib.sha256(f.read()).digest().hex())
+            with open(test_txt, "r") as f:
+                self.assertEqual("Hello world!", f.read())
+            self.assertEqual(first_txt_gz_ts, os.stat(test_txt_gz).st_mtime_ns)
+            self.assertEqual(first_txt_ts, os.stat(test_txt).st_mtime_ns)
+
+
     def testGzStripComponentsNotSupported(self):
         scm = self.createUrlScm({
             "url" : self.gzFile,
             "digestSHA256" : self.gzDigestSha256,
             "stripComponents" : 1,
         })
         with TemporaryWorkspace() as workspace:
-            with self.assertRaises(InvocationError):
+            with self.assertRaises(BuildError):
                 self.invokeScm(workspace, scm)