launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #32409
Re: [Merge] ~ruinedyourlife/launchpad:native-tools-publishing into launchpad:master
It mostly looks good to me, but I left some comments I think it is worth to review
Diff comments:
> diff --git a/lib/lp/archiveuploader/craftrecipeupload.py b/lib/lp/archiveuploader/craftrecipeupload.py
> index 7d62488..0596477 100644
> --- a/lib/lp/archiveuploader/craftrecipeupload.py
> +++ b/lib/lp/archiveuploader/craftrecipeupload.py
> @@ -81,12 +81,23 @@ class CraftRecipeUpload:
> with tarfile.open(craft_path, "r:xz") as tar:
> tar.extractall(path=tmpdir)
>
> - # Look for .crate files and metadata.yaml
> + # Look for .crate files, .jar files, pom.xml, and metadata.yaml
> crate_files = list(Path(tmpdir).rglob("*.crate"))
> + jar_files = list(Path(tmpdir).rglob("*.jar"))
> + pom_files = list(Path(tmpdir).rglob("pom.xml"))
> metadata_path = Path(tmpdir) / "metadata.yaml"
>
> + # Check for multiple artifact types
> + has_crate = bool(crate_files)
> + has_maven = bool(jar_files and pom_files)
> +
> + if has_crate and has_maven:
Maybe also worth to check that there is no more than 1 artifact in total per build. Below we are getting only the first element.
> + raise UploadError(
> + "Archive contains both Rust crate and Maven artifacts."
Adding a space will make it more readable:
"Archive contains both Rust crate and Maven artifacts. "
> + "Only one artifact type is allowed per build."
> + )
> +
> if crate_files and metadata_path.exists():
> - # If we found a crate file and metadata, upload those
> try:
> metadata = yaml.safe_load(metadata_path.read_text())
> crate_name = metadata.get("name")
> diff --git a/lib/lp/archiveuploader/tests/test_craftrecipeupload.py b/lib/lp/archiveuploader/tests/test_craftrecipeupload.py
> index 9796567..aaa48d8 100644
> --- a/lib/lp/archiveuploader/tests/test_craftrecipeupload.py
> +++ b/lib/lp/archiveuploader/tests/test_craftrecipeupload.py
> @@ -87,8 +89,50 @@ class TestCraftRecipeUploads(TestUploadProcessorBase):
>
> return archive_path
>
> - def test_processes_crate_from_archive(self):
> - """Test that crates are properly extracted and processed
> + def _createArchiveWithJarAndPom(
> + self, upload_dir, jar_name="test-jar", jar_version="0.1.0"
> + ):
> + """
> + Helper to create a tar.xz archive containing a JAR, POM, and metadata.
> + """
> + # Create a temporary directory to build our archive
> + with tempfile.TemporaryDirectory() as tmpdir:
> + # Create metadata.yaml
> + metadata = {
> + "name": jar_name,
> + "version": jar_version,
> + }
> + metadata_path = os.path.join(tmpdir, "metadata.yaml")
> + with open(metadata_path, "w") as f:
> + yaml.safe_dump(metadata, f)
> +
> + # Create dummy JAR file
> + jar_path = os.path.join(tmpdir, f"{jar_name}-{jar_version}.jar")
> + with open(jar_path, "wb") as f:
> + f.write(b"dummy jar contents")
> +
> + # Create dummy POM file
> + pom_content = f"""<project>
We can use string inside () concatenation to fit style
> + <modelVersion>4.0.0</modelVersion>
> + <groupId>com.example</groupId>
> + <artifactId>{jar_name}</artifactId>
> + <version>{jar_version}</version>
> +</project>"""
> + pom_path = os.path.join(tmpdir, "pom.xml")
> + with open(pom_path, "w") as f:
> + f.write(pom_content)
> +
> + # Create tar.xz archive
> + archive_path = os.path.join(upload_dir, "output.tar.xz")
> + with tarfile.open(archive_path, "w:xz") as tar:
> + tar.add(metadata_path, arcname="metadata.yaml")
> + tar.add(jar_path, arcname=os.path.basename(jar_path))
> + tar.add(pom_path, arcname="pom.xml")
> +
> + return archive_path
> +
> + def test_processes_artifact_from_archive(self):
> + """Test that artifacts are properly extracted and processed
> from archives."""
> upload_dir = os.path.join(
> self.incoming_folder, "test", str(self.build.id)
> diff --git a/lib/lp/crafts/model/craftrecipejob.py b/lib/lp/crafts/model/craftrecipejob.py
> index d573665..968d282 100644
> --- a/lib/lp/crafts/model/craftrecipejob.py
> +++ b/lib/lp/crafts/model/craftrecipejob.py
> @@ -337,3 +359,364 @@ class CraftRecipeRequestBuildsJob(CraftRecipeJobDerived):
> # are to this job's metadata and should be preserved.
> transaction.commit()
> raise
> +
> +
> +@implementer(ICraftPublishingJob)
> +@provider(ICraftPublishingJobSource)
> +class CraftPublishingJob(CraftRecipeJobDerived):
> + """
> + A Job that publishes craft recipe build artifacts to external
> + repositories.
> + """
> +
> + class_job_type = CraftRecipeJobType.PUBLISH_ARTIFACTS
> +
> + user_error_types = ()
> + retry_error_types = ()
> + max_retries = 5
> +
> + task_queue = "native_publisher_job"
> +
> + config = config.ICraftPublishingJobSource
> +
> + @classmethod
> + def create(cls, build):
> + """See `ICraftPublishingJobSource`."""
> + cls.metadata = {
Why are we using a class variable here?
> + "build_id": build.id,
> + }
> + recipe_job = CraftRecipeJob(
> + build.recipe, cls.class_job_type, cls.metadata
> + )
> + job = cls(recipe_job)
> + job.celeryRunOnCommit()
> + IStore(CraftRecipeJob).flush()
> + return job
> +
> + @property
> + def build_id(self):
> + """See `ICraftPublishingJob`."""
> + return self.metadata["build_id"]
> +
> + @cachedproperty
> + def build(self):
> + """See `ICraftPublishingJob`."""
> + return IStore(CraftRecipeBuild).get(CraftRecipeBuild, self.build_id)
> +
> + @property
> + def error_message(self):
> + """See `ICraftPublishingJob`."""
> + return self.metadata.get("error_message")
> +
> + @error_message.setter
> + def error_message(self, message):
> + """See `ICraftPublishingJob`."""
> + self.metadata["error_message"] = message
> +
> + def run(self):
> + """See `IRunnableJob`."""
> + try:
> + # Get the distribution name to access the correct configuration
> + distribution_name = None
> + git_repo = self.build.recipe.git_repository
> + if git_repo is not None:
> + if IDistributionSourcePackage.providedBy(git_repo.target):
> + distribution_name = git_repo.target.distribution.name
> +
> + if not distribution_name:
> + self.error_message = (
> + "Could not determine distribution for build"
> + )
> + raise Exception(self.error_message)
> +
> + # Get environment variables from configuration
> + try:
> + env_vars_json = config["craftbuild." + distribution_name][
> + "environment_variables"
> + ]
> + if env_vars_json and env_vars_json.lower() != "none":
> + env_vars = json.loads(env_vars_json)
> + # Replace auth placeholders
> + for key, value in env_vars.items():
> + if (
> + isinstance(value, str)
> + and "%(write_auth)s" in value
> + ):
> + env_vars[key] = value.replace(
> + "%(write_auth)s",
> + config.artifactory.write_credentials,
> + )
> + else:
> + env_vars = {}
> + except (NoSectionError, KeyError):
> + self.error_message = (
> + f"No configuration found for {distribution_name}"
> + )
> + raise Exception(self.error_message)
> +
> + # Check if we have a .crate file or .jar file
> + crate_file = None
> + jar_file = None
> + pom_file = None
> +
> + for _, lfa, _ in self.build.getFiles():
> + if lfa.filename.endswith(".crate"):
> + crate_file = lfa
> + elif lfa.filename.endswith(".jar"):
> + jar_file = lfa
> + elif lfa.filename == "pom.xml":
> + pom_file = lfa
> +
> + # Process the crate file
> + with tempfile.TemporaryDirectory() as tmpdir:
> + if crate_file is not None:
> + # Download the crate file
> + crate_path = os.path.join(tmpdir, crate_file.filename)
> + crate_file.open()
> + try:
> + with open(crate_path, "wb") as f:
> + f.write(crate_file.read())
> + finally:
> + crate_file.close()
> +
> + # Create a directory to extract the crate
> + crate_extract_dir = os.path.join(tmpdir, "crate_contents")
> + os.makedirs(crate_extract_dir, exist_ok=True)
> +
> + # Extract the .crate file using system tar command
> + result = subprocess.run(
> + ["tar", "-xf", crate_path, "-C", crate_extract_dir],
> + capture_output=True,
> + text=True,
> + )
> +
> + if result.returncode != 0:
> + raise Exception(
> + f"Failed to extract crate: {result.stderr}"
> + )
> +
> + # Find the extracted directory(should be the only one)
> + extracted_dirs = [
> + d
> + for d in os.listdir(crate_extract_dir)
> + if os.path.isdir(os.path.join(crate_extract_dir, d))
> + ]
> +
> + if not extracted_dirs:
> + raise Exception(
> + "No directory found in extracted crate"
> + )
> +
> + # Use the first directory as the crate directory
> + crate_dir = os.path.join(
> + crate_extract_dir, extracted_dirs[0]
> + )
> +
> + # Publish the Rust crate
> + self._publish_rust_crate(crate_dir, env_vars)
> + elif jar_file is not None and pom_file is not None:
> + # Download the jar file
> + jar_path = os.path.join(tmpdir, jar_file.filename)
> + jar_file.open()
> + try:
> + with open(jar_path, "wb") as f:
> + f.write(jar_file.read())
> + finally:
> + jar_file.close()
> +
> + # Download the pom file
> + pom_path = os.path.join(tmpdir, "pom.xml")
> + pom_file.open()
> + try:
> + with open(pom_path, "wb") as f:
> + f.write(pom_file.read())
> + finally:
> + pom_file.close()
> +
> + # Publish the Maven artifact
> + self._publish_maven_artifact(
> + tmpdir,
> + env_vars,
> + jar_path,
> + pom_path,
> + )
> +
> + else:
> + raise Exception("No publishable artifacts found in build")
> +
> + except Exception as e:
> + self.error_message = str(e)
> + # The normal job infrastructure will abort the transaction, but
> + # we want to commit instead: the only database changes we make
> + # are to this job's metadata and should be preserved.
> + transaction.commit()
> + raise
> +
> + def _publish_rust_crate(self, extract_dir, env_vars):
> + """Publish Rust crates from the extracted crate directory.
> +
> + :param extract_dir: Path to the extracted crate directory
> + :param env_vars: Environment variables from configuration
> + :raises: Exception if publishing fails
> + """
> + # Look for specific Cargo publishing repository configuration
> + cargo_publish_url = env_vars.get("CARGO_PUBLISH_URL")
> + cargo_publish_auth = env_vars.get("CARGO_PUBLISH_AUTH")
> +
> + if not cargo_publish_url or not cargo_publish_auth:
> + raise Exception(
> + "Missing Cargo publishing repository configuration"
> + )
> +
> + # Replace Cargo.toml with Cargo.toml.orig if it exists
> + cargo_toml_orig = os.path.join(extract_dir, "Cargo.toml.orig")
> + cargo_toml = os.path.join(extract_dir, "Cargo.toml")
> +
> + if os.path.exists(cargo_toml_orig):
> + import shutil
> +
> + shutil.move(cargo_toml_orig, cargo_toml)
> +
> + # Set up cargo config
> + cargo_dir = os.path.join(extract_dir, ".cargo")
> + os.makedirs(cargo_dir, exist_ok=True)
> +
> + # Create config.toml
> + with open(os.path.join(cargo_dir, "config.toml"), "w") as f:
> + f.write(
> + """
> +[registry]
> +global-credential-providers = ["cargo:token"]
> +
> +[registries.launchpad]
> +index = "{}"
> +""".format(
> + cargo_publish_url
> + )
> + )
> +
> + # Create credentials.toml
> + with open(os.path.join(cargo_dir, "credentials.toml"), "w") as f:
> + f.write(
> + """
> +[registries.launchpad]
> +token = "Bearer {}"
> +""".format(
> + cargo_publish_auth
> + )
> + )
> +
> + # Run cargo publish from the extracted directory
> + result = subprocess.run(
> + [
> + "cargo",
> + "publish",
> + "--no-verify",
> + "--allow-dirty",
> + "--registry",
> + "launchpad",
> + ],
> + capture_output=True,
> + cwd=extract_dir,
> + env={"CARGO_HOME": cargo_dir},
> + )
> +
> + if result.returncode != 0:
> + raise Exception(f"Failed to publish crate: {result.stderr}")
> +
> + def _publish_maven_artifact(
> + self, work_dir, env_vars, jar_path=None, pom_path=None
> + ):
> + """Publish Maven artifacts.
> +
> + :param work_dir: Working directory
> + :param env_vars: Environment variables from configuration
> + :param jar_path: Path to the JAR file
> + :param pom_path: Path to the pom.xml file
> + :raises: Exception if publishing fails
> + """
> + # Look for specific Maven publishing repository configuration
> + maven_publish_url = env_vars.get("MAVEN_PUBLISH_URL")
> + maven_publish_auth = env_vars.get("MAVEN_PUBLISH_AUTH")
> +
> + if not maven_publish_url or not maven_publish_auth:
> + raise Exception(
> + "Missing Maven publishing repository configuration"
> + )
> +
> + if jar_path is None or pom_path is None:
> + raise Exception("Missing JAR or POM file for Maven publishing")
> +
> + # Set up Maven settings
> + maven_dir = os.path.join(work_dir, ".m2")
> + os.makedirs(maven_dir, exist_ok=True)
> +
> + # Extract username and password from auth string
> + if ":" in maven_publish_auth:
> + username, password = maven_publish_auth.split(":", 1)
> + else:
> + username = "token"
> + password = maven_publish_auth
> +
> + # Generate settings.xml content
> + settings_xml = self._get_maven_settings_xml(username, password)
> +
> + with open(os.path.join(maven_dir, "settings.xml"), "w") as f:
> + f.write(settings_xml)
> +
> + # Run mvn deploy using the pom file
> + result = subprocess.run(
> + [
> + "mvn",
> + "deploy:deploy-file",
> + f"-DpomFile={pom_path}",
> + f"-Dfile={jar_path}",
> + "-DrepositoryId=central",
> + f"-Durl={maven_publish_url}",
> + "--settings={}".format(
> + os.path.join(maven_dir, "settings.xml")
> + ),
> + ],
> + capture_output=True,
> + cwd=work_dir,
> + )
> +
> + if result.returncode != 0:
> + raise Exception(
> + f"Failed to publish Maven artifact: {result.stderr}"
> + )
> +
> + def _get_maven_settings_xml(self, username, password):
> + """Generate Maven settings.xml content.
> +
> + :param username: Maven repository username
> + :param password: Maven repository password
> + :return: XML content as string
> + """
> + return f"""<?xml version="1.0" encoding="UTF-8"?>
> +<settings xmlns="http://maven.apache.org/SETTINGS/1.0.0";
> + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
> + xsi:schemaLocation="http://maven.apache.org/SETTINGS/1.0.0 \
> +http://maven.apache.org/xsd/settings-1.0.0.xsd";>
> + <servers>
> + <server>
> + <id>central</id>
> + <username>{username}</username>
> + <password>{password}</password>
> + </server>
> + </servers>
> + <profiles>
> + <profile>
> + <id>system</id>
> + <pluginRepositories>
> + <pluginRepository>
> + <id>central</id>
> + <url>file:///usr/share/maven-repo</url>
> + </pluginRepository>
> + </pluginRepositories>
> + </profile>
> + </profiles>
> + <activeProfiles>
> + <activeProfile>system</activeProfile>
> + </activeProfiles>
> +</settings>"""
--
https://code.launchpad.net/~ruinedyourlife/launchpad/+git/launchpad/+merge/483691
Your team Launchpad code reviewers is requested to review the proposed merge of ~ruinedyourlife/launchpad:native-tools-publishing into launchpad:master.
References
