Skip to content

Commit

Permalink
builder: calculate build-id before executing step
Browse files Browse the repository at this point in the history
Calculating the build-id after a step was executed has the unpleasant
effect that the job slot must be briefly relinquished. When running
multiple jobs in parallel, this will give the others an opportunity to
take over the job slot. In the worst case some other, long running task
is started. The original job is not finished, though, until the audit
was created.

When calculating the build-id before execution, this cannot happen. The
audit generation will run uninterrupted with the current job slot until
the end.
  • Loading branch information
jkloetzke committed Jan 15, 2024
1 parent e50bcda commit e20e250
Showing 1 changed file with 7 additions and 11 deletions.
18 changes: 7 additions & 11 deletions pym/bob/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -617,17 +617,12 @@ def __workspaceLock(self, step):
self.__workspaceLocks[path] = ret = asyncio.Lock()
return ret

async def _generateAudit(self, step, depth, resultHash, executed=True):
async def _generateAudit(self, step, depth, resultHash, buildId, executed=True):
auditPath = os.path.join(os.path.dirname(step.getWorkspacePath()), "audit.json.gz")
if os.path.lexists(auditPath): removePath(auditPath)
if not self.__audit:
return None

if step.isCheckoutStep():
buildId = resultHash
else:
buildId = await self._getBuildId(step, depth)

def auditOf(s):
return os.path.join(os.path.dirname(s.getWorkspacePath()), "audit.json.gz")

Expand Down Expand Up @@ -1020,7 +1015,8 @@ async def _cookStep(self, step, checkoutOnly, depth):
async with self.__workspaceLock(step):
if not self._wasAlreadyRun(step, checkoutOnly):
if not checkoutOnly:
await self._cookBuildStep(step, depth)
buildId = await self._getBuildId(step, depth)
await self._cookBuildStep(step, depth, buildId)
self._setAlreadyRun(step, False, checkoutOnly)
else:
assert step.isPackageStep()
Expand Down Expand Up @@ -1265,7 +1261,7 @@ async def _cookCheckoutStep(self, checkoutStep, depth):
# Generate audit trail before setResultHash() to force re-generation if
# the audit trail fails.
if checkoutHash != oldCheckoutHash or self.__force:
await self._generateAudit(checkoutStep, depth, checkoutHash, checkoutExecuted)
await self._generateAudit(checkoutStep, depth, checkoutHash, checkoutHash, checkoutExecuted)
BobState().setResultHash(prettySrcPath, checkoutHash)

# upload live build-id cache in case of fresh checkout
Expand All @@ -1288,7 +1284,7 @@ async def _cookCheckoutStep(self, checkoutStep, depth):
assert predicted, "Non-predicted incorrect Build-Id found!"
self.__handleChangedBuildId(checkoutStep, checkoutHash)

async def _cookBuildStep(self, buildStep, depth):
async def _cookBuildStep(self, buildStep, depth, buildBuildId):
# Add the execution path of the build step to the buildDigest to
# detect changes between sandbox and non-sandbox builds. This is
# necessary in any build mode. Include the actual directories of
Expand Down Expand Up @@ -1335,7 +1331,7 @@ async def _cookBuildStep(self, buildStep, depth):
# build it
await self._runShell(buildStep, "build", a, self.__cleanBuild)
buildHash = hashWorkspace(buildStep)
await self._generateAudit(buildStep, depth, buildHash)
await self._generateAudit(buildStep, depth, buildHash, buildBuildId)
BobState().setResultHash(prettyBuildPath, buildHash)
BobState().setVariantId(prettyBuildPath, buildDigest[0])
BobState().setInputHashes(prettyBuildPath, buildInputHashes)
Expand Down Expand Up @@ -1586,7 +1582,7 @@ async def _cookPackageStep(self, packageStep, depth, packageBuildId):
packageDigest = await self.__getIncrementalVariantId(packageStep)
workspaceChanged = True
self.__statistic.packagesBuilt += 1
audit = await self._generateAudit(packageStep, depth, packageHash)
audit = await self._generateAudit(packageStep, depth, packageHash, packageBuildId)

# Rehash directory if content was changed
if workspaceChanged:
Expand Down

0 comments on commit e20e250

Please sign in to comment.