launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+476755@xxxxxxxxxxxxxxxxxx>]

We need some tests.

Diff comments:

> diff --git a/lib/lp/soyuz/enums.py b/lib/lp/soyuz/enums.py
> index be67db0..723539c 100644
> --- a/lib/lp/soyuz/enums.py
> +++ b/lib/lp/soyuz/enums.py
> @@ -297,6 +297,19 @@ class BinaryPackageFileType(DBEnumeratedType):
>          """,
>      )
>  
> +    # XXX ruiendyourlife 2024-11-19: This is particularly confusing for Crate

ruiendyourlife -> ruinedyourlife

XXX comments are usually there when we intend to fix things later on - I do not think this is possible in that case, so I would add just a regular comment.

Background: At least Colin back then regularly searched for XXX comments and fixed them later on.

> +    # packages since they don't have a formal package format specification -
> +    # they're just tarballs of source code. We still need entries in both enums
> +    # to maintain database consistency.
> +    CRATE = DBItem(
> +        10,
> +        """
> +        Crate Package
> +
> +        The "crate" binary package format for Rust.
> +        """,
> +    )
> +
>      GENERIC = DBItem(
>          9,
>          """
> @@ -394,6 +407,18 @@ class BinaryPackageFormat(DBEnumeratedType):
>          """,
>      )
>  
> +    # XXX ruinedyourlife 2024-11-19: As noted above, there's unclear
> +    # distinction between BinaryPackageFormat and BinaryPackageFileType.
> +    # CRATE was added for Rust crate packages, requiring entries in both enums.
> +    CRATE = DBItem(

Same as above.

> +        10,
> +        """
> +        Crate Package
> +
> +        The "crate" binary package format for Rust.
> +        """,
> +    )
> +
>      GENERIC = DBItem(
>          9,
>          """
> diff --git a/lib/lp/soyuz/model/archivejob.py b/lib/lp/soyuz/model/archivejob.py
> index 4aeb4c8..d0f126e 100644
> --- a/lib/lp/soyuz/model/archivejob.py
> +++ b/lib/lp/soyuz/model/archivejob.py
> @@ -617,6 +622,89 @@ class CIBuildUploadJob(ArchiveJobDerived):
>              )
>          return all_metadata
>  
> +    def _scanRustArchive(
> +        self, report: IRevisionStatusReport, paths: Iterable[Path]
> +    ) -> Dict[str, ArtifactMetadata]:
> +        """Scan a Rust build archive for .crate files.
> +
> +        The Rust build process produces a tar.xz archive possibly containing a
> +        whole lot of files, some of which may be .crate files.
> +        This scanner extracts that archive to get the .crate files within.
> +        """
> +        all_metadata = {}
> +        for path in paths:
> +            if not path.name.endswith(".tar.xz"):
> +                continue
> +
> +            logger.info("Found Rust build archive: %s", path.name)
> +            try:
> +                # Create a temporary directory for extraction
> +                with tempfile.TemporaryDirectory() as tmpdir:
> +                    # Extract the tar.xz
> +                    with tarfile.open(str(path), "r:xz") as tar:
> +                        tar.extractall(path=tmpdir)
> +
> +                    # Read metadata.yaml for name and version
> +                    metadata_path = Path(tmpdir) / "metadata.yaml"
> +                    if not metadata_path.exists():
> +                        logger.warning(
> +                            "No metadata.yaml found in %s", path.name
> +                        )
> +                        continue
> +
> +                    with open(metadata_path) as f:
> +                        try:
> +                            import yaml
> +
> +                            metadata = yaml.safe_load(f)
> +                            crate_name = metadata.get("name")
> +                            crate_version = metadata.get("version")
> +                            if not crate_name or not crate_version:
> +                                logger.warning(
> +                                    "metadata.yaml missing required fields: "
> +                                    "name and/or version"
> +                                )
> +                                continue
> +                        except Exception as e:
> +                            logger.warning(
> +                                "Failed to parse metadata.yaml from %s: %s",
> +                                metadata_path.name,
> +                                e,
> +                            )
> +                            continue
> +
> +                    # Look for .crate files in the extracted contents
> +                    for root, _, files in os.walk(tmpdir):

You could add a comment that
Python 3.12 adds pathlib.Path.walk()

Then we could replace the os import, and consistently use the Path library.

> +                        for filename in files:
> +                            if filename.endswith(".crate"):
> +                                crate_path = Path(root) / filename
> +
> +                                # Create a new artifact for the .crate file
> +                                with open(str(crate_path), "rb") as f:
> +                                    # Add the .crate file as an artifact
> +                                    report.attach(name=filename, data=f.read())
> +
> +                                logger.info("Found Rust crate: %s", filename)
> +                                all_metadata[filename] = (
> +                                    BinaryArtifactMetadata(
> +                                        format=BinaryPackageFormat.CRATE,
> +                                        name=crate_name,
> +                                        version=crate_version,
> +                                        summary="",
> +                                        description="",
> +                                        architecturespecific=False,
> +                                        homepage="",
> +                                    )
> +                                )
> +
> +            except Exception as e:
> +                logger.warning(
> +                    "Failed to process Rust archive %s: %s", path.name, e
> +                )
> +                continue
> +
> +        return all_metadata
> +
>      def _scanGeneric(
>          self, report: IRevisionStatusReport, paths: Iterable[Path]
>      ) -> Dict[str, ArtifactMetadata]:


-- 
https://code.launchpad.net/~ruinedyourlife/launchpad/+git/launchpad/+merge/476755
Your team Launchpad code reviewers is requested to review the proposed merge of ~ruinedyourlife/launchpad:artifactory-crate-publishing into launchpad:master.