From 772c9ebedff26fd482fbbe67e5d7d5629b484aae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= <jan@kloetzke.net>
Date: Sun, 7 Jul 2024 21:18:39 +0200
Subject: [PATCH 1/3] test: add unit test that disabled deps can be provided

---
 test/unit/test_input_recipeset.py | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/test/unit/test_input_recipeset.py b/test/unit/test_input_recipeset.py
index 24fa69ee..318d3c7d 100644
--- a/test/unit/test_input_recipeset.py
+++ b/test/unit/test_input_recipeset.py
@@ -491,6 +491,35 @@ def testIncompatible(self):
         packages = recipes.generatePackages(lambda x,y: "unused")
         self.assertRaises(ParseError, packages.getRootPackage)
 
+    def testProvideDisabled(self):
+        """Providing disabled dependencies is not considered an error."""
+        self.writeRecipe("root", """\
+            root: True
+            depends:
+                - intermediate
+            buildScript: "true"
+            packageScript: "true"
+            """)
+        self.writeRecipe("intermediate", """\
+            depends:
+                - if: "false"
+                  name: a
+                - if: "true"
+                  name: b
+            buildScript: "true"
+            packageScript: "true"
+            provideDeps: ["a", "b"]
+            """)
+        self.writeRecipe("a", "packageScript: a")
+        self.writeRecipe("b", "packageScript: b")
+        packages = self.generate()
+        #packages.walkPackagePath("root")
+        rootArgs = packages.walkPackagePath("root").getBuildStep().getArguments()
+        self.assertEqual(len(rootArgs), 3)
+        self.assertEqual(rootArgs[0].getPackage().getName(), "root")
+        self.assertEqual(rootArgs[1].getPackage().getName(), "intermediate")
+        self.assertEqual(rootArgs[2].getPackage().getName(), "b")
+
     def testCyclic(self):
         """Cyclic dependencies must be detected during parsing"""
         self.writeRecipe("a", """\

From 8ccdb4f77baa0160ca347b5799f4f1ab49de21cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= <jan@kloetzke.net>
Date: Sun, 7 Jul 2024 21:19:09 +0200
Subject: [PATCH 2/3] input: evaluate deps condition first

If a dependency is disabled, all other variable substitutions in this
dependency shall be skipped. That has been the case already, except the
dependency name itself. Unfortunately, just moving the condition check
to the front is not sufficient because it is allowed to name disabled
dependencies in provideDeps. So we do the name substitution anyway but
will ignore a failure if the dependency shall be skipped.
---
 pym/bob/input.py | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/pym/bob/input.py b/pym/bob/input.py
index 1eeb03cb..1658924a 100644
--- a/pym/bob/input.py
+++ b/pym/bob/input.py
@@ -2321,6 +2321,19 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None,
             env.setFunArgs({ "recipe" : self, "sandbox" : bool(sandbox) and sandboxEnabled,
                 "__tools" : tools })
 
+            skip = dep.condition and not all(env.evaluate(cond, "dependency "+dep.recipe)
+                                                          for cond in dep.condition)
+            # The dependency name is always substituted because we allow
+            # provideDeps to name a disabled dependency. But in case the
+            # substiturion fails, the error is silently ignored.
+            try:
+                recipe = env.substitute(dep.recipe, "dependency::"+dep.recipe)
+                resolvedDeps.append(recipe)
+            except ParseError:
+                if skip: continue
+                raise
+            if skip: continue
+
             thisDepEnv = depEnv
             thisDepTools = depTools
             thisDepDiffTools = depDiffTools
@@ -2336,12 +2349,6 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None,
                 # Clear sandbox, if any
                 thisDepDiffSandbox = None
 
-            recipe = env.substitute(dep.recipe, "dependency::"+dep.recipe)
-            resolvedDeps.append(recipe)
-
-            if dep.condition and not all(env.evaluate(cond, "dependency "+recipe)
-                                                      for cond in dep.condition): continue
-
             if dep.toolOverride:
                 try:
                     thisDepTools = thisDepTools.derive({

From 783d141f1e6059c6874d207c035c5e751d07f6a0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= <jan@kloetzke.net>
Date: Sun, 7 Jul 2024 21:27:27 +0200
Subject: [PATCH 3/3] test: add unit test for deps substitution order

---
 test/unit/test_input_recipeset.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/test/unit/test_input_recipeset.py b/test/unit/test_input_recipeset.py
index 318d3c7d..dc0bf64d 100644
--- a/test/unit/test_input_recipeset.py
+++ b/test/unit/test_input_recipeset.py
@@ -395,6 +395,21 @@ def testVariableDeps(self):
         p = packages.walkPackagePath("root/b-foo")
         self.assertEqual(p.getName(), "b-foo")
 
+    def testDepConditionCheckOrder(self):
+        """Dependency conditions are tested before any other substitutions."""
+        self.writeRecipe("root", """\
+            root: True
+            depends:
+                - if: "false"
+                  name: "$DOES_NOT_EXIST"
+                  environment:
+                    FOO: "$DOES_NOT_EXIST"
+            buildScript: "true"
+            packageScript: "true"
+            """)
+        packages = self.generate()
+        packages.getRootPackage() # Must not fail due to substitution error
+
     def testGlobProvideDeps(self):
         """Test globbing pattern in provideDeps"""
         self.writeRecipe("root", """\