From 9e546cca2d0808e0aa5385612b39d717528cf708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 12 Jul 2024 21:53:40 +0200 Subject: [PATCH 1/5] input: move tool fingerprint mask into steps To calculate the variant-id, the applicable tools for each step must be known. The variant-id of a step is calculated already in its constructor. So far this worked because the list of used tools was static for each recipe. Now that we want to make tool usage conditional, we have to pass the tool mask directly into each step. --- pym/bob/input.py | 59 +++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/pym/bob/input.py b/pym/bob/input.py index 5c393721..48696345 100644 --- a/pym/bob/input.py +++ b/pym/bob/input.py @@ -1360,10 +1360,12 @@ def hasNetAccess(self): class CoreBuildStep(CoreStep): - __slots__ = [] + __slots__ = ["fingerprintMask"] - def __init__(self, corePackage, script=(None, None, None), digestEnv=Env(), env=Env(), args=[]): + def __init__(self, corePackage, script=(None, None, None), digestEnv=Env(), + env=Env(), args=[], fingerprintMask=0): isValid = script[1] is not None + self.fingerprintMask = fingerprintMask super().__init__(corePackage, isValid, True, digestEnv, env, args) def _getToolKeys(self): @@ -1393,19 +1395,6 @@ def getMainScript(self): def getDigestScript(self): return self.corePackage.recipe.buildDigestScript - @property - def fingerprintMask(self): - # Remove bits of all tools that are not used in buildStep - ret = self.corePackage.fingerprintMask - i = 3 - ourToolKeys = self.corePackage.recipe.toolDepBuild - packageToolKeys = self.corePackage.recipe.toolDepPackage - for t in sorted(packageToolKeys): - if t not in ourToolKeys: - ret &= ~i - i <<= 2 - return ret - class BuildStep(Step): def hasNetAccess(self): @@ -1414,10 +1403,12 @@ def hasNetAccess(self): class CorePackageStep(CoreStep): - __slots__ = [] + __slots__ = ["fingerprintMask"] - def __init__(self, corePackage, script=(None, None, None), digestEnv=Env(), env=Env(), args=[]): + def __init__(self, corePackage, script=(None, None, None), digestEnv=Env(), env=Env(), args=[], + fingerprintMask=0): isValid = script[1] is not None + self.fingerprintMask = fingerprintMask super().__init__(corePackage, isValid, True, digestEnv, env, args) def _getToolKeys(self): @@ -1447,10 +1438,6 @@ def getMainScript(self): def getDigestScript(self): return self.corePackage.recipe.packageDigestScript - @property - def fingerprintMask(self): - return self.corePackage.fingerprintMask - class PackageStep(Step): def isShared(self): @@ -1480,10 +1467,10 @@ def refDeref(self, stack, inputTools, inputSandbox, pathFormatter, cache=None): class CorePackage: __slots__ = ("recipe", "internalRef", "directDepSteps", "indirectDepSteps", "states", "tools", "sandbox", "checkoutStep", "buildStep", "packageStep", - "pkgId", "fingerprintMask", "metaEnv") + "pkgId", "metaEnv") def __init__(self, recipe, tools, diffTools, sandbox, diffSandbox, - directDepSteps, indirectDepSteps, states, pkgId, fingerprintMask, metaEnv): + directDepSteps, indirectDepSteps, states, pkgId, metaEnv): self.recipe = recipe self.tools = tools self.sandbox = sandbox @@ -1492,7 +1479,6 @@ def __init__(self, recipe, tools, diffTools, sandbox, diffSandbox, self.indirectDepSteps = indirectDepSteps self.states = states self.pkgId = pkgId - self.fingerprintMask = fingerprintMask self.metaEnv = metaEnv def refDeref(self, stack, inputTools, inputSandbox, pathFormatter): @@ -1509,16 +1495,17 @@ def createInvalidCoreCheckoutStep(self): ret = self.checkoutStep = CoreCheckoutStep(self) return ret - def createCoreBuildStep(self, script, digestEnv, env, args): - ret = self.buildStep = CoreBuildStep(self, script, digestEnv, env, args) + def createCoreBuildStep(self, script, digestEnv, env, args, fingerprintMask): + ret = self.buildStep = CoreBuildStep(self, script, digestEnv, env, args, + fingerprintMask) return ret def createInvalidCoreBuildStep(self, args): ret = self.buildStep = CoreBuildStep(self, args=args) return ret - def createCorePackageStep(self, script, digestEnv, env, args): - ret = self.packageStep = CorePackageStep(self, script, digestEnv, env, args) + def createCorePackageStep(self, script, digestEnv, env, args, fingerprintMask): + ret = self.packageStep = CorePackageStep(self, script, digestEnv, env, args, fingerprintMask) return ret def getCorePackageStep(self): @@ -2525,6 +2512,16 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None, if doFingerprint: doFingerprint |= doFingerprintMaybe + # Remove bits of all tools that are not used in buildStep. In case + # you're wondering: the checkout step is never fingerprinted because + # the sources are hashed. + doFingerprintBuild = doFingerprint + mask = 3 + for t in sorted(self.__toolDepPackage): + if t not in self.__toolDepBuild: + doFingerprintBuild &= ~mask + mask <<= 2 + # check that all tools that are required are available toolsDetached = tools.detach() if self.__recipeSet.getPolicy('noUndefinedTools') and \ @@ -2536,7 +2533,7 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None, # touchedTools = tools.touchedKeys() # diffTools = { n : t for n,t in diffTools.items() if n in touchedTools } p = CorePackage(self, toolsDetached, diffTools, sandbox, diffSandbox, - directPackages, indirectPackages, states, uidGen(), doFingerprint, metaEnv) + directPackages, indirectPackages, states, uidGen(), metaEnv) # optional checkout step if self.__checkout != (None, None, None) or self.__checkoutSCMs or self.__checkoutAsserts: @@ -2570,7 +2567,7 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None, buildEnv = ( env.prune(self.__buildVars | self.__buildVarsWeak) if self.__buildVarsWeak else buildDigestEnv ) buildCoreStep = p.createCoreBuildStep(self.__build, buildDigestEnv, buildEnv, - [CoreRef(srcCoreStep)] + results) + [CoreRef(srcCoreStep)] + results, doFingerprintBuild) else: buildCoreStep = p.createInvalidCoreBuildStep([CoreRef(srcCoreStep)] + results) @@ -2579,7 +2576,7 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None, packageEnv = ( env.prune(self.__packageVars | self.__packageVarsWeak) if self.__packageVarsWeak else packageDigestEnv ) packageCoreStep = p.createCorePackageStep(self.__package, packageDigestEnv, packageEnv, - [CoreRef(buildCoreStep)]) + [CoreRef(buildCoreStep)], doFingerprint) # provide environment provideEnv = {} From 5767a92178e3a575c44972bb368c5727c6ba3aa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 12 Jul 2024 22:12:14 +0200 Subject: [PATCH 2/5] input: make tool usage conditional Sometimes a recipe might only require a tool if certain conditions are met. So far, tools could only be used unconditionally. This adds a new syntax along the following lines: checkoutTools: - if: "${TEST_VAR:-}" name: graphics::package Fixes #572. --- pym/bob/cmds/show.py | 12 +- pym/bob/input.py | 221 +++++++++++++++++------------------ pym/bob/intermediate.py | 2 +- test/unit/test_input_step.py | 16 +-- 4 files changed, 118 insertions(+), 133 deletions(-) diff --git a/pym/bob/cmds/show.py b/pym/bob/cmds/show.py index 07cd5d5f..e68429c7 100644 --- a/pym/bob/cmds/show.py +++ b/pym/bob/cmds/show.py @@ -56,12 +56,12 @@ def dumpPackage(package): doc["checkoutTools"] = { name : "/".join(t.getStep().getPackage().getStack()) for name, t in checkoutStep.getTools().items() - if name in (recipe.toolDepCheckout - recipe.toolDepCheckoutWeak) + if name in (checkoutStep.toolDep - checkoutStep.toolDepWeak) } doc["checkoutToolsWeak"] = { name : "/".join(t.getStep().getPackage().getStack()) for name, t in checkoutStep.getTools().items() - if name in recipe.toolDepCheckoutWeak + if name in checkoutStep.toolDepWeak } doc["checkoutVars"] = { k : v for k, v in checkoutStep.getEnv().items() @@ -83,12 +83,12 @@ def dumpPackage(package): doc["buildTools"] = { name : "/".join(t.getStep().getPackage().getStack()) for name, t in buildStep.getTools().items() - if name in (recipe.toolDepBuild - recipe.toolDepBuildWeak) + if name in (buildStep.toolDep - buildStep.toolDepWeak) } doc["buildToolsWeak"] = { name : "/".join(t.getStep().getPackage().getStack()) for name, t in buildStep.getTools().items() - if name in recipe.toolDepBuildWeak + if name in buildStep.toolDepWeak } doc["buildVars"] = { k : v for k, v in buildStep.getEnv().items() @@ -104,12 +104,12 @@ def dumpPackage(package): doc["packageTools"] = { name : "/".join(t.getStep().getPackage().getStack()) for name, t in packageStep.getTools().items() - if name in (recipe.toolDepPackage - recipe.toolDepPackageWeak) + if name in (packageStep.toolDep - packageStep.toolDepWeak) } doc["packageToolsWeak"] = { name : "/".join(t.getStep().getPackage().getStack()) for name, t in packageStep.getTools().items() - if name in recipe.toolDepPackageWeak + if name in packageStep.toolDepWeak } doc["packageVars"] = { k : v for k, v in packageStep.getEnv().items() diff --git a/pym/bob/input.py b/pym/bob/input.py index 48696345..1434efbb 100644 --- a/pym/bob/input.py +++ b/pym/bob/input.py @@ -778,14 +778,17 @@ def getUser(self): class CoreStep(CoreItem): __slots__ = ( "corePackage", "digestEnv", "env", "args", "providedEnv", "providedTools", "providedDeps", "providedSandbox", - "variantId", "deterministic", "isValid" ) + "variantId", "deterministic", "isValid", "toolDep", "toolDepWeak" ) - def __init__(self, corePackage, isValid, deterministic, digestEnv, env, args): + def __init__(self, corePackage, isValid, deterministic, digestEnv, env, args, + toolDep, toolDepWeak): self.corePackage = corePackage self.isValid = isValid self.digestEnv = digestEnv.detach() self.env = env.detach() self.args = args + self.toolDep = toolDep + self.toolDepWeak = toolDepWeak self.deterministic = deterministic and all( arg.isDeterministic() for arg in self.getAllDepCoreSteps()) self.variantId = self.getDigest(lambda coreStep: coreStep.variantId) @@ -818,14 +821,6 @@ def getUpdateScript(self): def getLabel(self): raise NotImplementedError - def _getToolKeys(self): - """Return relevant tool names for this CoreStep.""" - raise NotImplementedError - - def _getToolKeysWeak(self): - """Return relevant weak tool names for this CoreStep.""" - raise NotImplementedError - def isDeterministic(self): return self.deterministic @@ -843,7 +838,7 @@ def isPackageStep(self): def getTools(self): if self.isValid: - toolKeys = self._getToolKeys() + toolKeys = self.toolDep return { name : tool for name,tool in self.corePackage.tools.items() if name in toolKeys } else: @@ -918,7 +913,7 @@ def getResultId(self): h.update(struct.pack(" Date: Fri, 12 Jul 2024 22:25:31 +0200 Subject: [PATCH 3/5] doc: document conditional tool usage --- doc/manual/configuration.rst | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/doc/manual/configuration.rst b/doc/manual/configuration.rst index ff474397..a3aced4d 100644 --- a/doc/manual/configuration.rst +++ b/doc/manual/configuration.rst @@ -613,7 +613,7 @@ Other than the above differences setup scripts are identical to {checkout,build,package}Tools ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Type: List of strings +Type: List of strings or tool dictionaries This is a list of tools that should be added to ``$PATH`` during the execution of the respective checkout/build/package script. A tool denotes a folder in an @@ -623,6 +623,23 @@ added to ``$LD_LIBRARY_PATH``. The order of tools in ``$PATH`` and separate set of executables so that the order of their inclusion does not matter. +In the simple form, a tool is only specified as simple string. This will use +the tool unconditionally:: + + checkoutTools: [foo, bar] + +If necessary, a tool can also be used conditionally. In this case the tool +is specified as a dictionary of the mandatory ``name`` and an optional ``if`` +condition:: + + checkoutTools: + - name: foo + if: "$CONDITION" + - bar + +The conditions will be checked with the final environment of a package, that is +after all dependencies of a recipe have been traversed. + A tool that is consumed in one step is also set in the following. This means a tool consumed through checkoutTools is also available during the build and package steps. Likewise a tool consumed by buildTools is available in the From 6955f6a53299e9c34bcb0897ebe09a4071abf05f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 12 Jul 2024 22:44:39 +0200 Subject: [PATCH 4/5] test: add unit test for conditional tools --- test/unit/test_input_recipeset.py | 48 +++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/test/unit/test_input_recipeset.py b/test/unit/test_input_recipeset.py index dc0bf64d..ca095720 100644 --- a/test/unit/test_input_recipeset.py +++ b/test/unit/test_input_recipeset.py @@ -62,9 +62,9 @@ def writeDefault(self, content, layer=[]): with open(os.path.join(path, "default.yaml"), "w") as f: f.write(yaml.dump(content)) - def generate(self, sandboxEnabled=False): + def generate(self, sandboxEnabled=False, env={}): recipes = RecipeSet() - recipes.parse() + recipes.parse(env) return recipes.generatePackages(lambda x,y: "unused", sandboxEnabled) @@ -1622,6 +1622,50 @@ def testStrongOverride(self): r2 = packages.walkPackagePath("r2").getPackageStep() self.assertNotEqual(r1.getVariantId(), r2.getVariantId()) +class TestConditionalTools(RecipesTmp, TestCase): + """Test behaviour of conditional tools""" + + def setUp(self): + super().setUp() + self.writeConfig({ + "bobMinimumVersion" : "0.24", + }) + self.writeRecipe("tool", """\ + multiPackage: + "1": + provideTools: + tool-foo: "." + packageScript: "foo" + "2": + provideTools: + tool-bar: "." + packageScript: "bar" + """) + + def testConditional(self): + self.writeRecipe("root", """\ + root: True + depends: + - name: tool-1 + use: [tools] + - name: tool-2 + use: [tools] + packageTools: + - tool-foo + - name: tool-bar + if: "${BAR:-}" + """) + + packages = self.generate() + r = packages.walkPackagePath("root").getPackageStep() + self.assertEqual(set(r.getTools().keys()), { "tool-foo" }, + "By default, 'tool-bar' is not used") + + packages = self.generate(env = { "BAR" : "1" }) + r = packages.walkPackagePath("root").getPackageStep() + self.assertEqual(set(r.getTools().keys()), { "tool-foo", "tool-bar" }, + "If enabled, 'tool-bar' is used") + class TestScmIgnoreUserPolicy(RecipesTmp, TestCase): """ Test behaviour of scmIgnoreUser policy""" From 52f44525548b3bf4a188ec5547efc471a1c3c947 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Fri, 12 Jul 2024 22:45:18 +0200 Subject: [PATCH 5/5] input: eliminate some unnecessary env copies --- pym/bob/input.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pym/bob/input.py b/pym/bob/input.py index 1434efbb..255f469c 100644 --- a/pym/bob/input.py +++ b/pym/bob/input.py @@ -2259,8 +2259,8 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None, "__tools" : tools }) env = inputEnv.derive() for i in self.__varSelf: - env = env.derive({ key : env.substitute(value, "environment::"+key) - for key, value in i.items() }) + env.update(( (key, env.substitute(value, "environment::"+key)) + for key, value in i.items() )) states = { n : s.copy() for (n,s) in inputStates.items() } # update plugin states @@ -2327,9 +2327,10 @@ def prepare(self, inputEnv, sandboxEnabled, inputStates, inputSandbox=None, k : depDiffTools.get(v, v) for k, v in dep.toolOverride.items() }) - thisDepEnv = thisDepEnv.derive( - { key : env.substitute(value, "depends["+recipe+"].environment["+key+"]") - for key, value in dep.envOverride.items() }) + if dep.envOverride: + thisDepEnv = thisDepEnv.derive( + { key : env.substitute(value, "depends["+recipe+"].environment["+key+"]") + for key, value in dep.envOverride.items() }) r = self.__recipeSet.getRecipe(recipe) try: