Skip to content

Commit

Permalink
Add watch_settings(ignore=) for global ignores. Fixes #3404 (#3793)
Browse files Browse the repository at this point in the history
  • Loading branch information
nicks committed Sep 22, 2020
1 parent 3a4eb3c commit b08d6af
Show file tree
Hide file tree
Showing 15 changed files with 298 additions and 70 deletions.
7 changes: 4 additions & 3 deletions internal/engine/configs/actions.go
Expand Up @@ -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
Expand All @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion internal/engine/configs/configs_controller.go
Expand Up @@ -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,
Expand All @@ -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,
})
}

Expand Down
70 changes: 40 additions & 30 deletions internal/engine/fswatch/watchmanager.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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]
Expand All @@ -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))
}
Expand All @@ -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 {
Expand Down
33 changes: 32 additions & 1 deletion internal/engine/fswatch/watchmanager_test.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
}
Expand Down
8 changes: 6 additions & 2 deletions internal/engine/upper.go
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions internal/engine/upper_test.go
Expand Up @@ -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()
Expand Down
4 changes: 3 additions & 1 deletion internal/store/engine_state.go
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions internal/tiltfile/include_test.go
Expand Up @@ -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) {
Expand Down
36 changes: 18 additions & 18 deletions internal/tiltfile/tiltfile.go
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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:"-"`
Expand Down Expand Up @@ -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)

Expand All @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit b08d6af

Please sign in to comment.