custom_build should allow 0 deps. Also, clean up path unpacking. fixes …

…#3891 (#3914)

internal/tiltfile/docker.go

@@ -268,7 +268,7 @@ func (s *tiltfileState) parseOnly(val starlark.Value) ([]string, error) {
 func (s *tiltfileState) customBuild(thread *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
 	var dockerRef string
 	var commandVal, commandBatVal starlark.Value
-	var deps *starlark.List
+	deps := value.NewLocalPathListUnpacker(thread)
 	var tag string
 	var disablePush bool
 	var liveUpdateVal, ignoreVal starlark.Value
@@ -302,22 +302,6 @@ func (s *tiltfileState) customBuild(thread *starlark.Thread, fn *starlark.Builti
 		return nil, fmt.Errorf("Argument 1 (ref): can't parse %q: %v", dockerRef, err)
 	}
 
-	if deps == nil || deps.Len() == 0 {
-		return nil, fmt.Errorf("Argument 3 (deps) can't be empty")
-	}
-
-	var localDeps []string
-	iter := deps.Iterate()
-	defer iter.Done()
-	var v starlark.Value
-	for iter.Next(&v) {
-		p, err := value.ValueToAbsPath(thread, v)
-		if err != nil {
-			return nil, fmt.Errorf("Argument 3 (deps): %v", err)
-		}
-		localDeps = append(localDeps, p)
-	}
-
 	liveUpdate, err := s.liveUpdateFromSteps(thread, liveUpdateVal)
 	if err != nil {
 		return nil, errors.Wrap(err, "live_update")
@@ -357,7 +341,7 @@ func (s *tiltfileState) customBuild(thread *starlark.Thread, fn *starlark.Builti
 		workDir:           starkit.AbsWorkingDir(thread),
 		configurationRef:  container.NewRefSelector(ref),
 		customCommand:     command,
-		customDeps:        localDeps,
+		customDeps:        deps.Value,
 		customTag:         tag,
 		disablePush:       disablePush,
 		skipsLocalDocker:  skipsLocalDocker,

internal/tiltfile/docker_compose.go

@@ -31,23 +31,15 @@ type dcResourceSet struct {
 func (dc dcResourceSet) Empty() bool { return reflect.DeepEqual(dc, dcResourceSet{}) }
 
 func (s *tiltfileState) dockerCompose(thread *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
-	var configPathsValue starlark.Value
+	configPaths := value.NewLocalPathListUnpacker(thread)
 
-	err := s.unpackArgs(fn.Name(), args, kwargs, "configPaths", &configPathsValue)
+	err := s.unpackArgs(fn.Name(), args, kwargs, "configPaths", &configPaths)
 	if err != nil {
 		return nil, err
 	}
 
-	pathSlice := starlarkValueOrSequenceToSlice(configPathsValue)
-	var configPaths []string
-	for _, v := range pathSlice {
-		path, err := value.ValueToAbsPath(thread, v)
-		if err != nil {
-			return nil, fmt.Errorf("docker_compose files must be a string or a sequence of strings; found a %T", v)
-		}
-		configPaths = append(configPaths, path)
-
-		err = io.RecordReadPath(thread, io.WatchFileOnly, path)
+	for _, v := range configPaths.Value {
+		err = io.RecordReadPath(thread, io.WatchFileOnly, v)
 		if err != nil {
 			return nil, err
 		}
@@ -64,7 +56,7 @@ func (s *tiltfileState) dockerCompose(thread *starlark.Thread, fn *starlark.Buil
 	// To make sure all the docker-compose files are compatible together,
 	// parse them all together.
 	allConfigPaths := append([]string{}, dc.configPaths...)
-	allConfigPaths = append(allConfigPaths, configPaths...)
+	allConfigPaths = append(allConfigPaths, configPaths.Value...)
 
 	services, err := parseDCConfig(s.ctx, s.dcCli, allConfigPaths)
 	if err != nil {

internal/tiltfile/docker_test.go

@@ -105,6 +105,19 @@ custom_build('gcr.io/fe', 'docker build -t $EXPECTED_REF .', ['src'])
 	assert.Contains(t, localPathStrings, f.JoinPath("src"))
 }
 
+func TestCustomBuildDepsZeroArgs(t *testing.T) {
+	f := newFixture(t)
+	defer f.TearDown()
+
+	f.yaml("fe.yaml", deployment("fe", image("gcr.io/fe")))
+	f.file("Tiltfile", `
+k8s_yaml('fe.yaml')
+custom_build('gcr.io/fe', 'docker build -t $EXPECTED_REF .', [])
+`)
+
+	f.load()
+}
+
 func TestCustomBuildOutputsImageRefsTo(t *testing.T) {
 	f := newFixture(t)
 	defer f.TearDown()

internal/tiltfile/live_update.go

@@ -132,22 +132,13 @@ func (s *tiltfileState) recordLiveUpdateStep(step liveUpdateStep) {
 }
 
 func (s *tiltfileState) liveUpdateFallBackOn(thread *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
-	var files starlark.Value
+	files := value.NewLocalPathListUnpacker(thread)
 	if err := s.unpackArgs(fn.Name(), args, kwargs, "paths", &files); err != nil {
 		return nil, err
 	}
-	filesSlice := starlarkValueOrSequenceToSlice(files)
-	var paths []string
-	for _, f := range filesSlice {
-		path, err := value.ValueToAbsPath(thread, f)
-		if err != nil {
-			return nil, fmt.Errorf("fall_back_on step contained value '%s' of type '%s'. it may only contain strings", f, f.Type())
-		}
-		paths = append(paths, path)
-	}
 
 	ret := liveUpdateFallBackOnStep{
-		files:    paths,
+		files:    files.Value,
 		position: thread.CallFrame(1).Pos,
 	}
 	s.recordLiveUpdateStep(ret)

internal/tiltfile/live_update_test.go

@@ -97,7 +97,8 @@ docker_build('gcr.io/foo', 'foo',
     sync('bar', '/baz'),
   ],
 )`)
-	f.loadErrString("fall_back_on", "value '4' of type 'int'")
+	f.loadErrString("fall_back_on",
+		"fall_back_on: for parameter paths: value should be a string or List or Tuple of strings, but is of type int")
 }
 
 func TestLiveUpdateNonStringInRunTriggers(t *testing.T) {

internal/tiltfile/local_resource.go

@@ -33,7 +33,9 @@ func (s *tiltfileState) localResource(thread *starlark.Thread, fn *starlark.Buil
 	var name string
 	var updateCmdVal, updateCmdBatVal, serveCmdVal, serveCmdBatVal starlark.Value
 	var triggerMode triggerMode
-	var deps starlark.Value
+
+	deps := value.NewLocalPathListUnpacker(thread)
+
 	var resourceDepsVal starlark.Sequence
 	var ignoresVal starlark.Value
 	var allowParallel bool
@@ -57,17 +59,7 @@ func (s *tiltfileState) localResource(thread *starlark.Thread, fn *starlark.Buil
 		return nil, err
 	}
 
-	depsVals := starlarkValueOrSequenceToSlice(deps)
-	var depsStrings []string
-	for _, v := range depsVals {
-		path, err := value.ValueToAbsPath(thread, v)
-		if err != nil {
-			return nil, fmt.Errorf("deps must be a string or a sequence of strings; found a %T", v)
-		}
-		depsStrings = append(depsStrings, path)
-	}
-
-	repos := reposForPaths(depsStrings)
+	repos := reposForPaths(deps.Value)
 
 	resourceDeps, err := value.SequenceToStringSlice(resourceDepsVal)
 	if err != nil {
@@ -97,7 +89,7 @@ func (s *tiltfileState) localResource(thread *starlark.Thread, fn *starlark.Buil
 		updateCmd:     updateCmd,
 		serveCmd:      serveCmd,
 		workdir:       filepath.Dir(starkit.CurrentExecPath(thread)),
-		deps:          depsStrings,
+		deps:          deps.Value,
 		triggerMode:   triggerMode,
 		autoInit:      autoInit,
 		repos:         repos,

internal/tiltfile/value/path.go

@@ -1,6 +1,8 @@
 package value
 
 import (
+	"fmt"
+
 	"go.starlark.net/starlark"
 )
 
@@ -24,3 +26,51 @@ func (p *LocalPath) Unpack(v starlark.Value) error {
 	p.Value = str
 	return nil
 }
+
+type LocalPathList struct {
+	t     *starlark.Thread
+	Value []string
+}
+
+func NewLocalPathListUnpacker(t *starlark.Thread) LocalPathList {
+	return LocalPathList{
+		t: t,
+	}
+}
+
+func (p *LocalPathList) Unpack(v starlark.Value) error {
+	_, ok := AsString(v)
+	if ok {
+		str, err := ValueToAbsPath(p.t, v)
+		if err != nil {
+			return err
+		}
+		p.Value = []string{str}
+		return nil
+	}
+
+	var iter starlark.Iterator
+	switch x := v.(type) {
+	case *starlark.List:
+		iter = x.Iterate()
+	case starlark.Tuple:
+		iter = x.Iterate()
+	default:
+		return fmt.Errorf("value should be a string or List or Tuple of strings, but is of type %s", v.Type())
+
+	}
+
+	defer iter.Done()
+
+	values := []string{}
+	var item starlark.Value
+	for iter.Next(&item) {
+		str, err := ValueToAbsPath(p.t, item)
+		if err != nil {
+			return fmt.Errorf("unpacking list item at index %d: %v", len(values), err)
+		}
+		values = append(values, str)
+	}
+	p.Value = values
+	return nil
+}