launchpad-reviewers team mailing list archive

[Quentin Debhi <mp+483332@xxxxxxxxxxxxxxxxxx>]

Quentin Debhi has proposed merging ~ruinedyourlife/launchpad:fix-distribution-source-package-craft-builds into launchpad:master.

Commit message:
Fix distribution source package for craft builds

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ruinedyourlife/launchpad/+git/launchpad/+merge/483332
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ruinedyourlife/launchpad:fix-distribution-source-package-craft-builds into launchpad:master.

diff --git a/lib/lp/crafts/model/craftrecipebuildbehaviour.py b/lib/lp/crafts/model/craftrecipebuildbehaviour.py
index dc75148..21207c7 100644
--- a/lib/lp/crafts/model/craftrecipebuildbehaviour.py
+++ b/lib/lp/crafts/model/craftrecipebuildbehaviour.py
@@ -122,27 +122,6 @@ class CraftRecipeBuildBehaviour(BuilderProxyMixin, BuildFarmJobBehaviourBase):
         build = self.build
         args: BuildArgs = yield super().extraBuildArgs(logger=logger)
 
-        if logger is not None:
-            logger.debug("Build recipe: %r", build.recipe)
-            logger.debug("Git ref: %r", build.recipe.git_ref)
-            if build.recipe.git_ref is not None:
-                logger.debug(
-                    "Git ref repository URL: %r",
-                    build.recipe.git_ref.repository_url,
-                )
-                logger.debug("Git repository: %r", build.recipe.git_repository)
-                logger.debug(
-                    "Git repository HTTPS URL: %r",
-                    build.recipe.git_repository.git_https_url,
-                )
-                logger.debug("Git path: %r", build.recipe.git_path)
-                logger.debug("Git ref name: %r", build.recipe.git_ref.name)
-                logger.debug(
-                    "Recipe information type: %r",
-                    build.recipe.information_type,
-                )
-                logger.debug("Is private: %r", build.is_private)
-
         yield self.startProxySession(
             args,
             use_fetch_service=build.recipe.use_fetch_service,
@@ -187,8 +166,14 @@ class CraftRecipeBuildBehaviour(BuilderProxyMixin, BuildFarmJobBehaviourBase):
                 )
             )
         args["private"] = build.is_private
-        if IDistributionSourcePackage.providedBy(
-            build.recipe.git_repository.target
+        args["environment_variables"] = {}
+        # If the git repository target is a DistributionSourcePackage,
+        # add the environment variables for that distribution
+        if (
+            build.recipe.git_repository is not None
+            and IDistributionSourcePackage.providedBy(
+                build.recipe.git_repository.target
+            )
         ):
             distribution_name = (
                 build.recipe.git_repository.target.distribution.name
@@ -196,6 +181,7 @@ class CraftRecipeBuildBehaviour(BuilderProxyMixin, BuildFarmJobBehaviourBase):
             args["environment_variables"] = self.build_environment_variables(
                 distribution_name
             )
+
         return args
 
     def verifySuccessfulBuild(self):
diff --git a/lib/lp/crafts/tests/test_craftrecipebuildbehaviour.py b/lib/lp/crafts/tests/test_craftrecipebuildbehaviour.py
index a64de30..a2a3d70 100644
--- a/lib/lp/crafts/tests/test_craftrecipebuildbehaviour.py
+++ b/lib/lp/crafts/tests/test_craftrecipebuildbehaviour.py
@@ -650,23 +650,10 @@ class TestAsyncCraftRecipeBuildBehaviour(
         with dbuser(config.builddmaster.dbuser):
             args = yield job.extraBuildArgs(logger=logger)
 
-        # Debug prints
-        print("\nDebug logs:")
-        print(logger.getLogBuffer())
-        print("\nArgs:", args)
-        if "git_repository" in args:
-            print("\nGit URL:", args["git_repository"])
-            parts = urlsplit(args["git_repository"])
-            print("URL parts:", parts)
-
         # Asserts that nothing here is a zope proxy, to avoid errors when
         # serializing it for XML-RPC call.
         self.assertHasNoZopeSecurityProxy(args)
 
-        # Print the log buffer for debugging
-        print("\nDebug logs:")
-        print(logger.getLogBuffer())
-
         # Add assertions similar to snap build test
         split_browse_root = urlsplit(config.codehosting.git_browse_root)
         self.assertThat(
@@ -712,9 +699,20 @@ class TestAsyncCraftRecipeBuildBehaviour(
             ),
         )
 
-        # Create build for a different distribution
+        # Create a distribution source package for SOSS
         distribution = self.factory.makeDistribution(name="distribution-123")
-        job = self.makeJob(distribution=distribution)
+        package = self.factory.makeDistributionSourcePackage(
+            distribution=distribution
+        )
+
+        # Create a git repository targeting that package
+        git_repository = self.factory.makeGitRepository(target=package)
+
+        # Create a git ref for that repository
+        [git_ref] = self.factory.makeGitRefs(repository=git_repository)
+
+        # Create a job using that git ref
+        job = self.makeJob(git_ref=git_ref)
 
         with dbuser(config.builddmaster.dbuser):
             args = yield job.extraBuildArgs()
@@ -741,9 +739,20 @@ class TestAsyncCraftRecipeBuildBehaviour(
             ),
         )
 
-        # Create build for SOSS distribution
+        # Create a distribution source package for SOSS
         distribution = self.factory.makeDistribution(name="soss")
-        job = self.makeJob(distribution=distribution)
+        package = self.factory.makeDistributionSourcePackage(
+            distribution=distribution
+        )
+
+        # Create a git repository targeting that package
+        git_repository = self.factory.makeGitRepository(target=package)
+
+        # Create a git ref for that repository
+        [git_ref] = self.factory.makeGitRefs(repository=git_repository)
+
+        # Create a job using that git ref
+        job = self.makeJob(git_ref=git_ref)
 
         with dbuser(config.builddmaster.dbuser):
             args = yield job.extraBuildArgs()
@@ -769,7 +778,12 @@ class TestAsyncCraftRecipeBuildBehaviour(
         variables should be included."""
         # Create build for SOSS distribution but don't configure any variables
         distribution = self.factory.makeDistribution(name="soss")
-        job = self.makeJob(distribution=distribution)
+        package = self.factory.makeDistributionSourcePackage(
+            distribution=distribution
+        )
+        git_repository = self.factory.makeGitRepository(target=package)
+        [git_ref] = self.factory.makeGitRefs(repository=git_repository)
+        job = self.makeJob(git_ref=git_ref)
 
         with dbuser(config.builddmaster.dbuser):
             args = yield job.extraBuildArgs()
@@ -798,7 +812,7 @@ class TestAsyncCraftRecipeBuildBehaviour(
             args = yield job.extraBuildArgs()
 
         # Verify no environment variables were included
-        self.assertNotIn("environment_variables", args)
+        self.assertEqual({}, args.get("environment_variables", {}))
 
 
 class TestAsyncCraftRecipeBuildBehaviourFetchService(