From f7f333ad529f6d68c60b1c0354ded196caf131ba Mon Sep 17 00:00:00 2001 From: Sergio Rubio Date: Mon, 17 Feb 2025 19:02:40 +0100 Subject: [PATCH 1/3] Auto-reload config file on RENAME Some editors (like Vim), create a temp file when saving, then replace the file being edited with the temp file. This causes the FS notify event to be RENAME, not WRITE, so auto-reload misses this. In addition to that, the file is removed from the watcher and the auto-reload functionality stops working entirely after the first RENAME. https://github.com/fsnotify/fsnotify/issues/255 describes this. --- internal/glance/config.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/internal/glance/config.go b/internal/glance/config.go index 7b3377b..88479dc 100644 --- a/internal/glance/config.go +++ b/internal/glance/config.go @@ -286,6 +286,20 @@ func configFilesWatcher( } if event.Has(fsnotify.Write) { debouncedCallback() + } else if event.Has(fsnotify.Rename) { + // wait for file to be available + for i := 0; i < 20; i++ { + _, err := os.Stat(mainFileAbsPath) + if err == nil { + break + } + time.Sleep(100 * time.Millisecond) + } + err := watcher.Add(mainFileAbsPath) + if err != nil { + onErr(fmt.Errorf("watching file:", err)) + } + debouncedCallback() } else if event.Has(fsnotify.Remove) { func() { mu.Lock() From 76a80ff034a4c1801ca9e03576f4e24e2f31f517 Mon Sep 17 00:00:00 2001 From: Sergio Rubio Date: Mon, 17 Feb 2025 19:17:49 +0100 Subject: [PATCH 2/3] Add clarifying comment --- internal/glance/config.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/glance/config.go b/internal/glance/config.go index 88479dc..875ce4f 100644 --- a/internal/glance/config.go +++ b/internal/glance/config.go @@ -295,6 +295,9 @@ func configFilesWatcher( } time.Sleep(100 * time.Millisecond) } + // fsnotify removes the file from the watch list on rename events, + // add it back. + // See https://github.com/fsnotify/fsnotify/issues/214 err := watcher.Add(mainFileAbsPath) if err != nil { onErr(fmt.Errorf("watching file:", err)) From 27af0400c0326f8554305175a2da1bb15bf39fcb Mon Sep 17 00:00:00 2001 From: Svilen Markov <7613769+svilenmarkov@users.noreply.github.com> Date: Mon, 17 Feb 2025 22:28:10 +0000 Subject: [PATCH 3/3] Tweak impl for handling config renames --- internal/glance/config.go | 57 ++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/internal/glance/config.go b/internal/glance/config.go index 875ce4f..0d424a2 100644 --- a/internal/glance/config.go +++ b/internal/glance/config.go @@ -242,7 +242,7 @@ func configFilesWatcher( // needed for lastContents and lastIncludes because they get updated in multiple goroutines mu := sync.Mutex{} - checkForContentChangesBeforeCallback := func() { + parseAndCompareBeforeCallback := func() { currentContents, currentIncludes, err := parseYAMLIncludes(mainFilePath) if err != nil { onErr(fmt.Errorf("parsing main file contents for comparison: %w", err)) @@ -268,15 +268,22 @@ func configFilesWatcher( const debounceDuration = 500 * time.Millisecond var debounceTimer *time.Timer - debouncedCallback := func() { + debouncedParseAndCompareBeforeCallback := func() { if debounceTimer != nil { debounceTimer.Stop() debounceTimer.Reset(debounceDuration) } else { - debounceTimer = time.AfterFunc(debounceDuration, checkForContentChangesBeforeCallback) + debounceTimer = time.AfterFunc(debounceDuration, parseAndCompareBeforeCallback) } } + deleteLastInclude := func(filePath string) { + mu.Lock() + defer mu.Unlock() + fileAbsPath, _ := filepath.Abs(filePath) + delete(lastIncludes, fileAbsPath) + } + go func() { for { select { @@ -285,33 +292,33 @@ func configFilesWatcher( return } if event.Has(fsnotify.Write) { - debouncedCallback() + debouncedParseAndCompareBeforeCallback() } else if event.Has(fsnotify.Rename) { - // wait for file to be available - for i := 0; i < 20; i++ { - _, err := os.Stat(mainFileAbsPath) - if err == nil { + // on linux the file will no longer be watched after a rename, on windows + // it will continue to be watched with the new name but we have no access to + // the new name in this event in order to stop watching it manually and match the + // behavior in linux, may lead to weird unintended behaviors on windows as we're + // only handling renames from linux's perspective + // see https://github.com/fsnotify/fsnotify/issues/255 + + // remove the old file from our manually tracked includes, calling + // debouncedParseAndCompareBeforeCallback will re-add it if it's still + // required after it triggers + deleteLastInclude(event.Name) + + // wait for file to maybe get created again + // see https://github.com/glanceapp/glance/pull/358 + for i := 0; i < 10; i++ { + if _, err := os.Stat(event.Name); err == nil { break } - time.Sleep(100 * time.Millisecond) + time.Sleep(200 * time.Millisecond) } - // fsnotify removes the file from the watch list on rename events, - // add it back. - // See https://github.com/fsnotify/fsnotify/issues/214 - err := watcher.Add(mainFileAbsPath) - if err != nil { - onErr(fmt.Errorf("watching file:", err)) - } - debouncedCallback() - } else if event.Has(fsnotify.Remove) { - func() { - mu.Lock() - defer mu.Unlock() - fileAbsPath, _ := filepath.Abs(event.Name) - delete(lastIncludes, fileAbsPath) - }() - debouncedCallback() + debouncedParseAndCompareBeforeCallback() + } else if event.Has(fsnotify.Remove) { + deleteLastInclude(event.Name) + debouncedParseAndCompareBeforeCallback() } case err, isOpen := <-watcher.Errors: if !isOpen {