From 335bf6900d9cc3f8e94a40f567423fbb07878e36 Mon Sep 17 00:00:00 2001 From: Evgeny Zemtsov Date: Fri, 30 Sep 2022 14:21:11 +0200 Subject: [PATCH] fix(nix/buildkite): follow parent skip behavior in extra steps We found a bug after updating to latest tvl-kit which broke incremental releases. Bug was related to the fact that extra steps skip attribute had precedence over parent configuration. This is a desired behavior when extra step is explicitly set to `skip=true` but otherwise it must follow parent. Due to extra step normalization skip parameter is always set to false if not explicitly set. Along the way, I'm adding support for setting skip attribute on extra steps as string so that people can define skip reasons. The bug was introduced by commit: b9d79109d feat(ops/buildkite): Allow skip of individual steps Change-Id: I8a46d0926a749d2434412b309c661b749e9dbf37 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6827 Autosubmit: ezemtsov Reviewed-by: tazjin Tested-by: BuildkiteCI --- nix/buildkite/default.nix | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/nix/buildkite/default.nix b/nix/buildkite/default.nix index f588b27b5..510c68673 100644 --- a/nix/buildkite/default.nix +++ b/nix/buildkite/default.nix @@ -397,7 +397,18 @@ rec { let step = { label = ":gear: ${cfg.label} (from ${cfg.parentLabel})"; - skip = if cfg.alwaysRun then false else cfg.skip or cfg.parent.skip or false; + skip = + let + # When parent doesn't have skip attribute set, default to false + parentSkip = cfg.parent.skip or false; + # Extra step skip parameter can be string explaining the + # skip reason. + extraStepSkip = if builtins.isString cfg.skip then true else cfg.skip; + # Don't run if extra step is explicitly set to skip. If + # parameter is not set or equal to false, follow parent behavior. + skip' = if extraStepSkip then cfg.skip else parentSkip; + in + if cfg.alwaysRun then false else skip'; depends_on = lib.optional (buildEnabled && !cfg.alwaysRun && !cfg.needsOutput)