Merge pull request #575 from jkloetzke/fix-disabled-dep-subst

Fix disabled dependencies variable substitutions

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({

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", """\
@@ -491,6 +506,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", """\