Auto-skip arg expansion (#3404)

This PR adds support for ARG passing and expansion to auto-skip. CLI args, `BUILD` args, `--pass-args`, and other command args are now used to generate the hash. Changing any of these will invalidate the auto-skip check.
earthly · Oct 21, 2023 · a3069ce · a3069ce
1 parent 37cd702
commit a3069ce
Show file tree

Hide file tree

Showing 15 changed files with 830 additions and 561 deletions.
diff --git a/cmd/earthly/subcmd/build_cmd.go b/cmd/earthly/subcmd/build_cmd.go
@@ -21,6 +21,7 @@ import (
 	"github.com/docker/cli/cli/config"
 	"github.com/fatih/color"
 	"github.com/joho/godotenv"
+	bkclient "github.com/moby/buildkit/client"
 	"github.com/moby/buildkit/client/llb"
 	"github.com/moby/buildkit/session"
 	"github.com/moby/buildkit/session/auth"
@@ -262,113 +263,6 @@ func (a *Build) ActionBuildImp(cliCtx *cli.Context, flagArgs, nonFlagArgs []stri
 	a.cli.Console().PrintPhaseHeader(builder.PhaseInit, false, "")
 	a.warnIfArgContainsBuildArg(flagArgs)
 
-	var skipDB bk.BuildkitSkipper
-	var targetHash []byte
-	if a.cli.Flags().SkipBuildkit {
-		var orgName string
-		var projectName string
-		orgName, projectName, targetHash, err = inputgraph.HashTarget(cliCtx.Context, target, a.cli.Console())
-		if err != nil {
-			a.cli.Console().Warnf("unable to calculate hash for %s: %s", target.String(), err.Error())
-		} else {
-			skipDB, err = bk.NewBuildkitSkipper(a.cli.Flags().LocalSkipDB, orgName, projectName, target.GetName(), cloudClient)
-			if err != nil {
-				return err
-			}
-			exists, err := skipDB.Exists(cliCtx.Context, targetHash)
-			if err != nil {
-				a.cli.Console().Warnf("unable to check if target %s (hash %x) has already been run: %s", target.String(), targetHash, err.Error())
-			}
-			if exists {
-				a.cli.Console().Printf("target %s (hash %x) has already been run; exiting", target.String(), targetHash)
-				return nil
-			}
-		}
-	}
-
-	err = a.cli.InitFrontend(cliCtx)
-	if err != nil {
-		return errors.Wrapf(err, "could not init frontend")
-	}
-
-	err = a.cli.ConfigureSatellite(cliCtx, cloudClient, gitCommitAuthor, gitConfigEmail)
-	if err != nil {
-		return errors.Wrapf(err, "could not configure satellite")
-	}
-
-	// After configuring frontend and satellites, buildkit address should not be empty.
-	// It should be set to a local container, remote address, or satellite address at this point.
-	if a.cli.Flags().BuildkitdSettings.BuildkitAddress == "" {
-		return errors.New("could not determine buildkit address - is Docker or Podman running?")
-	}
-
-	var runnerName string
-	isLocal := containerutil.IsLocal(a.cli.Flags().BuildkitdSettings.BuildkitAddress)
-	if isLocal {
-		hostname, err := os.Hostname()
-		if err != nil {
-			a.cli.Console().Warnf("failed to get hostname: %v", err)
-			hostname = "unknown"
-		}
-		runnerName = fmt.Sprintf("local:%s", hostname)
-	} else {
-		if a.cli.Flags().SatelliteName != "" {
-			runnerName = fmt.Sprintf("sat:%s/%s", a.cli.Flags().OrgName, a.cli.Flags().SatelliteName)
-		} else {
-			runnerName = fmt.Sprintf("bk:%s", a.cli.Flags().BuildkitdSettings.BuildkitAddress)
-		}
-	}
-	if !isLocal && (a.cli.Flags().UseInlineCache || a.cli.Flags().SaveInlineCache) {
-		a.cli.Console().Warnf("Note that inline cache (--use-inline-cache and --save-inline-cache) occasionally cause builds to get stuck at 100%% CPU on Satellites and remote Buildkit.")
-		a.cli.Console().Warnf("") // newline
-	}
-	if isLocal && !a.cli.Flags().ContainerFrontend.IsAvailable(cliCtx.Context) {
-		return errors.New("Frontend is not available to perform the build. Is Docker installed and running?")
-	}
-
-	bkClient, err := buildkitd.NewClient(
-		cliCtx.Context,
-		a.cli.Console(),
-		a.cli.Flags().BuildkitdImage,
-		a.cli.Flags().ContainerName,
-		a.cli.Flags().InstallationName,
-		a.cli.Flags().ContainerFrontend,
-		a.cli.Version(),
-		a.cli.Flags().BuildkitdSettings,
-	)
-	if err != nil {
-		return errors.Wrap(err, "build new buildkitd client")
-	}
-	defer bkClient.Close()
-	a.cli.SetAnaMetaIsRemoteBK(!isLocal)
-
-	nativePlatform, err := platutil.GetNativePlatformViaBkClient(cliCtx.Context, bkClient)
-	if err != nil {
-		return errors.Wrap(err, "get native platform via buildkit client")
-	}
-	if a.cli.Flags().Logstream {
-		a.cli.LogbusSetup().SetDefaultPlatform(platforms.Format(nativePlatform))
-	}
-	platr := platutil.NewResolver(nativePlatform)
-	a.cli.SetAnaMetaBKPlatform(platforms.Format(nativePlatform))
-	a.cli.SetAnaMetaUserPlatform(platforms.Format(platr.LLBUser()))
-	platr.AllowNativeAndUser = true
-	platformsSlice := make([]platutil.Platform, 0, len(a.platformsStr.Value()))
-	for _, p := range a.platformsStr.Value() {
-		platform, err := platr.Parse(p)
-		if err != nil {
-			return errors.Wrapf(err, "parse platform %s", p)
-		}
-		platformsSlice = append(platformsSlice, platform)
-	}
-	switch len(platformsSlice) {
-	case 0:
-	case 1:
-		platr.UpdatePlatform(platformsSlice[0])
-	default:
-		return errors.Errorf("multi-platform builds are not yet supported on the command line. You may, however, create a target with the instruction BUILD --platform ... --platform ... %s", target)
-	}
-
 	showUnexpectedEnvWarnings := true
 	dotEnvMap, err := godotenv.Read(a.cli.Flags().EnvFile)
 	if err != nil {
@@ -416,6 +310,62 @@ func (a *Build) ActionBuildImp(cliCtx *cli.Context, flagArgs, nonFlagArgs []stri
 		}
 	}
 
+	overridingVars, err := common.CombineVariables(argMap, flagArgs, a.buildArgs.Value())
+	if err != nil {
+		return err
+	}
+
+	skipDB, targetHash, doSkip, err := a.initAutoSkip(cliCtx.Context, target, overridingVars, cloudClient)
+	if err != nil {
+		return err
+	}
+	if doSkip {
+		return nil
+	}
+
+	err = a.cli.InitFrontend(cliCtx)
+	if err != nil {
+		return errors.Wrapf(err, "could not init frontend")
+	}
+
+	err = a.cli.ConfigureSatellite(cliCtx, cloudClient, gitCommitAuthor, gitConfigEmail)
+	if err != nil {
+		return errors.Wrapf(err, "could not configure satellite")
+	}
+
+	// After configuring frontend and satellites, buildkit address should not be empty.
+	// It should be set to a local container, remote address, or satellite address at this point.
+	if a.cli.Flags().BuildkitdSettings.BuildkitAddress == "" {
+		return errors.New("could not determine buildkit address - is Docker or Podman running?")
+	}
+
+	bkClient, err := buildkitd.NewClient(
+		cliCtx.Context,
+		a.cli.Console(),
+		a.cli.Flags().BuildkitdImage,
+		a.cli.Flags().ContainerName,
+		a.cli.Flags().InstallationName,
+		a.cli.Flags().ContainerFrontend,
+		a.cli.Version(),
+		a.cli.Flags().BuildkitdSettings,
+	)
+	if err != nil {
+		return errors.Wrap(err, "build new buildkitd client")
+	}
+	defer bkClient.Close()
+
+	platr, err := a.platformResolver(cliCtx.Context, bkClient, target)
+	if err != nil {
+		return err
+	}
+
+	runnerName, isLocal, err := a.runnerName(cliCtx.Context)
+	if err != nil {
+		return err
+	}
+
+	a.cli.SetAnaMetaIsRemoteBK(!isLocal)
+
 	localhostProvider, err := localhostprovider.NewLocalhostProvider()
 	if err != nil {
 		return errors.Wrap(err, "failed to create localhostprovider")
@@ -512,11 +462,6 @@ func (a *Build) ActionBuildImp(cliCtx *cli.Context, flagArgs, nonFlagArgs []stri
 		enttlmnts = append(enttlmnts, entitlements.EntitlementSecurityInsecure)
 	}
 
-	overridingVars, err := common.CombineVariables(argMap, flagArgs, a.buildArgs.Value())
-	if err != nil {
-		return err
-	}
-
 	imageResolveMode := llb.ResolveModePreferLocal
 	if a.cli.Flags().Pull {
 		imageResolveMode = llb.ResolveModeForcePull
@@ -807,6 +752,114 @@ func receiveFileVersion2(ctx context.Context, conn io.ReadWriteCloser, localArti
 	return f.Close()
 }
 
+// runnerName returns the name of the local or remote BK "runner"; which is a
+// representation of what BuildKit instance is being used,
+// e.g. local:<hostname>, sat:<org>/<name>, or bk:<remote-address>
+func (a *Build) runnerName(ctx context.Context) (string, bool, error) {
+	var runnerName string
+	isLocal := containerutil.IsLocal(a.cli.Flags().BuildkitdSettings.BuildkitAddress)
+	if isLocal {
+		hostname, err := os.Hostname()
+		if err != nil {
+			a.cli.Console().Warnf("failed to get hostname: %v", err)
+			hostname = "unknown"
+		}
+		runnerName = fmt.Sprintf("local:%s", hostname)
+	} else {
+		if a.cli.Flags().SatelliteName != "" {
+			runnerName = fmt.Sprintf("sat:%s/%s", a.cli.Flags().OrgName, a.cli.Flags().SatelliteName)
+		} else {
+			runnerName = fmt.Sprintf("bk:%s", a.cli.Flags().BuildkitdSettings.BuildkitAddress)
+		}
+	}
+	if !isLocal && (a.cli.Flags().UseInlineCache || a.cli.Flags().SaveInlineCache) {
+		a.cli.Console().Warnf("Note that inline cache (--use-inline-cache and --save-inline-cache) occasionally cause builds to get stuck at 100%% CPU on Satellites and remote Buildkit.")
+		a.cli.Console().Warnf("") // newline
+	}
+	if isLocal && !a.cli.Flags().ContainerFrontend.IsAvailable(ctx) {
+		return "", false, errors.New("Frontend is not available to perform the build. Is Docker installed and running?")
+	}
+	return runnerName, isLocal, nil
+}
+
+func (a *Build) platformResolver(ctx context.Context, bkClient *bkclient.Client, target domain.Target) (*platutil.Resolver, error) {
+	nativePlatform, err := platutil.GetNativePlatformViaBkClient(ctx, bkClient)
+	if err != nil {
+		return nil, errors.Wrap(err, "get native platform via buildkit client")
+	}
+	if a.cli.Flags().Logstream {
+		a.cli.LogbusSetup().SetDefaultPlatform(platforms.Format(nativePlatform))
+	}
+	platr := platutil.NewResolver(nativePlatform)
+	a.cli.SetAnaMetaBKPlatform(platforms.Format(nativePlatform))
+	a.cli.SetAnaMetaUserPlatform(platforms.Format(platr.LLBUser()))
+	platr.AllowNativeAndUser = true
+	platformsSlice := make([]platutil.Platform, 0, len(a.platformsStr.Value()))
+	for _, p := range a.platformsStr.Value() {
+		platform, err := platr.Parse(p)
+		if err != nil {
+			return nil, errors.Wrapf(err, "parse platform %s", p)
+		}
+		platformsSlice = append(platformsSlice, platform)
+	}
+	switch len(platformsSlice) {
+	case 0:
+	case 1:
+		platr.UpdatePlatform(platformsSlice[0])
+	default:
+		return nil, errors.Errorf("multi-platform builds are not yet supported on the command line. You may, however, create a target with the instruction BUILD --platform ... --platform ... %s", target)
+	}
+
+	return platr, nil
+}
+
+func (a *Build) initAutoSkip(ctx context.Context, target domain.Target, overridingVars *variables.Scope, client *cloud.Client) (bk.BuildkitSkipper, []byte, bool, error) {
+	if !a.cli.Flags().SkipBuildkit {
+		return nil, nil, false, nil
+	}
+
+	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,
+	})
+	if err != nil {
+		console.Warnf("unable to calculate hash for %s: %s", target.String(), err.Error())
+		return nil, nil, false, nil
+	}
+
+	skipDB, err = bk.NewBuildkitSkipper(a.cli.Flags().LocalSkipDB, orgName, projectName, target.GetName(), client)
+	if err != nil {
+		return nil, nil, false, err
+	}
+
+	exists, err := skipDB.Exists(ctx, targetHash)
+	if err != nil {
+		console.Warnf("unable to check if target %s (hash %x) has already been run: %s", target.String(), targetHash, err.Error())
+		return nil, nil, false, nil
+	}
+
+	if exists {
+		console.Printf("target %s (hash %x) has already been run; exiting", target.String(), targetHash)
+		return nil, nil, true, nil
+	}
+
+	return skipDB, targetHash, false, nil
+}
+
 func (a *Build) logShareLink(ctx context.Context, cloudClient *cloud.Client, target domain.Target, clean *cleanup.Collection) (string, bool, func()) {
 
 	if a.cli.Cfg().Global.DisableLogSharing {

diff --git a/earthfile2llb/earthfile_info.go b/earthfile2llb/earthfile_info.go
@@ -8,6 +8,7 @@ import (
 	"github.com/earthly/earthly/ast/spec"
 	"github.com/earthly/earthly/buildcontext"
 	"github.com/earthly/earthly/domain"
+	"github.com/earthly/earthly/util/flagutil"
 	"github.com/earthly/earthly/util/platutil"
 	gwclient "github.com/moby/buildkit/frontend/gateway/client"
 	"github.com/pkg/errors"
@@ -54,7 +55,7 @@ func GetTargetArgs(ctx context.Context, resolver *buildcontext.Resolver, gwClien
 			isBase := t.Name == "base"
 			// since Arg opts are ignored (and feature flags are not available) we set explicitGlobalArgFlag as false
 			explicitGlobal := false
-			_, argName, _, err := parseArgArgs(ctx, *stmt.Command, isBase, explicitGlobal)
+			_, argName, _, err := flagutil.ParseArgArgs(ctx, *stmt.Command, isBase, explicitGlobal)
 			if err != nil {
 				return nil, errors.Wrapf(err, "failed to parse ARG arguments %v", stmt.Command.Args)
 			}
@@ -71,7 +72,7 @@ func ArgName(ctx context.Context, cmd spec.Command, isBase bool, explicitGlobal
 	if cmd.Name != "ARG" {
 		return "", nil, false, false, errors.Errorf("ArgName was called with non-arg command type '%v'", cmd.Name)
 	}
-	opts, argName, dflt, err := parseArgArgs(ctx, cmd, isBase, explicitGlobal)
+	opts, argName, dflt, err := flagutil.ParseArgArgs(ctx, cmd, isBase, explicitGlobal)
 	if err != nil {
 		return "", nil, false, false, errors.Wrapf(err, "could not parse opts for ARG [%v]", cmd)
 	}
@@ -97,7 +98,7 @@ func ArtifactName(ctx context.Context, cmd spec.Command) (string, *string, error
 // ImageNames returns the parsed names of a SAVE IMAGE command.
 func ImageNames(ctx context.Context, cmd spec.Command) ([]string, error) {
 	var opts commandflag.SaveImageOpts
-	args, err := parseArgs("SAVE IMAGE", &opts, getArgsCopy(cmd))
+	args, err := flagutil.ParseArgs("SAVE IMAGE", &opts, flagutil.GetArgsCopy(cmd))
 	if err != nil {
 		return nil, errors.Wrapf(err, "invalid SAVE IMAGE arguments %v", cmd.Args)
 	}