Repository: tilt-dev/tilt
Pull request #6488 (2025-01-16 03:29:33)

watch: tweak dir change events
fixes https://github.com/tilt-dev/tilt/issues/6485

Signed-off-by: Nick Santos <nick.santos@docker.com>


nicks: hmm..something weird is happening on mac that i have to look into...sadly, circleci turned off our mac runners and i haven't figured out how to fix them.

Patch:
diff --git a/internal/controllers/core/filewatch/controller.go b/internal/controllers/core/filewatch/controller.go
index 13309173c..5b66aedab 100644
--- a/internal/controllers/core/filewatch/controller.go
+++ b/internal/controllers/core/filewatch/controller.go
@@ -190,6 +190,10 @@ func (c *Controller) addOrReplace(ctx context.Context, name types.NamespacedName
 		w.restartBackoff = existing.restartBackoff
 		status.Error = existing.status.Error
 	}
+	if hasExisting {
+		status.FileEvents = existing.status.FileEvents
+		status.LastEventTime = existing.status.LastEventTime
+	}
 
 	ignoreMatcher := ignore.CreateFileChangeFilter(fw.Spec.Ignores)
 	startFileChangeLoop := false
diff --git a/internal/docker/client.go b/internal/docker/client.go
index 4e224ec91..03e8018d1 100644
--- a/internal/docker/client.go
+++ b/internal/docker/client.go
@@ -69,10 +69,6 @@ var minDockerVersionExperimentalBuildkit = semver.MustParse("1.38.0")
 
 var versionTimeout = 5 * time.Second
 
-// microk8s exposes its own docker socket
-// https://github.com/ubuntu/microk8s/blob/master/docs/dockerd.md
-const microK8sDockerHost = "unix:///var/snap/microk8s/current/docker.sock"
-
 // Create an interface so this can be mocked out.
 type Client interface {
 	CheckConnected() error
diff --git a/internal/docker/env.go b/internal/docker/env.go
index 14d0be15a..86e9bae28 100644
--- a/internal/docker/env.go
+++ b/internal/docker/env.go
@@ -246,21 +246,6 @@ func ProvideClusterEnv(
 		}
 	}
 
