feat(nix/buildkite): Check target map of parent to determine skips

This changes the logic for build pipeline generation to inspect
an (optional) parentTargetMap attribute which contains the derivation
map of a target commit.

Targets that existed in a parent commit with the same drv hash will be
skipped, as they are not considered to have changed.

This does not yet wire up any logic for retrieving the target map from
storage, meaning that at this commit all targets are always built.

The intention is that we will have logic to fetch the target
map (initially from Buildkite artefact storage), which we then pass to
the depot via externalArgs when actually generating the pipeline.

Change-Id: I3373c60aaf4b56b94c6ab64e2e5eef68dea9287c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/4946
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This commit is contained in:
Vincent Ambo 2022-01-16 15:55:36 +03:00 committed by tazjin
parent 0a21da2bb4
commit 0779f96687
3 changed files with 30 additions and 24 deletions

View file

@ -2,7 +2,9 @@
# (see //nix/readTree for details) and constructing a matching attribute set # (see //nix/readTree for details) and constructing a matching attribute set
# tree. # tree.
{ nixpkgsBisectPath ? null, nixpkgsConfig ? {}, ... }@args: { nixpkgsBisectPath ? null
, parentTargetMap ? null
, nixpkgsConfig ? {}, ... }@args:
let let
inherit (builtins) inherit (builtins)

View file

@ -16,6 +16,7 @@ let
filter filter
foldl' foldl'
getEnv getEnv
hasAttr
length length
listToAttrs listToAttrs
mapAttrs mapAttrs
@ -46,26 +47,24 @@ in rec {
then "${label}:${target.__subtarget}" then "${label}:${target.__subtarget}"
else label; else label;
# Skip build steps if their out path has already been built. # Determine whether to skip a target if it has not diverged from the
skip = headBranch: target: let # HEAD branch.
shouldSkip = shouldSkip = parentTargetMap: label: drvPath:
# Only skip in real Buildkite builds if (hasAttr label parentTargetMap) && parentTargetMap."${label}".drvPath == drvPath
(getEnv "BUILDKITE_BUILD_ID" != "") && then "Target has not changed."
# Always build everything for the canon branch. else false;
(getEnv "BUILDKITE_BRANCH" != headBranch) &&
# Discard string context to avoid realising the store path during
# pipeline construction.
(pathExists (unsafeDiscardStringContext target.outPath));
in if shouldSkip then "Target was already built." else false;
# Create a pipeline step from a single target. # Create a pipeline step from a single target.
mkStep = headBranch: skipIfBuilt: target: { mkStep = headBranch: parentTargetMap: target:
label = ":nix: ${mkLabel target}"; let
skip = if skipIfBuilt then skip headBranch target else false; label = mkLabel target;
drvPath = unsafeDiscardStringContext target.drvPath;
shouldSkip' = shouldSkip parentTargetMap;
in {
label = ":nix: " + label;
skip = shouldSkip' label drvPath;
command = let command = concatStringsSep " " [
drvPath = unsafeDiscardStringContext target.drvPath;
in concatStringsSep " " [
# First try to realise the drvPath of the target so we don't evaluate twice. # First try to realise the drvPath of the target so we don't evaluate twice.
# Nix has no concept of depending on a derivation file without depending on # Nix has no concept of depending on a derivation file without depending on
# at least one of its `outPath`s, so we need to discard the string context # at least one of its `outPath`s, so we need to discard the string context
@ -124,9 +123,10 @@ in rec {
# possible, in order, without any concurrency restrictions. # possible, in order, without any concurrency restrictions.
drvTargets, drvTargets,
# Should build steps be skipped (on non-HEAD builds) if the output # Derivation map of a parent commit. Only targets which no longer
# path has already been built? # correspond to the content of this map will be built. Passing an
skipIfBuilt ? false, # empty map will always build all targets.
parentTargetMap ? {},
# A list of plain Buildkite step structures to run alongside the # A list of plain Buildkite step structures to run alongside the
# build for all drvTargets, but before proceeding with any # build for all drvTargets, but before proceeding with any
@ -141,7 +141,7 @@ in rec {
# Can be used for status reporting steps and the like. # Can be used for status reporting steps and the like.
postBuildSteps ? [] postBuildSteps ? []
}: let }: let
mkStep' = mkStep headBranch skipIfBuilt; mkStep' = mkStep headBranch parentTargetMap;
steps = steps =
# Add build steps for each derivation target. # Add build steps for each derivation target.
(map mkStep' drvTargets) (map mkStep' drvTargets)

View file

@ -1,6 +1,6 @@
# This file configures the primary build pipeline used for the # This file configures the primary build pipeline used for the
# top-level list of depot targets. # top-level list of depot targets.
{ depot, pkgs, ... }: { depot, pkgs, externalArgs, ... }:
let let
# Protobuf check step which validates that changes to .proto files # Protobuf check step which validates that changes to .proto files
@ -17,11 +17,15 @@ let
command = "${depot.tools.depotfmt.check}"; command = "${depot.tools.depotfmt.check}";
label = ":evergreen_tree: (tools/depotfmt)"; label = ":evergreen_tree: (tools/depotfmt)";
}; };
pipeline = depot.nix.buildkite.mkPipeline { pipeline = depot.nix.buildkite.mkPipeline {
headBranch = "refs/heads/canon"; headBranch = "refs/heads/canon";
drvTargets = depot.ci.targets; drvTargets = depot.ci.targets;
skipIfBuilt = true;
additionalSteps = [ depotfmtCheck protoCheck ]; additionalSteps = [ depotfmtCheck protoCheck ];
parentTargetMap = if (externalArgs ? parentTargetMap)
then builtins.fromJSON (builtins.readFile externalArgs.parentTargetMap)
else {};
}; };
drvmap = depot.nix.buildkite.mkDrvmap depot.ci.targets; drvmap = depot.nix.buildkite.mkDrvmap depot.ci.targets;