From 9d8ada7e12e0346d1608771e7eb2a2e0d01d3afd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Mon, 29 Dec 2025 18:37:17 +0100 Subject: [PATCH 1/5] input: track origin recipe of a dependency Previously the information which recipe/class/file declared the dependency originally was lost. --- pym/bob/input.py | 28 +++++++++++++++------------- test/unit/test_input_recipe.py | 16 ++++++++-------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/pym/bob/input.py b/pym/bob/input.py index 7ee3fab2..bd3eb86f 100644 --- a/pym/bob/input.py +++ b/pym/bob/input.py @@ -1977,10 +1977,11 @@ def result(self): class DepTracker: - __slots__ = ('item', 'isNew', 'usedResult') + __slots__ = ('item', 'isNew', 'usedResult', 'depEntry') - def __init__(self, item): + def __init__(self, item, depEntry): self.item = item + self.depEntry = depEntry self.isNew = True self.usedResult = False @@ -2084,9 +2085,10 @@ class Dependency(object): __slots__ = ('recipe', 'envOverride', 'provideGlobal', 'inherit', 'use', 'useEnv', 'useTools', 'useBuildResult', 'useDeps', 'useSandbox', 'condition', 'toolOverride', 'checkoutDep', - 'alias') + 'alias', 'origin') - def __init__(self, recipe, env, fwd, use, cond, tools, checkoutDep, inherit, alias): + def __init__(self, origin, recipe, env, fwd, use, cond, tools, checkoutDep, inherit, alias): + self.origin = origin self.recipe = recipe self.envOverride = env self.provideGlobal = fwd @@ -2103,9 +2105,9 @@ def __init__(self, recipe, env, fwd, use, cond, tools, checkoutDep, inherit, ali self.alias = alias @staticmethod - def __parseEntry(dep, env, fwd, use, cond, tools, checkoutDep, inherit): + def __parseEntry(origin, dep, env, fwd, use, cond, tools, checkoutDep, inherit): if isinstance(dep, str): - return [ Recipe.Dependency(dep, env, fwd, use, cond, tools, checkoutDep, + return [ Recipe.Dependency(origin, dep, env, fwd, use, cond, tools, checkoutDep, inherit, None) ] else: envOverride = dep.get("environment") @@ -2127,22 +2129,22 @@ def __parseEntry(dep, env, fwd, use, cond, tools, checkoutDep, inherit): if name: if "depends" in dep: raise ParseError("A dependency must not use 'name' and 'depends' at the same time!") - return [ Recipe.Dependency(name, env, fwd, use, cond, tools, + return [ Recipe.Dependency(origin, name, env, fwd, use, cond, tools, checkoutDep, inherit, dep.get("alias")) ] dependencies = dep.get("depends") if dependencies is None: raise ParseError("Either 'name' or 'depends' required for dependencies!") - return Recipe.Dependency.parseEntries(dependencies, env, fwd, + return Recipe.Dependency.parseEntries(origin, dependencies, env, fwd, use, cond, tools, checkoutDep, inherit) @staticmethod - def parseEntries(deps, env={}, fwd=False, use=["result", "deps"], + def parseEntries(origin, deps, env={}, fwd=False, use=["result", "deps"], cond=None, tools={}, checkoutDep=False, inherit=True): """Returns an iterator yielding all dependencies as flat list""" # return flattened list of dependencies return chain.from_iterable( - Recipe.Dependency.__parseEntry(dep, env, fwd, use, cond, tools, + Recipe.Dependency.__parseEntry(origin, dep, env, fwd, use, cond, tools, checkoutDep, inherit) for dep in deps ) @@ -2205,7 +2207,7 @@ def __init__(self, recipeSet, recipe, layer, sourceFile, baseDir, packageName, b self.__inherit = recipe.get("inherit", []) self.__anonBaseClass = anonBaseClass self.__defaultScriptLanguage = scriptLanguage - self.__deps = list(Recipe.Dependency.parseEntries(recipe.get("depends", []))) + self.__deps = list(Recipe.Dependency.parseEntries(self, recipe.get("depends", []))) self.__packageName = packageName self.__baseName = baseName self.__root = recipe.get("root") @@ -2628,7 +2630,7 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None, # A dependency should be named only once. Hence we can # optimistically create the DepTracker object. If the dependency is # named more than one we make sure that it is the same variant. - depTrack = thisDeps.setdefault(p.getName(), DepTracker(depRef)) + depTrack = thisDeps.setdefault(p.getName(), DepTracker(depRef, dep)) if depTrack.prime(): directPackages.append(depRef) elif depCoreStep.variantId != depTrack.item.refGetDestination().variantId: @@ -2704,7 +2706,7 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None, name = depCoreStep.corePackage.getName() depTrack = thisDeps.get(name) if depTrack is None: - thisDeps[name] = depTrack = DepTracker(depRef) + thisDeps[name] = depTrack = DepTracker(depRef, None) if depTrack.prime(): indirectPackages.append(depRef) diff --git a/test/unit/test_input_recipe.py b/test/unit/test_input_recipe.py index 9b7de720..034009ec 100644 --- a/test/unit/test_input_recipe.py +++ b/test/unit/test_input_recipe.py @@ -25,7 +25,7 @@ def cmpEntry(self, entry, name, env={}, fwd=False, use=["result", "deps"], def testSimpleList(self): deps = [ "a", "b" ] - res = list(Recipe.Dependency.parseEntries(deps)) + res = list(Recipe.Dependency.parseEntries(MagicMock(), deps)) self.assertEqual(len(res), 2) self.cmpEntry(res[0], "a") @@ -33,7 +33,7 @@ def testSimpleList(self): def testMixedList(self): deps = [ "a", { "name" : "b", "environment" : { "foo" : ("bar", None) }} ] - res = list(Recipe.Dependency.parseEntries(deps)) + res = list(Recipe.Dependency.parseEntries(MagicMock(), deps)) self.assertEqual(len(res), 2) self.cmpEntry(res[0], "a") @@ -47,7 +47,7 @@ def testNestedList(self): { "depends" : [ "c" ] } ]} ] - res = list(Recipe.Dependency.parseEntries(deps)) + res = list(Recipe.Dependency.parseEntries(MagicMock(), deps)) self.assertEqual(len(res), 3) self.cmpEntry(res[0], "a") @@ -70,7 +70,7 @@ def testNestedEnv(self): }, "e" ] - res = list(Recipe.Dependency.parseEntries(deps)) + res = list(Recipe.Dependency.parseEntries(MagicMock(), deps)) self.assertEqual(len(res), 5) self.cmpEntry(res[0], "a") @@ -95,7 +95,7 @@ def testNestedIf(self): }, "e" ] - res = list(Recipe.Dependency.parseEntries(deps)) + res = list(Recipe.Dependency.parseEntries(MagicMock(), deps)) self.assertEqual(len(res), 5) self.cmpEntry(res[0], "a") @@ -120,7 +120,7 @@ def testNestedUse(self): }, "e" ] - res = list(Recipe.Dependency.parseEntries(deps)) + res = list(Recipe.Dependency.parseEntries(MagicMock(), deps)) self.assertEqual(len(res), 5) self.cmpEntry(res[0], "a") @@ -145,7 +145,7 @@ def testNestedFwd(self): }, "e" ] - res = list(Recipe.Dependency.parseEntries(deps)) + res = list(Recipe.Dependency.parseEntries(MagicMock(), deps)) self.assertEqual(len(res), 5) self.cmpEntry(res[0], "a") @@ -174,7 +174,7 @@ def testNestedCheckoutDep(self): "checkoutDep" : True, } ] - res = list(Recipe.Dependency.parseEntries(deps)) + res = list(Recipe.Dependency.parseEntries(MagicMock(), deps)) self.assertEqual(len(res), 6) self.cmpEntry(res[0], "a") From 23c98b940ad0403df6ce78caca629e85400acb59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Mon, 29 Dec 2025 18:38:46 +0100 Subject: [PATCH 2/5] input: improve duplicate dependency error message If multiple classes or multiPackages are involved, it can be hard to see where a colliding dependency was declared. Now that we track the origin of a dependency, output the involved files. --- pym/bob/input.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pym/bob/input.py b/pym/bob/input.py index bd3eb86f..a1735595 100644 --- a/pym/bob/input.py +++ b/pym/bob/input.py @@ -2636,8 +2636,10 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None, elif depCoreStep.variantId != depTrack.item.refGetDestination().variantId: self.__raiseIncompatibleLocal(depCoreStep) else: + sources = " and ".join(set([dep.origin.getPrimarySource(), depTrack.depEntry.origin.getPrimarySource()])) raise ParseError("Duplicate dependency '{}'. Each dependency must only be named once!" - .format(p.getName())) + .format(p.getName()), + help=f"The dependencies were declared in {sources}.") # Remember dependency diffs before changing them origDepDiffTools = thisDepDiffTools From 5ee2aea66c9080f9e4a3658e6c01a5868faa002f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Mon, 29 Dec 2025 18:40:17 +0100 Subject: [PATCH 3/5] input: add hint if a dependency uses 'name' and 'depends' --- pym/bob/input.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pym/bob/input.py b/pym/bob/input.py index a1735595..729a2089 100644 --- a/pym/bob/input.py +++ b/pym/bob/input.py @@ -2128,7 +2128,8 @@ def __parseEntry(origin, dep, env, fwd, use, cond, tools, checkoutDep, inherit): name = dep.get("name") if name: if "depends" in dep: - raise ParseError("A dependency must not use 'name' and 'depends' at the same time!") + raise ParseError("A dependency must not use 'name' and 'depends' at the same time!", + help=f"The offending entries 'name' attribute is '{name}'") return [ Recipe.Dependency(origin, name, env, fwd, use, cond, tools, checkoutDep, inherit, dep.get("alias")) ] dependencies = dep.get("depends") From 7fd17ebf109d116e61a7ae6557588ff215675d37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Mon, 29 Dec 2025 18:44:58 +0100 Subject: [PATCH 4/5] error: move methods to proper class They make no sense on the generic error class. --- pym/bob/errors.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pym/bob/errors.py b/pym/bob/errors.py index cdff2a77..94960bf7 100644 --- a/pym/bob/errors.py +++ b/pym/bob/errors.py @@ -22,17 +22,14 @@ def __str__(self): ret = ret + "\n" + self.help return ret - def pushFrame(self, frame): - if not self.stack or (self.stack[0] != frame): - self.stack.insert(0, frame) - - def setStack(self, stack): - if not self.stack: self.stack = stack[:] - class ParseError(BobError): def __init__(self, slogan, *args, **kwargs): BobError.__init__(self, slogan, "Parse", "Processing stack", *args, **kwargs) + def pushFrame(self, frame): + if not self.stack or (self.stack[0] != frame): + self.stack.insert(0, frame) + def setPath(self, path): self.stackSlogan = "Offending file" self.stack = [path] @@ -41,6 +38,9 @@ class BuildError(BobError): def __init__(self, slogan, *args, **kwargs): BobError.__init__(self, slogan, "Build", "Failed package", *args, **kwargs) + def setStack(self, stack): + if not self.stack: self.stack = stack[:] + class MultiBobError(BobError): def __init__(self, others): From e276e650b04b77a3bcbd5a25b06898a58010d298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Mon, 29 Dec 2025 21:03:27 +0100 Subject: [PATCH 5/5] test: add tests for duplicate dependencies --- test/unit/test_input_recipe.py | 21 +++++++++++++++++++++ test/unit/test_input_recipeset.py | 26 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/test/unit/test_input_recipe.py b/test/unit/test_input_recipe.py index 034009ec..a189a0b6 100644 --- a/test/unit/test_input_recipe.py +++ b/test/unit/test_input_recipe.py @@ -184,6 +184,27 @@ def testNestedCheckoutDep(self): self.cmpEntry(res[4], "e") self.cmpEntry(res[5], "f", checkoutDep=True) + def testNameAndDepends(self): + """A dependency must not use 'name' and 'depends' at the same time""" + deps = [ + { + "name" : "a", + "depends" : [], + } + ] + with self.assertRaises(ParseError): + list(Recipe.Dependency.parseEntries(MagicMock(), deps)) + + def testNeitherNameNorDepends(self): + """A dependency must use 'name' or 'depends'""" + deps = [ + { + "if" : "a", + } + ] + with self.assertRaises(ParseError): + list(Recipe.Dependency.parseEntries(MagicMock(), deps)) + class RecipeCommon: diff --git a/test/unit/test_input_recipeset.py b/test/unit/test_input_recipeset.py index aa79bfdc..952b5694 100644 --- a/test/unit/test_input_recipeset.py +++ b/test/unit/test_input_recipeset.py @@ -877,6 +877,32 @@ def testPackageDepends(self): self.assertEqual(p.getPackageStep().getArguments()[2].getPackage().getName(), "lib2") + def testDuplicateDep(self): + """Dependencies must only be named once""" + self.writeRecipe("root", """\ + root: True + depends: [a, a] + """) + self.writeRecipe("a", "") + + packages = self.generate() + self.assertRaises(ParseError, packages.getRootPackage) + + def testDuplicateDepWithClass(self): + """Dependencies must only be named once""" + self.writeRecipe("root", """\ + root: True + inherit: [cls] + depends: [a] + """) + self.writeClass("cls", """\ + depends: [a] + """) + self.writeRecipe("a", "") + + packages = self.generate() + self.assertRaises(ParseError, packages.getRootPackage) + class TestDependencyEnv(RecipesTmp, TestCase): """Tests related to "environment" block in dependencies"""