From bacfca044a86367bc100ee513e1f3bc9f3156ad0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Tue, 9 Jan 2024 22:39:41 +0100 Subject: [PATCH 1/2] builder: checkout modifications trigger rebuild If there is a checkoutScript or some checkoutAssert's and the checkout workspace was changed since the last time, we must re-run the step. This fixes a problem on Jenkins builds where the checkoutScript modifies a git repository. This change will be squashed by the Jenkins git plugin every time the job runs. Now if the script was marked deterministic, Bob still has to run it again to re-apply any modifications. Another example are code generators that should probably be run again if the workspace has changed. In fact, if the script was enabled for checkout-only updates (checkoutUpdateIf), then such modifications would already trigger the re-run if the workspace was modified. --- pym/bob/builder.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/pym/bob/builder.py b/pym/bob/builder.py index 02792a95..282732b7 100644 --- a/pym/bob/builder.py +++ b/pym/bob/builder.py @@ -65,6 +65,17 @@ def hashWorkspace(step): return hashDirectory(step.getStoragePath(), os.path.join(os.path.dirname(step.getWorkspacePath()), "cache.bin")) +class HashOnce: + def __init__(self, step): + self.step = step + self.result = None + def hashWorkspace(self): + if self.result is None: + self.result = hashWorkspace(self.step) + return self.result + def invalidate(self): + self.result = None + CHECKOUT_STATE_VARIANT_ID = None # Key in checkout directory state for step variant-id CHECKOUT_STATE_BUILD_ONLY = 1 # Key for checkout state of build-only builds @@ -1104,9 +1115,10 @@ async def _cookCheckoutStep(self, checkoutStep, depth): checkoutState = checkoutStep.getScmDirectories().copy() checkoutState[CHECKOUT_STATE_VARIANT_ID] = (checkoutDigest, None) checkoutState[CHECKOUT_STATE_BUILD_ONLY] = checkoutBuildOnlyState(checkoutStep, checkoutInputHashes) + currentResultHash = HashOnce(checkoutStep) if self.__buildOnly and (BobState().getResultHash(prettySrcPath) is not None): inputChanged = checkoutBuildOnlyStateChanged(checkoutState, oldCheckoutState) - rehash = lambda: hashWorkspace(checkoutStep) + rehash = currentResultHash.hashWorkspace if checkoutStep.mayUpdate(inputChanged, BobState().getResultHash(prettySrcPath), rehash): if checkoutBuildOnlyStateCompatible(checkoutState, oldCheckoutState): with stepExec(checkoutStep, "UPDATE", @@ -1115,6 +1127,7 @@ async def _cookCheckoutStep(self, checkoutStep, depth): newCheckoutState = oldCheckoutState.copy() newCheckoutState[CHECKOUT_STATE_BUILD_ONLY] = checkoutState[CHECKOUT_STATE_BUILD_ONLY] BobState().setDirectoryState(prettySrcPath, newCheckoutState) + currentResultHash.invalidate() # force recalculation else: stepMessage(checkoutStep, "UPDATE", "WARNING: recipe changed - cannot update ({})" .format(prettySrcPath), WARNING) @@ -1138,10 +1151,13 @@ async def _cookCheckoutStep(self, checkoutStep, depth): # Do not use None here to distinguish it from a non-existent directory. oldCheckoutState[scmDir] = (False, scmSpec) + oldResultHash = BobState().getResultHash(prettySrcPath) if (self.__force or (not checkoutStep.isDeterministic()) or - (BobState().getResultHash(prettySrcPath) is None) or + (oldResultHash is None) or not compareDirectoryState(checkoutState, oldCheckoutState) or - (checkoutInputHashes != BobState().getInputHashes(prettySrcPath))): + (checkoutInputHashes != BobState().getInputHashes(prettySrcPath)) or + ( (checkoutStep.getMainScript() or checkoutStep.getPostRunCmds()) + and (oldResultHash != currentResultHash.hashWorkspace())) ): # Switch or move away old or changed source directories. In # case of nested SCMs the loop relies on the property of # checkoutsFromState() to return the directories in a top-down @@ -1223,6 +1239,7 @@ async def _cookCheckoutStep(self, checkoutStep, depth): await self._runShell(checkoutStep, "checkout", a) self.__statistic.checkouts += 1 checkoutExecuted = True + currentResultHash.invalidate() # force recalculation # reflect new checkout state BobState().setDirectoryState(prettySrcPath, checkoutState) BobState().setInputHashes(prettySrcPath, checkoutInputHashes) @@ -1233,7 +1250,7 @@ async def _cookCheckoutStep(self, checkoutStep, depth): # We always have to rehash the directory as the user might have # changed the source code manually. - checkoutHash = hashWorkspace(checkoutStep) + checkoutHash = currentResultHash.hashWorkspace() # Generate audit trail before setResultHash() to force re-generation if # the audit trail fails. From f04e34f5f4b9305bc6e19cb018b0de2a381571ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kl=C3=B6tzke?= Date: Tue, 9 Jan 2024 22:54:08 +0100 Subject: [PATCH 2/2] builder: print checkout reason Sometimes its hard to know why a checkout step was executed because there are a couple of possible reasons. Let the user know why exactly the checkout as triggered instead of guessing. --- pym/bob/builder.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/pym/bob/builder.py b/pym/bob/builder.py index 282732b7..14232b54 100644 --- a/pym/bob/builder.py +++ b/pym/bob/builder.py @@ -1152,12 +1152,20 @@ async def _cookCheckoutStep(self, checkoutStep, depth): oldCheckoutState[scmDir] = (False, scmSpec) oldResultHash = BobState().getResultHash(prettySrcPath) - if (self.__force or (not checkoutStep.isDeterministic()) or - (oldResultHash is None) or - not compareDirectoryState(checkoutState, oldCheckoutState) or - (checkoutInputHashes != BobState().getInputHashes(prettySrcPath)) or - ( (checkoutStep.getMainScript() or checkoutStep.getPostRunCmds()) - and (oldResultHash != currentResultHash.hashWorkspace())) ): + checkoutReason = None + if self.__force: + checkoutReason = "forced" + elif not checkoutStep.isDeterministic(): + checkoutReason = "indeterminsitic" + elif not compareDirectoryState(checkoutState, oldCheckoutState): + checkoutReason = "recipe changed" + elif (checkoutInputHashes != BobState().getInputHashes(prettySrcPath)): + checkoutReason = "input changed" + elif (checkoutStep.getMainScript() or checkoutStep.getPostRunCmds()) \ + and (oldResultHash != currentResultHash.hashWorkspace()): + checkoutReason = "workspace changed" + + if checkoutReason: # Switch or move away old or changed source directories. In # case of nested SCMs the loop relies on the property of # checkoutsFromState() to return the directories in a top-down @@ -1235,7 +1243,7 @@ async def _cookCheckoutStep(self, checkoutStep, depth): BobState().setResultHash(prettySrcPath, oldCheckoutHash) with stepExec(checkoutStep, "CHECKOUT", - "{} {}".format(prettySrcPath, overridesString)) as a: + "{} ({}) {}".format(prettySrcPath, checkoutReason, overridesString)) as a: await self._runShell(checkoutStep, "checkout", a) self.__statistic.checkouts += 1 checkoutExecuted = True