Auto-skip: handle --no-cache & --push on command line (earthly#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)
rrjjvv · Nov 9, 2023 · 3b04a98 · 3b04a98
1 parent 1ad2128
commit 3b04a98
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 17 deletions.
diff --git 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,20 +825,28 @@ 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
 		orgName     string
 		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
@@ -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
@@ -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
@@ -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
@@ -12,3 +12,11 @@ mytarget:
 mypipeline:
   PIPELINE
   BUILD +mytarget
+
+no-cache:
+  FROM alpine
+  RUN --no-cache echo "hello"
+
+simple:
+  FROM alpine
+  RUN echo "hello"