launchpad-reviewers team mailing list archive

[Quentin Debhi <mp+480149@xxxxxxxxxxxxxxxxxx>]

Quentin Debhi has proposed merging ~ruinedyourlife/launchpad:fix-sourcecraft-build-output into launchpad:master.

Commit message:
Output archive files if no crate files are found

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ruinedyourlife/launchpad/+git/launchpad/+merge/480149
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ruinedyourlife/launchpad:fix-sourcecraft-build-output into launchpad:master.

diff --git a/lib/lp/archiveuploader/craftrecipeupload.py b/lib/lp/archiveuploader/craftrecipeupload.py
index 75ab430..152a469 100644
--- a/lib/lp/archiveuploader/craftrecipeupload.py
+++ b/lib/lp/archiveuploader/craftrecipeupload.py
@@ -53,51 +53,59 @@ class CraftRecipeUpload:
             raise UploadError("Build did not produce any craft files.")
 
         for craft_path in sorted(craft_paths):
-            # Extract and process .crate files from archive
-            self._process_rust_archive(build, str(craft_path))
+            # Check if archive contains .crate files
+            with tempfile.TemporaryDirectory() as tmpdir:
+                with tarfile.open(craft_path, "r:xz") as tar:
+                    tar.extractall(path=tmpdir)
+
+                # Look for .crate files and metadata.yaml
+                crate_files = list(Path(tmpdir).rglob("*.crate"))
+                metadata_path = Path(tmpdir) / "metadata.yaml"
+
+                if crate_files and metadata_path.exists():
+                    # If we found a crate file and metadata, upload it
+                    try:
+                        metadata = yaml.safe_load(metadata_path.read_text())
+                        crate_name = metadata.get("name")
+                        crate_version = metadata.get("version")
+                        self.logger.debug(
+                            "Found crate %s version %s",
+                            crate_name,
+                            crate_version,
+                        )
+                    except Exception as e:
+                        self.logger.warning(
+                            "Failed to parse metadata.yaml: %s", e
+                        )
+
+                    crate_path = crate_files[
+                        0
+                    ]  # Take the first (and should be only) crate file
+                    with open(crate_path, "rb") as file:
+                        libraryfile = self.librarian.create(
+                            os.path.basename(str(crate_path)),
+                            os.stat(crate_path).st_size,
+                            file,
+                            filenameToContentType(str(crate_path)),
+                            restricted=build.is_private,
+                        )
+                    build.addFile(libraryfile)
+                else:
+                    # If no crate file found, upload the original archive
+                    self.logger.debug(
+                        "No crate files found, uploading archive"
+                    )
+                    with open(craft_path, "rb") as file:
+                        libraryfile = self.librarian.create(
+                            os.path.basename(str(craft_path)),
+                            os.stat(craft_path).st_size,
+                            file,
+                            filenameToContentType(str(craft_path)),
+                            restricted=build.is_private,
+                        )
+                    build.addFile(libraryfile)
 
         # The master verifies the status to confirm successful upload.
         self.logger.debug("Updating %s" % build.title)
         build.updateStatus(BuildStatus.FULLYBUILT)
         self.logger.debug("Finished upload.")
-
-    def _process_rust_archive(self, build, archive_path):
-        """Process a .tar.xz archive that may contain .crate files."""
-        with tempfile.TemporaryDirectory() as tmpdir:
-            with tarfile.open(archive_path, "r:xz") as tar:
-                tar.extractall(path=tmpdir)
-
-            # Read metadata.yaml for crate info
-            metadata_path = Path(tmpdir) / "metadata.yaml"
-            if metadata_path.exists():
-                try:
-                    metadata = yaml.safe_load(metadata_path.read_text())
-                    # XXX: ruinedyourlife 2024-12-06
-                    # We will need this later to give the crate a name and
-                    # version to artifactory. This is a placeholder for now,
-                    # which will be changed when we find a way to send that
-                    # information to artifactory.
-                    _ = metadata.get("name")
-                    _ = metadata.get("version")
-                except Exception as e:
-                    self.logger.warning("Failed to parse metadata.yaml: %s", e)
-            else:
-                self.logger.debug(
-                    "No metadata.yaml found at %s", metadata_path
-                )
-
-            # Look for .crate files in extracted contents
-            for crate_path in Path(tmpdir).rglob("*.crate"):
-                self._process_crate_file(build, str(crate_path))
-
-    def _process_crate_file(self, build, crate_path):
-        """Process a single .crate file."""
-        with open(crate_path, "rb") as file:
-            libraryfile = self.librarian.create(
-                os.path.basename(crate_path),
-                os.stat(crate_path).st_size,
-                file,
-                filenameToContentType(crate_path),
-                restricted=build.is_private,
-            )
-        build.addFile(libraryfile)
diff --git a/lib/lp/archiveuploader/tests/test_craftrecipeupload.py b/lib/lp/archiveuploader/tests/test_craftrecipeupload.py
index c137c78..24ed0f0 100644
--- a/lib/lp/archiveuploader/tests/test_craftrecipeupload.py
+++ b/lib/lp/archiveuploader/tests/test_craftrecipeupload.py
@@ -73,45 +73,61 @@ class TestCraftRecipeUploads(TestUploadProcessorBase):
 
             return archive_path
 
