diff --git a/internal/engine/configs/actions.go b/internal/engine/configs/actions.go index b7ac944026..699d3b3a6a 100644 --- a/internal/engine/configs/actions.go +++ b/internal/engine/configs/actions.go @@ -20,9 +20,9 @@ func (ConfigsReloadStartedAction) Action() {} type ConfigsReloadedAction struct { // TODO(nick): Embed TiltfileLoadResult instead of copying fields. - Manifests []model.Manifest - TiltIgnoreContents string - ConfigFiles []string + Manifests []model.Manifest + Tiltignore model.Dockerignore + ConfigFiles []string FinishTime time.Time Err error @@ -36,6 +36,7 @@ type ConfigsReloadedAction struct { AnalyticsTiltfileOpt analytics.Opt VersionSettings model.VersionSettings UpdateSettings model.UpdateSettings + WatchSettings model.WatchSettings // A checkpoint into the logstore when Tiltfile execution started. // Useful for knowing how far back in time we have to scrub secrets. diff --git a/internal/engine/configs/configs_controller.go b/internal/engine/configs/configs_controller.go index 03ef8a60be..bbbbe44a62 100644 --- a/internal/engine/configs/configs_controller.go +++ b/internal/engine/configs/configs_controller.go @@ -159,8 +159,8 @@ func (cc *ConfigsController) loadTiltfile(ctx context.Context, st store.RStore, st.Dispatch(ConfigsReloadedAction{ Manifests: tlr.Manifests, + Tiltignore: tlr.Tiltignore, ConfigFiles: tlr.ConfigFiles, - TiltIgnoreContents: tlr.TiltIgnoreContents, FinishTime: cc.clock(), Err: tlr.Error, Features: tlr.FeatureFlags, @@ -173,6 +173,7 @@ func (cc *ConfigsController) loadTiltfile(ctx context.Context, st store.RStore, CheckpointAtExecStart: entry.checkpointAtExecStart, VersionSettings: tlr.VersionSettings, UpdateSettings: tlr.UpdateSettings, + WatchSettings: tlr.WatchSettings, }) } diff --git a/internal/engine/fswatch/watchmanager.go b/internal/engine/fswatch/watchmanager.go index 0164543d85..a2c7ee840d 100644 --- a/internal/engine/fswatch/watchmanager.go +++ b/internal/engine/fswatch/watchmanager.go @@ -4,17 +4,15 @@ import ( "context" "fmt" "path/filepath" - "strings" "sync" "time" - builderDockerignore "github.com/docker/docker/builder/dockerignore" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/tilt-dev/fsnotify" "github.com/tilt-dev/tilt/internal/dockerignore" "github.com/tilt-dev/tilt/internal/ignore" - "github.com/tilt-dev/tilt/internal/sliceutils" "github.com/tilt-dev/tilt/internal/store" "github.com/tilt-dev/tilt/internal/watch" "github.com/tilt-dev/tilt/pkg/logger" @@ -102,13 +100,13 @@ type targetNotifyCancel struct { } type WatchManager struct { - targetWatches map[model.TargetID]targetNotifyCancel - fsWatcherMaker FsWatcherMaker - timerMaker TimerMaker - globalIgnorePatterns []string - globalIgnore model.PathMatcher - disabledForTesting bool - mu sync.Mutex + targetWatches map[model.TargetID]targetNotifyCancel + fsWatcherMaker FsWatcherMaker + timerMaker TimerMaker + globalIgnores []model.Dockerignore + globalIgnore model.PathMatcher + disabledForTesting bool + mu sync.Mutex } func NewWatchManager(watcherMaker FsWatcherMaker, timerMaker TimerMaker) *WatchManager { @@ -145,12 +143,8 @@ func (w *WatchManager) diff(ctx context.Context, st store.RStore) (setup []Watch targetsToProcess[ConfigsTargetID] = &configsTarget{dependencies: append([]string(nil), state.ConfigFiles...)} } - newGlobalIgnore, err := globalIgnorePatterns(state) - if err != nil { - st.Dispatch(store.NewErrorAction(err)) - } - - globalIgnoreChanged := !sliceutils.StringSliceEquals(w.globalIgnorePatterns, newGlobalIgnore) + newGlobalIgnores := globalIgnores(state) + globalIgnoreChanged := !cmp.Equal(newGlobalIgnores, w.globalIgnores, cmpopts.EquateEmpty()) for name, mnc := range w.targetWatches { m, ok := targetsToProcess[name] @@ -173,10 +167,9 @@ func (w *WatchManager) diff(ctx context.Context, st store.RStore) (setup []Watch } if globalIgnoreChanged { - w.globalIgnorePatterns = newGlobalIgnore + w.globalIgnores = newGlobalIgnores - tiltRoot := filepath.Dir(state.TiltfilePath) - globalIgnoreFilter, err := dockerignore.NewDockerPatternMatcher(tiltRoot, newGlobalIgnore) + globalIgnoreFilter, err := dockerignoresToMatcher(newGlobalIgnores) if err != nil { st.Dispatch(store.NewErrorAction(err)) } @@ -187,27 +180,44 @@ func (w *WatchManager) diff(ctx context.Context, st store.RStore) (setup []Watch } // Return a list of global ignore patterns. -// -// NOTE(nick): This is getting pretty messy, because we're mixing -// absolute paths (from the custom_builds) with relative paths (from the .tiltignore). -// In the medium term, it would be good if we had good data structures -// for passing around ignore patterns that identified their source and context. -func globalIgnorePatterns(es store.EngineState) ([]string, error) { - result, err := builderDockerignore.ReadAll(strings.NewReader(es.TiltIgnoreContents)) - if err != nil { - return nil, err +func globalIgnores(es store.EngineState) []model.Dockerignore { + ignores := []model.Dockerignore{} + if !es.Tiltignore.Empty() { + ignores = append(ignores, es.Tiltignore) } + ignores = append(ignores, es.WatchSettings.Ignores...) + outputs := []string{} for _, manifest := range es.Manifests() { for _, iTarget := range manifest.ImageTargets { customBuild := iTarget.CustomBuildInfo() if customBuild.OutputsImageRefTo != "" { - result = append(result, customBuild.OutputsImageRefTo) + outputs = append(outputs, customBuild.OutputsImageRefTo) } } } - return result, nil + if len(outputs) > 0 { + ignores = append(ignores, model.Dockerignore{ + LocalPath: filepath.Dir(es.TiltfilePath), + Source: "outputs_image_ref_to", + Patterns: outputs, + }) + } + + return ignores +} + +func dockerignoresToMatcher(ignores []model.Dockerignore) (model.PathMatcher, error) { + matchers := make([]model.PathMatcher, 0, len(ignores)) + for _, ignore := range ignores { + matcher, err := dockerignore.NewDockerPatternMatcher(ignore.LocalPath, ignore.Patterns) + if err != nil { + return nil, err + } + matchers = append(matchers, matcher) + } + return model.NewCompositeMatcher(matchers), nil } func watchRulesMatch(w1, w2 WatchableTarget) bool { diff --git a/internal/engine/fswatch/watchmanager_test.go b/internal/engine/fswatch/watchmanager_test.go index 684c3746a6..02a36b0903 100644 --- a/internal/engine/fswatch/watchmanager_test.go +++ b/internal/engine/fswatch/watchmanager_test.go @@ -6,9 +6,11 @@ import ( "path/filepath" "reflect" "runtime" + "strings" "testing" "time" + "github.com/docker/docker/builder/dockerignore" "github.com/stretchr/testify/assert" "github.com/tilt-dev/tilt/internal/k8s/testyaml" @@ -159,6 +161,29 @@ func TestWatchManager_IgnoreTiltIgnore(t *testing.T) { f.AssertActionsNotContain(actions, filepath.Join("bar", "foo")) } +func TestWatchManager_IgnoreWatchSettings(t *testing.T) { + f := newWMFixture(t) + defer f.TearDown() + + target := model.DockerComposeTarget{Name: "foo"}. + WithBuildPath(".") + f.SetManifestTarget(target) + + f.store.WithState(func(es *store.EngineState) { + es.WatchSettings.Ignores = append(es.WatchSettings.Ignores, model.Dockerignore{ + LocalPath: f.Path(), + Patterns: []string{"**/foo"}, + }) + }) + f.wm.OnChange(f.ctx, f.store) + + f.ChangeFile(t, filepath.Join("bar", "foo")) + + actions := f.Stop(t) + + f.AssertActionsNotContain(actions, filepath.Join("bar", "foo")) +} + func TestWatchManager_PickUpTiltIgnoreChanges(t *testing.T) { f := newWMFixture(t) defer f.TearDown() @@ -304,7 +329,13 @@ func (f *wmFixture) SetManifestTarget(target model.DockerComposeTarget) { func (f *wmFixture) SetTiltIgnoreContents(s string) { state := f.store.LockMutableStateForTesting() - state.TiltIgnoreContents = s + + patterns, err := dockerignore.ReadAll(strings.NewReader(s)) + assert.NoError(f.T(), err) + state.Tiltignore = model.Dockerignore{ + LocalPath: f.Path(), + Patterns: patterns, + } f.store.UnlockMutableState() f.wm.OnChange(f.ctx, f.store) } diff --git a/internal/engine/upper.go b/internal/engine/upper.go index 65751ebc18..67523bed2f 100644 --- a/internal/engine/upper.go +++ b/internal/engine/upper.go @@ -564,8 +564,12 @@ func handleConfigsReloaded( state.LogStore.ScrubSecretsStartingAt(newSecrets, event.CheckpointAtExecStart) // Add tiltignore if it exists, even if execution failed. - if event.TiltIgnoreContents != "" || event.Err == nil { - state.TiltIgnoreContents = event.TiltIgnoreContents + if !event.Tiltignore.Empty() || event.Err == nil { + state.Tiltignore = event.Tiltignore + } + + if !event.WatchSettings.Empty() || event.Err == nil { + state.WatchSettings = event.WatchSettings } // Add team id if it exists, even if execution failed. diff --git a/internal/engine/upper_test.go b/internal/engine/upper_test.go index a4baefd498..df94584047 100644 --- a/internal/engine/upper_test.go +++ b/internal/engine/upper_test.go @@ -3531,14 +3531,14 @@ fail('x')`) }) f.WaitUntil(".tiltignore processed", func(es store.EngineState) bool { - return strings.Contains(es.TiltIgnoreContents, "a.txt") + return strings.Contains(strings.Join(es.Tiltignore.Patterns, "\n"), "a.txt") }) f.WriteFile(".tiltignore", "a.txt\nb.txt\n") f.fsWatcher.Events <- watch.NewFileEvent(f.JoinPath("Tiltfile")) f.WaitUntil(".tiltignore processed", func(es store.EngineState) bool { - return strings.Contains(es.TiltIgnoreContents, "b.txt") + return strings.Contains(strings.Join(es.Tiltignore.Patterns, "\n"), "b.txt") }) err := f.Stop() diff --git a/internal/store/engine_state.go b/internal/store/engine_state.go index 707939cc04..adddc80a84 100644 --- a/internal/store/engine_state.go +++ b/internal/store/engine_state.go @@ -78,7 +78,9 @@ type EngineState struct { // because this could refer to directories that are watched recursively. ConfigFiles []string - TiltIgnoreContents string + Tiltignore model.Dockerignore + WatchSettings model.WatchSettings + PendingConfigFileChanges map[string]time.Time TriggerQueue []model.ManifestName diff --git a/internal/tiltfile/include_test.go b/internal/tiltfile/include_test.go index 2ad4ffe838..bd4a62d55e 100644 --- a/internal/tiltfile/include_test.go +++ b/internal/tiltfile/include_test.go @@ -34,6 +34,10 @@ k8s_yaml(['foo.yaml', 'bar.yaml']) f.assertNextManifest("bar", db(image("gcr.io/bar")), deployment("bar")) + + f.assertConfigFiles(".tiltignore", "Tiltfile", + "bar.yaml", "bar/.dockerignore", "bar/Dockerfile", "bar/Tiltfile", + "foo.yaml", "foo/.dockerignore", "foo/Dockerfile", "foo/Tiltfile") } func TestIncludeCircular(t *testing.T) { diff --git a/internal/tiltfile/tiltfile.go b/internal/tiltfile/tiltfile.go index 7f63f6da1e..38cce65a12 100644 --- a/internal/tiltfile/tiltfile.go +++ b/internal/tiltfile/tiltfile.go @@ -3,16 +3,11 @@ package tiltfile import ( "context" "fmt" - "io/ioutil" "os" "path/filepath" "strconv" "time" - "github.com/tilt-dev/tilt/internal/tiltfile/config" - "github.com/tilt-dev/tilt/internal/tiltfile/metrics" - "github.com/tilt-dev/tilt/internal/tiltfile/starkit" - wmanalytics "github.com/tilt-dev/wmclient/pkg/analytics" "go.starlark.net/resolve" "go.starlark.net/starlark" @@ -24,19 +19,22 @@ import ( "github.com/tilt-dev/tilt/internal/ospath" "github.com/tilt-dev/tilt/internal/sliceutils" tiltfileanalytics "github.com/tilt-dev/tilt/internal/tiltfile/analytics" + "github.com/tilt-dev/tilt/internal/tiltfile/config" "github.com/tilt-dev/tilt/internal/tiltfile/dockerprune" "github.com/tilt-dev/tilt/internal/tiltfile/io" "github.com/tilt-dev/tilt/internal/tiltfile/k8scontext" + "github.com/tilt-dev/tilt/internal/tiltfile/metrics" "github.com/tilt-dev/tilt/internal/tiltfile/secretsettings" + "github.com/tilt-dev/tilt/internal/tiltfile/starkit" "github.com/tilt-dev/tilt/internal/tiltfile/telemetry" "github.com/tilt-dev/tilt/internal/tiltfile/updatesettings" "github.com/tilt-dev/tilt/internal/tiltfile/value" "github.com/tilt-dev/tilt/internal/tiltfile/version" + "github.com/tilt-dev/tilt/internal/tiltfile/watch" "github.com/tilt-dev/tilt/pkg/model" ) const FileName = "Tiltfile" -const TiltIgnoreFileName = ".tiltignore" func init() { resolve.AllowLambda = true @@ -47,8 +45,8 @@ func init() { type TiltfileLoadResult struct { Manifests []model.Manifest + Tiltignore model.Dockerignore ConfigFiles []string - TiltIgnoreContents string FeatureFlags map[string]bool TeamID string TelemetrySettings model.TelemetrySettings @@ -59,6 +57,7 @@ type TiltfileLoadResult struct { AnalyticsOpt wmanalytics.Opt VersionSettings model.VersionSettings UpdateSettings model.UpdateSettings + WatchSettings model.WatchSettings // For diagnostic purposes only BuiltinCalls []starkit.BuiltinCall `json:"-"` @@ -162,20 +161,20 @@ func (tfl tiltfileLoader) Load(ctx context.Context, filename string, userConfigS } } - tiltIgnorePath := tiltIgnorePath(absFilename) + tiltignorePath := watch.TiltignorePath(absFilename) tlr := TiltfileLoadResult{ - ConfigFiles: []string{absFilename, tiltIgnorePath}, + ConfigFiles: []string{absFilename, tiltignorePath}, } - tiltIgnoreContents, err := ioutil.ReadFile(tiltIgnorePath) + tiltignore, err := watch.ReadTiltignore(tiltignorePath) // missing tiltignore is fine, but a filesystem error is not - if err != nil && !os.IsNotExist(err) { + if err != nil { tlr.Error = err return tlr } - tlr.TiltIgnoreContents = string(tiltIgnoreContents) + tlr.Tiltignore = tiltignore localRegistry := tfl.kCli.LocalRegistry(ctx) @@ -185,12 +184,18 @@ func (tfl tiltfileLoader) Load(ctx context.Context, filename string, userConfigS tlr.BuiltinCalls = result.BuiltinCalls + ws, _ := watch.GetState(result) + tlr.WatchSettings = ws + // NOTE(maia): if/when add secret settings that affect the engine, add them to tlr here ss, _ := secretsettings.GetState(result) s.secretSettings = ss ioState, _ := io.GetState(result) - tlr.ConfigFiles = sliceutils.AppendWithoutDupes(ioState.Paths, s.postExecReadFiles...) + + tlr.ConfigFiles = append(tlr.ConfigFiles, ioState.Paths...) + tlr.ConfigFiles = append(tlr.ConfigFiles, s.postExecReadFiles...) + tlr.ConfigFiles = sliceutils.DedupedAndSorted(tlr.ConfigFiles) dps, _ := dockerprune.GetState(result) tlr.DockerPruneSettings = dps @@ -227,11 +232,6 @@ func (tfl tiltfileLoader) Load(ctx context.Context, filename string, userConfigS return tlr } -// .tiltignore sits next to Tiltfile -func tiltIgnorePath(tiltfilePath string) string { - return filepath.Join(filepath.Dir(tiltfilePath), TiltIgnoreFileName) -} - func starlarkValueOrSequenceToSlice(v starlark.Value) []starlark.Value { return value.ValueOrSequenceToSlice(v) } diff --git a/internal/tiltfile/tiltfile_state.go b/internal/tiltfile/tiltfile_state.go index 2c2b17f843..48086d6df0 100644 --- a/internal/tiltfile/tiltfile_state.go +++ b/internal/tiltfile/tiltfile_state.go @@ -37,6 +37,7 @@ import ( "github.com/tilt-dev/tilt/internal/tiltfile/tiltextension" "github.com/tilt-dev/tilt/internal/tiltfile/updatesettings" "github.com/tilt-dev/tilt/internal/tiltfile/version" + "github.com/tilt-dev/tilt/internal/tiltfile/watch" "github.com/tilt-dev/tilt/pkg/logger" "github.com/tilt-dev/tilt/pkg/model" ) @@ -210,6 +211,7 @@ func (s *tiltfileState) loadManifests(absFilename string, userConfigState model. secretsettings.NewExtension(), encoding.NewExtension(), shlex.NewExtension(), + watch.NewExtension(), tiltextension.NewExtension(fetcher, tiltextension.NewLocalStore(filepath.Dir(absFilename))), ) if err != nil { @@ -407,7 +409,7 @@ func (s *tiltfileState) OnBuiltinCall(name string, fn *starlark.Builtin) { } func (s *tiltfileState) OnExec(t *starlark.Thread, tiltfilePath string) error { - return io.RecordReadPath(t, io.WatchFileOnly, tiltIgnorePath(tiltfilePath)) + return nil } // wrap a builtin such that it's only allowed to run when we have a known safe k8s context diff --git a/internal/tiltfile/watch/tiltignore.go b/internal/tiltfile/watch/tiltignore.go new file mode 100644 index 0000000000..bcc4b8fcdc --- /dev/null +++ b/internal/tiltfile/watch/tiltignore.go @@ -0,0 +1,43 @@ +package watch + +import ( + "bytes" + "fmt" + "io/ioutil" + "os" + "path/filepath" + + "github.com/docker/docker/builder/dockerignore" + + "github.com/tilt-dev/tilt/pkg/model" +) + +const TiltignoreFileName = ".tiltignore" + +// .tiltignore sits next to Tiltfile +func TiltignorePath(tiltfilePath string) string { + return filepath.Join(filepath.Dir(tiltfilePath), TiltignoreFileName) +} + +func ReadTiltignore(tiltignorePath string) (model.Dockerignore, error) { + tiltignoreContents, err := ioutil.ReadFile(tiltignorePath) + + // missing tiltignore is fine, but a filesystem error is not + if err != nil { + if os.IsNotExist(err) { + return model.Dockerignore{}, nil + } + return model.Dockerignore{}, err + } + + patterns, err := dockerignore.ReadAll(bytes.NewBuffer(tiltignoreContents)) + if err != nil { + return model.Dockerignore{}, fmt.Errorf("Parsing .tiltignore: %v", err) + } + + return model.Dockerignore{ + LocalPath: filepath.Dir(tiltignorePath), + Source: tiltignorePath, + Patterns: patterns, + }, nil +} diff --git a/internal/tiltfile/watch/watch.go b/internal/tiltfile/watch/watch.go new file mode 100644 index 0000000000..66f2867d71 --- /dev/null +++ b/internal/tiltfile/watch/watch.go @@ -0,0 +1,63 @@ +package watch + +import ( + "go.starlark.net/starlark" + + "github.com/tilt-dev/tilt/internal/tiltfile/starkit" + "github.com/tilt-dev/tilt/internal/tiltfile/value" + "github.com/tilt-dev/tilt/pkg/model" +) + +type Extension struct { +} + +func NewExtension() Extension { + return Extension{} +} + +func (e Extension) NewState() interface{} { + return model.WatchSettings{} +} + +func (e Extension) OnStart(env *starkit.Environment) error { + return env.AddBuiltin("watch_settings", e.setWatchSettings) +} + +func (e Extension) setWatchSettings(thread *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) { + err := starkit.SetState(thread, func(settings model.WatchSettings) (model.WatchSettings, error) { + var ignores value.StringOrStringList + if err := starkit.UnpackArgs(thread, fn.Name(), args, kwargs, + "ignore?", &ignores, + ); err != nil { + return settings, err + } + + if len(ignores.Values) != 0 { + settings.Ignores = append(settings.Ignores, model.Dockerignore{ + LocalPath: starkit.AbsWorkingDir(thread), + Patterns: ignores.Values, + Source: "watch_settings()", + }) + } + + return settings, nil + }) + + return starlark.None, err +} + +var _ starkit.StatefulExtension = Extension{} + +func MustState(model starkit.Model) model.WatchSettings { + state, err := GetState(model) + if err != nil { + panic(err) + } + return state +} + +func GetState(m starkit.Model) (model.WatchSettings, error) { + var state model.WatchSettings + err := m.Load(&state) + return state, err +} diff --git a/internal/tiltfile/watch/watch_test.go b/internal/tiltfile/watch/watch_test.go new file mode 100644 index 0000000000..ac1df6ae06 --- /dev/null +++ b/internal/tiltfile/watch/watch_test.go @@ -0,0 +1,54 @@ +package watch + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/tilt-dev/tilt/internal/tiltfile/starkit" + "github.com/tilt-dev/tilt/pkg/model" +) + +func TestBasic(t *testing.T) { + f := NewFixture(t) + f.File("Tiltfile", ` +watch_settings(ignore=['foo']) +`) + result, err := f.ExecFile("Tiltfile") + require.NoError(t, err) + require.Equal(t, model.WatchSettings{ + Ignores: []model.Dockerignore{ + { + LocalPath: f.Path(), + Patterns: []string{"foo"}, + Source: "watch_settings()", + }, + }, + }, MustState(result)) +} + +func TestLoaded(t *testing.T) { + f := NewFixture(t) + f.File("foo/Tiltfile", ` +watch_settings(ignore=['bar']) +x = 1 +`) + f.File("Tiltfile", ` +load('./foo/Tiltfile', 'x') +`) + result, err := f.ExecFile("Tiltfile") + require.NoError(t, err) + require.Equal(t, model.WatchSettings{ + Ignores: []model.Dockerignore{ + { + LocalPath: f.JoinPath("foo"), + Patterns: []string{"bar"}, + Source: "watch_settings()", + }, + }, + }, MustState(result)) +} + +func NewFixture(tb testing.TB) *starkit.Fixture { + return starkit.NewFixture(tb, NewExtension()) +} diff --git a/pkg/model/manifest.go b/pkg/model/manifest.go index 6ff39efbd5..74b2698bff 100644 --- a/pkg/model/manifest.go +++ b/pkg/model/manifest.go @@ -286,17 +286,6 @@ type Sync struct { ContainerPath string } -type Dockerignore struct { - // The path to evaluate the dockerignore contents relative to - LocalPath string - - // A human-readable string that identifies where the ignores come from. - Source string - - // Patterns parsed out of the .dockerignore file. - Patterns []string -} - type LocalGitRepo struct { LocalPath string } diff --git a/pkg/model/watch_settings.go b/pkg/model/watch_settings.go new file mode 100644 index 0000000000..fa7572bd04 --- /dev/null +++ b/pkg/model/watch_settings.go @@ -0,0 +1,24 @@ +package model + +type WatchSettings struct { + Ignores []Dockerignore +} + +func (ws WatchSettings) Empty() bool { + return len(ws.Ignores) == 0 +} + +type Dockerignore struct { + // The path to evaluate the dockerignore contents relative to + LocalPath string + + // A human-readable string that identifies where the ignores come from. + Source string + + // Patterns parsed out of the .dockerignore file. + Patterns []string +} + +func (d Dockerignore) Empty() bool { + return len(d.Patterns) == 0 +}