From 3b04a981aaa5d0c47be51dcf3de818583cc45114 Mon Sep 17 00:00:00 2001 From: Mike Holly Date: Thu, 9 Nov 2023 09:40:57 -0800 Subject: [PATCH] Auto-skip: handle --no-cache & --push on command line (#3478) Includes the following auto-skip changes: * Fail with messages when `--no-cache` and `--push` are passed to the CLI with `--auto-skip` * ~Abort when `--no-cache` is used in a visited `RUN` command~ * Don't add log-sharing link when build is skipped * Added back prefix for some auto-skip messages (only when build continues) --- cmd/earthly/subcmd/build_cmd.go | 24 ++++++++++++++++++------ inputgraph/hash.go | 4 +--- inputgraph/loader.go | 9 +-------- tests/autoskip/Earthfile | 11 +++++++++++ tests/autoskip/simple.earth | 8 ++++++++ 5 files changed, 39 insertions(+), 17 deletions(-) diff --git a/cmd/earthly/subcmd/build_cmd.go b/cmd/earthly/subcmd/build_cmd.go index 9bdd41b8..0ef122b5 100644 --- a/cmd/earthly/subcmd/build_cmd.go +++ b/cmd/earthly/subcmd/build_cmd.go @@ -63,6 +63,8 @@ import ( buildkitgitutil "github.com/moby/buildkit/util/gitutil" ) +const autoSkipPrefix = "auto-skip" + type Build struct { cli CLI @@ -258,7 +260,6 @@ func (a *Build) ActionBuildImp(cliCtx *cli.Context, flagArgs, nonFlagArgs []stri // Determine if Logstream is enabled and create log sharing link in either case. logstreamURL, doLogstreamUpload, printLinkFn := a.logShareLink(cliCtx.Context, cloudClient, target, cleanCollection) - a.cli.AddDeferredFunc(printLinkFn) // Output log sharing link after build and other possible messages a.cli.Console().PrintPhaseHeader(builder.PhaseInit, false, "") a.warnIfArgContainsBuildArg(flagArgs) @@ -323,6 +324,9 @@ func (a *Build) ActionBuildImp(cliCtx *cli.Context, flagArgs, nonFlagArgs []stri return nil } + // Output log sharing link after build. Invoked after auto-skip is checked (above). + a.cli.AddDeferredFunc(printLinkFn) + err = a.cli.InitFrontend(cliCtx) if err != nil { return errors.Wrapf(err, "could not init frontend") @@ -618,7 +622,7 @@ func (a *Build) ActionBuildImp(cliCtx *cli.Context, flagArgs, nonFlagArgs []stri if a.cli.Flags().SkipBuildkit && targetHash != nil { err := skipDB.Add(cliCtx.Context, targetHash) if err != nil { - a.cli.Console().Warnf("failed to record %s (hash %x) as completed: %s", target.String(), target, err) + a.cli.Console().WithPrefix(autoSkipPrefix).Warnf("failed to record %s (hash %x) as completed: %s", target.String(), target, err) } } @@ -821,6 +825,17 @@ func (a *Build) initAutoSkip(ctx context.Context, target domain.Target, overridi return nil, nil, false, nil } + console := a.cli.Console().WithPrefix(autoSkipPrefix) + consoleNoPrefix := a.cli.Console() + + if a.cli.Flags().Push { + return nil, nil, false, errors.New("--push cannot be used with --auto-skip") + } + + if a.cli.Flags().NoCache { + return nil, nil, false, errors.New("--no-cache cannot be used with --auto-skip") + } + var ( skipDB bk.BuildkitSkipper targetHash []byte @@ -828,13 +843,10 @@ func (a *Build) initAutoSkip(ctx context.Context, target domain.Target, overridi projectName string ) - console := a.cli.Console() - orgName, projectName, targetHash, err := inputgraph.HashTarget(ctx, inputgraph.HashOpt{ Target: target, Console: a.cli.Console(), CI: a.cli.Flags().CI, - Push: a.cli.Flags().Push, BuiltinArgs: variables.DefaultArgs{EarthlyVersion: a.cli.Version(), EarthlyBuildSha: a.cli.GitSHA()}, OverridingVars: overridingVars, EarthlyCIRunner: a.cli.Flags().EarthlyCIRunner, @@ -856,7 +868,7 @@ func (a *Build) initAutoSkip(ctx context.Context, target domain.Target, overridi } if exists { - console.Printf("target %s (hash %x) has already been run; exiting", target.String(), targetHash) + consoleNoPrefix.Printf("target %s (hash %x) has already been run; exiting", target.String(), targetHash) return nil, nil, true, nil } diff --git a/inputgraph/hash.go b/inputgraph/hash.go index 8851b76f..76daf08a 100644 --- a/inputgraph/hash.go +++ b/inputgraph/hash.go @@ -2,7 +2,6 @@ package inputgraph import ( "context" - "errors" "github.com/earthly/earthly/conslogging" "github.com/earthly/earthly/domain" @@ -14,7 +13,6 @@ type HashOpt struct { Target domain.Target Console conslogging.ConsoleLogger CI bool - Push bool BuiltinArgs variables.DefaultArgs OverridingVars *variables.Scope EarthlyCIRunner bool @@ -31,7 +29,7 @@ func HashTarget(ctx context.Context, opt HashOpt) (org, project string, hash []b err = l.load(ctx) if err != nil { - return "", "", nil, errors.Join(ErrUnableToDetermineHash, err) + return "", "", nil, err } return org, project, l.hasher.GetHash(), nil diff --git a/inputgraph/loader.go b/inputgraph/loader.go index cac7ce6d..4303344c 100644 --- a/inputgraph/loader.go +++ b/inputgraph/loader.go @@ -22,10 +22,7 @@ import ( "github.com/pkg/errors" ) -var ( - ErrRemoteNotSupported = fmt.Errorf("remote targets not supported") - ErrUnableToDetermineHash = fmt.Errorf("unable to determine hash") -) +var ErrRemoteNotSupported = fmt.Errorf("remote targets not supported") type loader struct { conslog conslogging.ConsoleLogger @@ -36,7 +33,6 @@ type loader struct { features *features.Features isBaseTarget bool ci bool - push bool builtinArgs variables.DefaultArgs overridingVars *variables.Scope earthlyCIRunner bool @@ -51,7 +47,6 @@ func newLoader(ctx context.Context, opt HashOpt) *loader { hasher: hasher.New(), isBaseTarget: opt.Target.Target == "base", ci: opt.CI, - push: opt.Push, builtinArgs: opt.BuiltinArgs, overridingVars: opt.OverridingVars, earthlyCIRunner: opt.EarthlyCIRunner, @@ -496,7 +491,6 @@ func (l *loader) forTarget(ctx context.Context, target domain.Target, args []str hasher: l.hasher, isBaseTarget: target.Target == "base", ci: l.ci, - push: l.push, builtinArgs: l.builtinArgs, overridingVars: overriding, earthlyCIRunner: l.earthlyCIRunner, @@ -586,7 +580,6 @@ func (l *loader) load(ctx context.Context) error { Console: l.conslog, Target: l.target, CI: l.ci, - Push: l.push, BuiltinArgs: l.builtinArgs, OverridingVars: l.overridingVars, EarthlyCIRunner: l.earthlyCIRunner, diff --git a/tests/autoskip/Earthfile b/tests/autoskip/Earthfile index 3923b333..8b866854 100644 --- a/tests/autoskip/Earthfile +++ b/tests/autoskip/Earthfile @@ -20,6 +20,8 @@ test-all: BUILD +test-copy-target-args BUILD +test-arg-matrix BUILD +test-try-catch + BUILD +test-push + BUILD +test-no-cache test-files: RUN echo hello > my-file @@ -137,6 +139,14 @@ test-try-catch: DO --pass-args +RUN_EARTHLY_ARGS --earthfile=try-catch.earth --target=+basic --output_contains="Artifact +basic/hello.txt output as hello.txt" DO --pass-args +RUN_EARTHLY_ARGS --earthfile=try-catch.earth --target=+basic --output_contains="target .* has already been run; exiting" +test-no-cache: + DO --pass-args +RUN_EARTHLY_ARGS --earthfile=simple.earth --target=+no-cache --should_fail=true --extra_args="--no-cache" --output_contains="no-cache cannot be used" + DO --pass-args +RUN_EARTHLY_ARGS --earthfile=simple.earth --target=+no-cache --should_fail=true --extra_args="--no-cache" --output_does_not_contain="target .* has already been run; exiting" + +test-push: + DO --pass-args +RUN_EARTHLY_ARGS --earthfile=simple.earth --target=+simple --should_fail=true --extra_args="--push" --output_contains="push cannot be used" + DO --pass-args +RUN_EARTHLY_ARGS --earthfile=simple.earth --target=+simple --should_fail=true --extra_args="--push" --output_does_not_contain="target .* has already been run; exiting" + RUN_EARTHLY_ARGS: COMMAND ARG earthfile @@ -145,6 +155,7 @@ RUN_EARTHLY_ARGS: ARG output_contains ARG output_does_not_contain ARG post_command + ARG extra_args DO --pass-args tests+RUN_EARTHLY \ --earthfile=$earthfile \ --target=$target \ diff --git a/tests/autoskip/simple.earth b/tests/autoskip/simple.earth index 6241436b..b7451a66 100644 --- a/tests/autoskip/simple.earth +++ b/tests/autoskip/simple.earth @@ -12,3 +12,11 @@ mytarget: mypipeline: PIPELINE BUILD +mytarget + +no-cache: + FROM alpine + RUN --no-cache echo "hello" + +simple: + FROM alpine + RUN echo "hello"