-    def test_sets_build_and_state(self):
-        # The upload processor uploads files and sets the correct status.
-        self.assertFalse(self.build.verifySuccessfulUpload())
+    def _createArchiveWithoutCrate(self, upload_dir, filename="output.tar.xz"):
+        """Helper to create a tar.xz archive without crate files."""
+        archive_path = os.path.join(upload_dir, filename)
+        os.makedirs(os.path.dirname(archive_path), exist_ok=True)
+
+        with tarfile.open(archive_path, "w:xz") as tar:
+            # Add a dummy file
+            with tempfile.NamedTemporaryFile(mode="w") as tmp:
+                tmp.write("test content")
+                tmp.flush()
+                tar.add(tmp.name, arcname="test.txt")
+
+        return archive_path
+
+    def test_processes_crate_from_archive(self):
+        """Test that crates are properly extracted and processed
+        from archives."""
         upload_dir = os.path.join(
-            self.incoming_folder, "test", str(self.build.id), "ubuntu"
+            self.incoming_folder, "test", str(self.build.id)
         )
         os.makedirs(upload_dir, exist_ok=True)
-        self._createArchiveWithCrate(upload_dir)
+
+        # Create archive with specific crate name and version
+        crate_name = "test-crate"
+        crate_version = "0.2.0"
+        self._createArchiveWithCrate(upload_dir, crate_name, crate_version)
 
         handler = UploadHandler.forProcessor(
             self.uploadprocessor, self.incoming_folder, "test", self.build
         )
         result = handler.processCraftRecipe(self.log)
-        self.assertEqual(
-            UploadStatusEnum.ACCEPTED,
-            result,
-            "Craft upload failed\nGot: %s" % self.log.getLogBuffer(),
-        )
+
+        # Verify upload succeeded
+        self.assertEqual(UploadStatusEnum.ACCEPTED, result)
         self.assertEqual(BuildStatus.FULLYBUILT, self.build.status)
-        self.assertTrue(self.build.verifySuccessfulUpload())
 
-        # Verify that the crate file was properly extracted and stored
+        # Verify only the crate file was stored (not the archive)
         build = removeSecurityProxy(self.build)
         files = list(build.getFiles())
         self.assertEqual(1, len(files))
         stored_file = files[0][1]
-        self.assertTrue(stored_file.filename.endswith(".crate"))
+        expected_filename = f"{crate_name}-{crate_version}.crate"
+        self.assertEqual(expected_filename, stored_file.filename)
 
-    def test_processes_crate_from_archive(self):
-        """Test extracting/processing crates within .tar.xz archives."""
+    def test_uploads_archive_without_crate(self):
+        """Test that the original archive is uploaded when no crate
+        files exist."""
         upload_dir = os.path.join(
-            self.incoming_folder, "test", str(self.build.id), "ubuntu"
+            self.incoming_folder, "test", str(self.build.id)
         )
         os.makedirs(upload_dir, exist_ok=True)
 
-        # Create archive with specific crate name and version
-        crate_name = "test-crate"
-        crate_version = "0.2.0"
-        self._createArchiveWithCrate(upload_dir, crate_name, crate_version)
+        # Create archive without crate files
+        archive_name = "test-output.tar.xz"
+        self._createArchiveWithoutCrate(upload_dir, archive_name)
 
         handler = UploadHandler.forProcessor(
             self.uploadprocessor, self.incoming_folder, "test", self.build
@@ -122,26 +138,25 @@ class TestCraftRecipeUploads(TestUploadProcessorBase):
         self.assertEqual(UploadStatusEnum.ACCEPTED, result)
         self.assertEqual(BuildStatus.FULLYBUILT, self.build.status)
 
-        # Verify the crate file was properly stored
+        # Verify the original archive was stored
         build = removeSecurityProxy(self.build)
         files = list(build.getFiles())
         self.assertEqual(1, len(files))
         stored_file = files[0][1]
-        expected_filename = f"{crate_name}-{crate_version}.crate"
-        self.assertEqual(expected_filename, stored_file.filename)
+        self.assertEqual(archive_name, stored_file.filename)
 
     def test_requires_craft(self):
-        # The upload processor fails if the upload does not contain any
-        # .craft files.
-        self.assertFalse(self.build.verifySuccessfulUpload())
+        """Test that the upload fails if no .tar.xz files are found."""
         upload_dir = os.path.join(
-            self.incoming_folder, "test", str(self.build.id), "ubuntu"
+            self.incoming_folder, "test", str(self.build.id)
         )
         write_file(os.path.join(upload_dir, "foo_0_all.manifest"), b"manifest")
+
         handler = UploadHandler.forProcessor(
             self.uploadprocessor, self.incoming_folder, "test", self.build
         )
         result = handler.processCraftRecipe(self.log)
+
         self.assertEqual(UploadStatusEnum.REJECTED, result)
         self.assertIn(
             "ERROR Build did not produce any craft files.",