-	if product == clusterid.ProductMicroK8s && kClient.ContainerRuntime(ctx) == container.RuntimeDocker {
-		// If we're running Microk8s with a docker runtime, talk to Microk8s's docker socket.
-		d, err := creator.FromEnvMap(map[string]string{"DOCKER_HOST": microK8sDockerHost})
-		if err != nil {
-			return ClusterEnv{Error: fmt.Errorf("connecting to microk8s: %v", err)}
-		}
-
-		// Handle the case where people manually set DOCKER_HOST to microk8s.
-		if hostOverride == "" || hostOverride == d.DaemonHost() {
-			env.Client = d
-			env.Environ = append(env.Environ, fmt.Sprintf("DOCKER_HOST=%s", microK8sDockerHost))
-			env.BuildToKubeContexts = append(env.BuildToKubeContexts, string(kubeContext))
-		}
-	}
-
 	if env.Client == nil {
 		client, err := creator.FromCLI(ctx)
 		env.Client = client
@@ -274,8 +259,7 @@ func ProvideClusterEnv(
 	// images)
 	//
 	// currently, we handle this by inspecting the Docker + K8s configs to see
-	// if they're matched up, but with the exception of microk8s (handled above),
-	// we don't override the environmental Docker config
+	// if they're matched up, but we don't override the environmental Docker config
 	if willBuildToKubeContext(ctx, product, kubeContext, env) &&
 		kClient.ContainerRuntime(ctx) == container.RuntimeDocker {
 		env.BuildToKubeContexts = append(env.BuildToKubeContexts, string(kubeContext))
diff --git a/internal/ospath/ospath.go b/internal/ospath/ospath.go
index 618542fb4..2adb78e91 100644
--- a/internal/ospath/ospath.go
+++ b/internal/ospath/ospath.go
@@ -154,6 +154,15 @@ func IsDir(path string) bool {
 	return f.Mode().IsDir()
 }
 
+func IsDirLstat(path string) bool {
+	f, err := os.Lstat(path)
+	if err != nil {
+		return false
+	}
+
+	return f.Mode().IsDir()
+}
+
 func IsBrokenSymlink(path string) (bool, error) {
 	// Stat resolves symlinks, lstat does not.
 	// So if Stat reports IsNotExist, but Lstat does not,
diff --git a/internal/watch/watcher_darwin.go b/internal/watch/watcher_darwin.go
index 785b70ad4..ba0014791 100644
--- a/internal/watch/watcher_darwin.go
+++ b/internal/watch/watcher_darwin.go
@@ -38,12 +38,19 @@ func (d *darwinNotify) loop() {
 				e.Path = filepath.Join("/", e.Path)
 
 				_, isPathWereWatching := d.pathsWereWatching[e.Path]
-				if e.Flags&fsevents.ItemIsDir == fsevents.ItemIsDir && isPathWereWatching {
+				isDir := e.Flags&fsevents.ItemIsDir == fsevents.ItemIsDir
+				if isDir && isPathWereWatching {
 					// For consistency with Linux and Windows, don't fire any events
 					// for directories that we're watching -- only their contents.
 					continue
 				}
 
+				// On MacOS, modifying a directory entry fires Created | InodeMetaMod
+				// Ignore these events, mod time modifications shouldnt trigger copies.
+				if isDir && (e.Flags&fsevents.ItemInodeMetaMod) == fsevents.ItemInodeMetaMod {
+					continue
+				}
+
 				ignore, err := d.ignore.Matches(e.Path)
 				if err != nil {
 					d.logger.Infof("Error matching path %q: %v", e.Path, err)
diff --git a/internal/watch/watcher_naive.go b/internal/watch/watcher_naive.go
index 00ca01174..e7b4482fc 100644
--- a/internal/watch/watcher_naive.go
+++ b/internal/watch/watcher_naive.go
@@ -151,16 +151,32 @@ func (d *naiveNotify) loop() {
 		}
 
 		if e.Op&fsnotify.Create != fsnotify.Create {
-			if d.shouldNotify(e.Name) {
-				d.wrappedEvents <- FileEvent{e.Name}
+			if !d.shouldNotify(e.Name) {
+				continue
 			}
+
+			// Don't send events for directories when the modtime is being changed.
+			//
+			// This is a bit of a hack because every OS represents modtime updates
+			// a bit differently and they don't map well to fsnotify events.
+			//
+			// On Windows, updating the modtime of a directory is a fsnotify.Write.
+			// On Linux, it's a fsnotify.Chmod.
+			isDirUpdateOnly := (e.Op == fsnotify.Write || e.Op == fsnotify.Chmod) &&
+				ospath.IsDir(e.Name)
+			if isDirUpdateOnly {
+				continue
+			}
+
+			d.wrappedEvents <- FileEvent{e.Name}
 			continue
 		}
 
 		if d.isWatcherRecursive {
-			if d.shouldNotify(e.Name) {
-				d.wrappedEvents <- FileEvent{e.Name}
+			if !d.shouldNotify(e.Name) {
+				continue
 			}
+			d.wrappedEvents <- FileEvent{e.Name}
 			continue
 		}
 
@@ -223,8 +239,7 @@ func (d *naiveNotify) shouldNotify(path string) bool {
 
 	if _, ok := d.notifyList[path]; ok {
 		// We generally don't care when directories change at the root of an ADD
-		stat, err := os.Lstat(path)
-		isDir := err == nil && stat.IsDir()
+		isDir := ospath.IsDirLstat(path)
 		if isDir {
 			return false
 		}


Test patch:
diff --git a/internal/controllers/core/filewatch/controller_test.go b/internal/controllers/core/filewatch/controller_test.go
index ba392f531..1afbe0c6e 100644
--- a/internal/controllers/core/filewatch/controller_test.go
+++ b/internal/controllers/core/filewatch/controller_test.go
@@ -324,13 +324,14 @@ func TestController_Reconcile_Watches(t *testing.T) {
 	assert.Truef(t, updatedStart.After(originalStart),
 		"Monitor start time should be more recent after update, (original: %s, updated: %s)",
 		originalStart, updatedStart)
-	if assert.Equal(t, 2, len(updated.Status.FileEvents)) {
+	if assert.Equal(t, 3, len(updated.Status.FileEvents)) {
 		// ensure ONLY the expected files were seen
 		assert.NotZero(t, updated.Status.FileEvents[0].Time.Time)
-		mostRecentEventTime := updated.Status.FileEvents[1].Time.Time
+		mostRecentEventTime := updated.Status.FileEvents[2].Time.Time
 		assert.NotZero(t, mostRecentEventTime)
-		assert.Equal(t, []string{f.tmpdir.JoinPath("d", "1")}, updated.Status.FileEvents[0].SeenFiles)
-		assert.Equal(t, []string{f.tmpdir.JoinPath("d", "2")}, updated.Status.FileEvents[1].SeenFiles)
+		assert.Equal(t, []string{f.tmpdir.JoinPath("a", "1")}, updated.Status.FileEvents[0].SeenFiles)
+		assert.Equal(t, []string{f.tmpdir.JoinPath("d", "1")}, updated.Status.FileEvents[1].SeenFiles)
+		assert.Equal(t, []string{f.tmpdir.JoinPath("d", "2")}, updated.Status.FileEvents[2].SeenFiles)
 		assert.Equal(t, mostRecentEventTime, updated.Status.LastEventTime.Time)
 	}
 }
diff --git a/internal/docker/client_test.go b/internal/docker/client_test.go
index e71b16a8d..99ec4d2f9 100644
--- a/internal/docker/client_test.go
+++ b/internal/docker/client_test.go
@@ -150,18 +150,6 @@ func TestProvideClusterProduct(t *testing.T) {
 				Client: hostClient{Host: "tcp://192.168.99.100:2376"},
 			},
 		},
-		{
-			env:     clusterid.ProductMicroK8s,
-			runtime: container.RuntimeDocker,
-			expectedCluster: Env{
-				Client:              hostClient{Host: "unix:///var/snap/microk8s/current/docker.sock"},
-				Environ:             []string{"DOCKER_HOST=unix:///var/snap/microk8s/current/docker.sock"},
-				BuildToKubeContexts: []string{"microk8s-me"},
-			},
-			expectedLocal: Env{
-				Client: hostClient{Host: "unix:///var/run/docker.sock"},
-			},
-		},
 		{
 			env:     clusterid.ProductMicroK8s,
 			runtime: container.RuntimeCrio,
diff --git a/internal/watch/notify_test.go b/internal/watch/notify_test.go
index 533bce175..d44316a7f 100644
--- a/internal/watch/notify_test.go
+++ b/internal/watch/notify_test.go
@@ -218,6 +218,24 @@ func TestWatchDirectoryAndTouchIt(t *testing.T) {
 	f.assertEvents()
 }
 
+func TestWatchDirectoryAndTouchSubdir(t *testing.T) {
+	f := newNotifyFixture(t)
+
+	cTime := time.Now()
+	root := f.TempDir("root")
+	path := filepath.Join(root, "change")
+	a := filepath.Join(path, "a.txt")
+	f.WriteFile(a, "a")
+
+	f.watch(root)
+	f.fsync()
+
+	err := os.Chtimes(path, cTime, time.Now().Add(time.Minute))
+	assert.NoError(t, err)
+
+	f.assertEvents()
+}
+
 func TestWatchNonExistentPathDoesNotFireSiblingEvent(t *testing.T) {
 	f := newNotifyFixture(t)