From 168585838fc67084551860ab9a8190ed05373113 Mon Sep 17 00:00:00 2001 From: arexon Date: Thu, 26 Sep 2024 00:20:39 +0300 Subject: [PATCH 01/17] Use fsnotify for file watching --- go.mod | 3 +- go.sum | 4 ++ regolith/compatibility_other_os.go | 21 ------- regolith/compatibility_windows.go | 93 ------------------------------ regolith/filter.go | 26 +-------- regolith/watcher.go | 69 ++++++++++++++++++++++ 6 files changed, 78 insertions(+), 138 deletions(-) create mode 100644 regolith/watcher.go diff --git a/go.mod b/go.mod index ab92014d..c53d47b5 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/stirante/go-simple-eval v0.0.0-20230131075324-9ed520afbec1 go.uber.org/zap v1.23.0 golang.org/x/mod v0.6.0 - golang.org/x/sys v0.4.0 + golang.org/x/sys v0.13.0 ) replace github.com/hashicorp/go-getter => github.com/arikkfir/go-getter v1.6.3-0.20220803164326-281b7670b734 @@ -27,6 +27,7 @@ require ( cloud.google.com/go/iam v0.3.0 // indirect cloud.google.com/go/storage v1.21.0 // indirect github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20221202181307-76fa05c21b12 // indirect + github.com/arexon/fsnotify v0.0.0-20240925210538-c862c914df35 // indirect github.com/aws/aws-sdk-go v1.43.25 // indirect github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect github.com/gammazero/deque v0.2.1 // indirect diff --git a/go.sum b/go.sum index b2ea63be..29ee03cb 100644 --- a/go.sum +++ b/go.sum @@ -67,6 +67,8 @@ github.com/alessio/shellescape v1.4.1/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPp github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20221202181307-76fa05c21b12 h1:npHgfD4Tl2WJS3AJaMUi5ynGDPUBfkg3U3fCzDyXZ+4= github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20221202181307-76fa05c21b12/go.mod h1:pSwJ0fSY5KhvocuWSx4fz3BA8OrA1bQn+K1Eli3BRwM= +github.com/arexon/fsnotify v0.0.0-20240925210538-c862c914df35 h1:LcWID+cU05U8/rovFXL+bB6eBhm4waCvzKCvaBHlJoo= +github.com/arexon/fsnotify v0.0.0-20240925210538-c862c914df35/go.mod h1:fMK1EJDCm6IfeqTBptyizpl356fZy33nWqFKELbFouQ= github.com/arikkfir/go-getter v1.6.3-0.20220803164326-281b7670b734 h1:csFUhbcumnsC5d0SMF8CvtR6Z/i4UeNgOZ6xUaQUYas= github.com/arikkfir/go-getter v1.6.3-0.20220803164326-281b7670b734/go.mod h1:IZCrswsZPeWv9IkVnLElzRU/gz/QPi6pZHn4tv6vbwA= github.com/aws/aws-sdk-go v1.15.78/go.mod h1:E3/ieXAlvM0XWO57iftYVDLLvQ824smPP3ATZkfNZeM= @@ -473,6 +475,8 @@ golang.org/x/sys v0.0.0-20220517195934-5e4e11fc645e/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.4.0 h1:Zr2JFtRQNX3BCZ8YtxRE9hNJYC8J6I1MVbMg6owUp18= golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= +golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/regolith/compatibility_other_os.go b/regolith/compatibility_other_os.go index a7d520a0..434af4a3 100644 --- a/regolith/compatibility_other_os.go +++ b/regolith/compatibility_other_os.go @@ -30,27 +30,6 @@ func copyFileSecurityInfo(source string, target string) error { return nil } -type DirWatcher struct{} - -func NewDirWatcher(path string) (*DirWatcher, error) { - return nil, burrito.WrappedError(notImplementedOnThisSystemError) -} - -func (d *DirWatcher) WaitForChange() error { - return burrito.WrappedError(notImplementedOnThisSystemError) -} - -func (d *DirWatcher) WaitForChangeGroup( - groupTimeout uint32, interruptionChannel chan string, - interruptionMessage string, -) error { - return burrito.WrappedError(notImplementedOnThisSystemError) -} - -func (d *DirWatcher) Close() error { - return burrito.WrappedError(notImplementedOnThisSystemError) -} - func FindStandardMojangDir() (string, error) { comMojang := os.Getenv("COM_MOJANG") if comMojang == "" { diff --git a/regolith/compatibility_windows.go b/regolith/compatibility_windows.go index 5ac179cf..4c70b1a3 100644 --- a/regolith/compatibility_windows.go +++ b/regolith/compatibility_windows.go @@ -53,99 +53,6 @@ func copyFileSecurityInfo(source string, target string) error { return nil } -// DirWatcher is a struct that provides easy to use methods for watching a -// directory for changes. It uses FindFirstChangeNotification instead of -// ReadDirectoryChanges, so it doesn't provide any information about the -// changes, only the fact that something changed. -// -// Useful links: -// https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-findfirstchangenotificationa -// -// https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-findnextchangenotification -// -// https://pkg.go.dev/golang.org/x/sys@v0.0.0-20220412211240-33da011f77ad/windows -// -// https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-readdirectorychangesw -type DirWatcher struct { - handle windows.Handle -} - -// NewDirWatcher creates a new DirWatcher for the given path. It filters out -// some of the less interesting events like FILE_NOTIFY_CHANGE_LAST_ACCESS. -func NewDirWatcher(path string) (*DirWatcher, error) { - var notifyFilter uint32 = (windows.FILE_NOTIFY_CHANGE_FILE_NAME | - windows.FILE_NOTIFY_CHANGE_DIR_NAME | - // windows.FILE_NOTIFY_CHANGE_ATTRIBUTES | - // windows.FILE_NOTIFY_CHANGE_SIZE | - windows.FILE_NOTIFY_CHANGE_LAST_WRITE | - // windows.FILE_NOTIFY_CHANGE_LAST_ACCESS | - // windows.FILE_NOTIFY_CHANGE_SECURITY | - windows.FILE_NOTIFY_CHANGE_CREATION) - handle, err := windows.FindFirstChangeNotification( - path, true, notifyFilter) - if err != nil { - return nil, err - } - return &DirWatcher{handle: handle}, nil -} - -// WaitForChange locks the goroutine until a single change is detected. Note -// that some changes are reported multiple times, for example saving a file -// will cause a change to the file and a change to the directory. If you want -// to report cases like that as one event, see WaitForChangeGroup. -func (d *DirWatcher) WaitForChange() error { - _, err := windows.WaitForSingleObject(d.handle, windows.INFINITE) - if err != nil { - return err - } - err = windows.FindNextChangeNotification(d.handle) - if err != nil { - return err - } - return nil -} - -// WaitForChangeGroup locks a goroutine until it receives a change notification. -// When that happens it sends the interruptionMessage to the -// interruptionChannel. -// Then it continues locking as long as other changes keep coming with -// intervals less than the given timeout, to group notifications that come -// in short intervals together. -func (d *DirWatcher) WaitForChangeGroup( - groupTimeout uint32, interruptionChannel chan string, - interruptionMessage string, -) error { - err := d.WaitForChange() - if err != nil { - return err - } - // Instantly report the change - interruptionChannel <- interruptionMessage - // Consume all changes for groupDelay duration - for { - event, err := windows.WaitForSingleObject(d.handle, groupTimeout) - if err != nil { - return err - } - // Possible options: WAIT_OBJECT_0, WAIT_ABANDONED, WAIT_TIMEOUT, - // WAIT_FAILED - if event == uint32(windows.WAIT_TIMEOUT) || - event == uint32(windows.WAIT_ABANDONED) { - break - } - err = windows.FindNextChangeNotification(d.handle) - if err != nil { - return err - } - } - return nil -} - -// Close closes DirWatcher handle. -func (d *DirWatcher) Close() error { - return windows.CloseHandle(d.handle) -} - // FindStandardMojangDir returns path to the com.mojang folder in the standard // Minecraft build. func FindStandardMojangDir() (string, error) { diff --git a/regolith/filter.go b/regolith/filter.go index c80bdee3..25ebacfb 100644 --- a/regolith/filter.go +++ b/regolith/filter.go @@ -64,41 +64,21 @@ func (c *RunContext) StartWatchingSourceFiles() error { c.interruptionChannel = make(chan string) c.fileWatchingErrorChannel = make(chan error) - yieldChanges := func( - watcher *DirWatcher, sourceName string, - ) { - for { - err := watcher.WaitForChangeGroup(100, c.interruptionChannel, sourceName) - if err != nil { - c.fileWatchingErrorChannel <- err - } - } - } - - addWatcher := func(watchedPath, watcherString string) error { - watcher, err := NewDirWatcher(watchedPath) - if err != nil { - return burrito.PassError(err) - } - go yieldChanges(watcher, watcherString) - return nil - } - var err error if c.Config.ResourceFolder != "" { - err = addWatcher(c.Config.ResourceFolder, "rp") + err := NewDirWatcher(c.Config.ResourceFolder, "rp", c.interruptionChannel, c.fileWatchingErrorChannel) if err != nil { return burrito.WrapError(err, "Could not create resource pack watcher.") } } if c.Config.BehaviorFolder != "" { - err = addWatcher(c.Config.BehaviorFolder, "bp") + err := NewDirWatcher(c.Config.BehaviorFolder, "bp", c.interruptionChannel, c.fileWatchingErrorChannel) if err != nil { return burrito.WrapError(err, "Could not create behavior pack watcher.") } } if c.Config.DataPath != "" { - err = addWatcher(c.Config.DataPath, "data") + err := NewDirWatcher(c.Config.DataPath, "data", c.interruptionChannel, c.fileWatchingErrorChannel) if err != nil { return burrito.WrapError(err, "Could not create data watcher.") } diff --git a/regolith/watcher.go b/regolith/watcher.go new file mode 100644 index 00000000..93adbf42 --- /dev/null +++ b/regolith/watcher.go @@ -0,0 +1,69 @@ +package regolith + +import ( + "path/filepath" + + "github.com/Bedrock-OSS/go-burrito/burrito" + "github.com/arexon/fsnotify" +) + +// DirWatcher handles watching for changes in a single directory (e.g. RP). +// +// fsnotify doesn't *officially* support recursive file watching. Windows and +// and Linux are supported, but not macOS. For now, this implementation uses a +// custom fork with patches to manually enable it. See +// https://github.com/arexon/fsnotify/blob/main/fsnotify.go#L481 +type DirWatcher struct { + watcher *fsnotify.Watcher + root string + kind string // Whether the watched directory is "RP", "BP", or "data". + interruption chan string // see RunContext + errors chan error // see RunContext +} + +// NewDirWatcher creates a new pack watcher for the given pack kind. +func NewDirWatcher( + root string, + kind string, + interruption chan string, + errors chan error, +) error { + watcher, err := fsnotify.NewWatcher() + if err != nil { + return burrito.WrapError(err, "Could not initialize directory watching") + } + d := &DirWatcher{ + watcher, + root, + kind, + interruption, + errors, + } + path := filepath.Join(root, "...") + if err := d.watcher.Add(path); err != nil { + return burrito.WrapErrorf(err, "Could not start watching `%f`", root) + } + go d.start() + return nil +} + +// Start starts the file watching loop and blocks the goroutine until it +// receives an event. Once it does, it sends a message to interruptionChannel +// then resumes blocking the goroutine. +func (d *DirWatcher) start() { + for { + select { + case err, ok := <-d.watcher.Errors: + if !ok { + return + } + d.errors <- err + return + case _, ok := <-d.watcher.Events: + if !ok { + return + } + d.interruption <- d.kind + } + } +} From 9409b9d4f64dbd4355a4c4b6becbefe83f23497f Mon Sep 17 00:00:00 2001 From: github-actions Date: Wed, 25 Sep 2024 21:31:42 +0000 Subject: [PATCH 02/17] Generated CREDITS.csv --- CREDITS.csv | 34 ++-------------------------------- 1 file changed, 2 insertions(+), 32 deletions(-) diff --git a/CREDITS.csv b/CREDITS.csv index 7a7c3c1e..e1ac4e1f 100644 --- a/CREDITS.csv +++ b/CREDITS.csv @@ -1,34 +1,14 @@ -cloud.google.com/go/compute/metadata,https://github.com/googleapis/google-cloud-go/blob/compute/v1.5.0/compute/LICENSE,Apache-2.0 -cloud.google.com/go/iam,https://github.com/googleapis/google-cloud-go/blob/iam/v0.3.0/iam/LICENSE,Apache-2.0 -cloud.google.com/go/internal,https://github.com/googleapis/google-cloud-go/blob/v0.100.2/LICENSE,Apache-2.0 -cloud.google.com/go/storage,https://github.com/googleapis/google-cloud-go/blob/storage/v1.21.0/storage/LICENSE,Apache-2.0 github.com/Bedrock-OSS/go-burrito/burrito,https://github.com/Bedrock-OSS/go-burrito/blob/v1.0.3/LICENSE,MIT github.com/Bedrock-OSS/regolith,https://github.com/Bedrock-OSS/regolith/blob/HEAD/LICENSE,MIT github.com/alessio/shellescape,https://github.com/alessio/shellescape/blob/v1.4.1/LICENSE,MIT github.com/antlr/antlr4/runtime/Go/antlr/v4,https://github.com/antlr/antlr4/blob/76fa05c21b12/runtime/Go/antlr/v4/LICENSE,BSD-3-Clause -github.com/aws/aws-sdk-go,https://github.com/aws/aws-sdk-go/blob/v1.43.25/LICENSE.txt,Apache-2.0 -github.com/aws/aws-sdk-go/internal/sync/singleflight,https://github.com/aws/aws-sdk-go/blob/v1.43.25/internal/sync/singleflight/LICENSE,BSD-3-Clause -github.com/bgentry/go-netrc/netrc,https://github.com/bgentry/go-netrc/blob/9fd32a8b3d3d/LICENSE,MIT +github.com/arexon/fsnotify,https://github.com/arexon/fsnotify/blob/c862c914df35/LICENSE,BSD-3-Clause github.com/fatih/color,https://github.com/fatih/color/blob/v1.14.1/LICENSE.md,MIT github.com/gammazero/deque,https://github.com/gammazero/deque/blob/v0.2.1/LICENSE,MIT -github.com/golang/groupcache/lru,https://github.com/golang/groupcache/blob/41bb18bfe9da/LICENSE,Apache-2.0 -github.com/golang/protobuf,https://github.com/golang/protobuf/blob/v1.5.2/LICENSE,BSD-3-Clause -github.com/google/go-cmp/cmp,https://github.com/google/go-cmp/blob/v0.5.8/LICENSE,BSD-3-Clause github.com/google/go-github/v39/github,https://github.com/google/go-github/blob/v39.2.0/LICENSE,BSD-3-Clause github.com/google/go-querystring/query,https://github.com/google/go-querystring/blob/v1.1.0/LICENSE,BSD-3-Clause -github.com/googleapis/gax-go/v2,https://github.com/googleapis/gax-go/blob/v2.2.0/v2/LICENSE,BSD-3-Clause -github.com/hashicorp/go-cleanhttp,https://github.com/hashicorp/go-cleanhttp/blob/v0.5.2/LICENSE,MPL-2.0 -github.com/hashicorp/go-getter,https://github.com/arikkfir/go-getter/blob/281b7670b734/LICENSE,MPL-2.0 -github.com/hashicorp/go-safetemp,https://github.com/hashicorp/go-safetemp/blob/v1.0.0/LICENSE,MPL-2.0 -github.com/hashicorp/go-version,https://github.com/hashicorp/go-version/blob/v1.4.0/LICENSE,MPL-2.0 -github.com/jmespath/go-jmespath,https://github.com/jmespath/go-jmespath/blob/v0.4.0/LICENSE,Apache-2.0 -github.com/klauspost/compress,https://github.com/klauspost/compress/blob/v1.15.1/LICENSE,Apache-2.0 -github.com/klauspost/compress/internal/snapref,https://github.com/klauspost/compress/blob/v1.15.1/internal/snapref/LICENSE,BSD-3-Clause -github.com/klauspost/compress/zstd/internal/xxhash,https://github.com/klauspost/compress/blob/v1.15.1/zstd/internal/xxhash/LICENSE.txt,MIT github.com/mattn/go-colorable,https://github.com/mattn/go-colorable/blob/v0.1.13/LICENSE,MIT github.com/mattn/go-isatty,https://github.com/mattn/go-isatty/blob/v0.0.17/LICENSE,MIT -github.com/mitchellh/go-homedir,https://github.com/mitchellh/go-homedir/blob/v1.1.0/LICENSE,MIT -github.com/mitchellh/go-testing-interface,https://github.com/mitchellh/go-testing-interface/blob/v1.14.1/LICENSE,MIT github.com/muhammadmuzzammil1998/jsonc,https://github.com/muhammadmuzzammil1998/jsonc/blob/v1.0.0/LICENSE,MIT github.com/nightlyone/lockfile,https://github.com/nightlyone/lockfile/blob/v1.0.0/LICENSE,MIT github.com/otiai10/copy,https://github.com/otiai10/copy/blob/v1.7.0/LICENSE,MIT @@ -36,21 +16,11 @@ github.com/paul-mannino/go-fuzzywuzzy,https://github.com/paul-mannino/go-fuzzywu github.com/spf13/cobra,https://github.com/spf13/cobra/blob/v1.6.1/LICENSE.txt,Apache-2.0 github.com/spf13/pflag,https://github.com/spf13/pflag/blob/v1.0.5/LICENSE,BSD-3-Clause github.com/stirante/go-simple-eval,https://github.com/stirante/go-simple-eval/blob/9ed520afbec1/LICENSE,GPL-3.0 -github.com/ulikunitz/xz,https://github.com/ulikunitz/xz/blob/v0.5.10/LICENSE,BSD-3-Clause -go.opencensus.io,https://github.com/census-instrumentation/opencensus-go/blob/v0.23.0/LICENSE,Apache-2.0 go.uber.org/atomic,https://github.com/uber-go/atomic/blob/v1.9.0/LICENSE.txt,MIT go.uber.org/multierr,https://github.com/uber-go/multierr/blob/v1.8.0/LICENSE.txt,MIT go.uber.org/zap,https://github.com/uber-go/zap/blob/v1.23.0/LICENSE.txt,MIT golang.org/x/crypto,https://cs.opensource.google/go/x/crypto/+/v0.1.0:LICENSE,BSD-3-Clause golang.org/x/exp,https://cs.opensource.google/go/x/exp/+/aae9b4e6:LICENSE,BSD-3-Clause golang.org/x/mod/semver,https://cs.opensource.google/go/x/mod/+/v0.6.0:LICENSE,BSD-3-Clause -golang.org/x/net,https://cs.opensource.google/go/x/net/+/v0.1.0:LICENSE,BSD-3-Clause -golang.org/x/oauth2,https://cs.opensource.google/go/x/oauth2/+/6242fa91:LICENSE,BSD-3-Clause -golang.org/x/sys/unix,https://cs.opensource.google/go/x/sys/+/v0.4.0:LICENSE,BSD-3-Clause +golang.org/x/sys/unix,https://cs.opensource.google/go/x/sys/+/v0.13.0:LICENSE,BSD-3-Clause golang.org/x/text,https://cs.opensource.google/go/x/text/+/v0.6.0:LICENSE,BSD-3-Clause -golang.org/x/xerrors,https://cs.opensource.google/go/x/xerrors/+/5ec99f83:LICENSE,BSD-3-Clause -google.golang.org/api,https://github.com/googleapis/google-api-go-client/blob/v0.73.0/LICENSE,BSD-3-Clause -google.golang.org/api/internal/third_party/uritemplates,https://github.com/googleapis/google-api-go-client/blob/v0.73.0/internal/third_party/uritemplates/LICENSE,BSD-3-Clause -google.golang.org/genproto/googleapis,https://github.com/googleapis/go-genproto/blob/acbaeb5b85eb/LICENSE,Apache-2.0 -google.golang.org/grpc,https://github.com/grpc/grpc-go/blob/v1.45.0/LICENSE,Apache-2.0 -google.golang.org/protobuf,https://github.com/protocolbuffers/protobuf-go/blob/v1.28.0/LICENSE,BSD-3-Clause From 76c4270e0dab47f2aa79a32b5e9197a7d5e5fcde Mon Sep 17 00:00:00 2001 From: arexon Date: Thu, 26 Sep 2024 00:35:16 +0300 Subject: [PATCH 03/17] Update comment in DirWatcher --- regolith/watcher.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/regolith/watcher.go b/regolith/watcher.go index 93adbf42..0d100724 100644 --- a/regolith/watcher.go +++ b/regolith/watcher.go @@ -9,10 +9,11 @@ import ( // DirWatcher handles watching for changes in a single directory (e.g. RP). // -// fsnotify doesn't *officially* support recursive file watching. Windows and -// and Linux are supported, but not macOS. For now, this implementation uses a -// custom fork with patches to manually enable it. See -// https://github.com/arexon/fsnotify/blob/main/fsnotify.go#L481 +// fsnotify doesn't *officially* support recursive file watching yet. Windows +// and and Linux are supported, but not macOS. For now, this implementation uses +// a custom fork with patches to manually enable it. +// +// Fork patch: https://github.com/arexon/fsnotify/blob/main/fsnotify.go#L481 type DirWatcher struct { watcher *fsnotify.Watcher root string From adb4c97bb81459bd849c26614535199bdee5eeec Mon Sep 17 00:00:00 2001 From: arexon Date: Thu, 26 Sep 2024 17:03:26 +0300 Subject: [PATCH 04/17] Ignore file attribute events in DirWatcher --- regolith/watcher.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/regolith/watcher.go b/regolith/watcher.go index 0d100724..19693e1f 100644 --- a/regolith/watcher.go +++ b/regolith/watcher.go @@ -49,7 +49,7 @@ func NewDirWatcher( } // Start starts the file watching loop and blocks the goroutine until it -// receives an event. Once it does, it sends a message to interruptionChannel +// receives an event. Once it does, it sends a message to interruption channel // then resumes blocking the goroutine. func (d *DirWatcher) start() { for { @@ -60,10 +60,13 @@ func (d *DirWatcher) start() { } d.errors <- err return - case _, ok := <-d.watcher.Events: + case event, ok := <-d.watcher.Events: if !ok { return } + if event.Op&fsnotify.Chmod == fsnotify.Chmod { + continue + } d.interruption <- d.kind } } From 05746a45042b4d80c3e19404ffaa384a7345e83e Mon Sep 17 00:00:00 2001 From: arexon Date: Thu, 26 Sep 2024 21:02:06 +0300 Subject: [PATCH 05/17] Clarify some stuff in DirWatcher --- regolith/watcher.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/regolith/watcher.go b/regolith/watcher.go index 19693e1f..5dad0ad3 100644 --- a/regolith/watcher.go +++ b/regolith/watcher.go @@ -7,7 +7,7 @@ import ( "github.com/arexon/fsnotify" ) -// DirWatcher handles watching for changes in a single directory (e.g. RP). +// DirWatcher handles watching for changes in a specific directory (e.g. RP). // // fsnotify doesn't *officially* support recursive file watching yet. Windows // and and Linux are supported, but not macOS. For now, this implementation uses @@ -18,11 +18,11 @@ type DirWatcher struct { watcher *fsnotify.Watcher root string kind string // Whether the watched directory is "RP", "BP", or "data". - interruption chan string // see RunContext - errors chan error // see RunContext + interruption chan string // See RunContext. + errors chan error // See RunContext. } -// NewDirWatcher creates a new pack watcher for the given pack kind. +// NewDirWatcher creates a new directory watcher. func NewDirWatcher( root string, kind string, @@ -40,8 +40,10 @@ func NewDirWatcher( interruption, errors, } - path := filepath.Join(root, "...") - if err := d.watcher.Add(path); err != nil { + // We have to manually signal to fsnotify that it should recursively watch this + // path by using "/..." or "\...". + recursiveRoot := filepath.Join(root, "...") + if err := d.watcher.Add(recursiveRoot); err != nil { return burrito.WrapErrorf(err, "Could not start watching `%f`", root) } go d.start() From c909a2b648a214458412dc6e3c5ada335a51182d Mon Sep 17 00:00:00 2001 From: arexon Date: Fri, 27 Sep 2024 01:31:27 +0300 Subject: [PATCH 06/17] Simplify file attribute event check in DirWatcher --- regolith/watcher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/regolith/watcher.go b/regolith/watcher.go index 5dad0ad3..f99c694e 100644 --- a/regolith/watcher.go +++ b/regolith/watcher.go @@ -66,7 +66,7 @@ func (d *DirWatcher) start() { if !ok { return } - if event.Op&fsnotify.Chmod == fsnotify.Chmod { + if event.Op.Has(fsnotify.Chmod) { continue } d.interruption <- d.kind From 88d7fee560929995de8ac05602a8556bb005aa62 Mon Sep 17 00:00:00 2001 From: arexon Date: Fri, 27 Sep 2024 03:50:23 +0300 Subject: [PATCH 07/17] Re-add debouncing to DirWatcher --- regolith/watcher.go | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/regolith/watcher.go b/regolith/watcher.go index f99c694e..fe6d8939 100644 --- a/regolith/watcher.go +++ b/regolith/watcher.go @@ -1,7 +1,10 @@ package regolith import ( + "math" "path/filepath" + "sync" + "time" "github.com/Bedrock-OSS/go-burrito/burrito" "github.com/arexon/fsnotify" @@ -17,9 +20,11 @@ import ( type DirWatcher struct { watcher *fsnotify.Watcher root string - kind string // Whether the watched directory is "RP", "BP", or "data". - interruption chan string // See RunContext. - errors chan error // See RunContext. + kind string // Whether the watched directory is "RP", "BP", or "data". + mu sync.Mutex // Used for locking during deboucning. + timers map[string]*time.Timer // Stores event path -> debounce timer. + interruption chan string // See RunContext. + errors chan error // See RunContext. } // NewDirWatcher creates a new directory watcher. @@ -33,13 +38,16 @@ func NewDirWatcher( if err != nil { return burrito.WrapError(err, "Could not initialize directory watching") } + d := &DirWatcher{ - watcher, - root, - kind, - interruption, - errors, + watcher: watcher, + root: root, + kind: kind, + timers: make(map[string]*time.Timer), + interruption: interruption, + errors: errors, } + // We have to manually signal to fsnotify that it should recursively watch this // path by using "/..." or "\...". recursiveRoot := filepath.Join(root, "...") @@ -69,7 +77,21 @@ func (d *DirWatcher) start() { if event.Op.Has(fsnotify.Chmod) { continue } - d.interruption <- d.kind + + d.mu.Lock() + timer, exists := d.timers[event.Name] + d.mu.Unlock() + + if !exists { + timer = time.AfterFunc(math.MaxInt64, func() { d.interruption <- d.kind }) + timer.Stop() + + d.mu.Lock() + d.timers[event.Name] = timer + d.mu.Unlock() + } + + timer.Reset(100 * time.Millisecond) } } } From e3c6903120031f30e6d3b555ac994aa5b85f4c92 Mon Sep 17 00:00:00 2001 From: arexon Date: Mon, 30 Sep 2024 00:23:46 +0300 Subject: [PATCH 08/17] Implement a lazy debouncer that ignores events after receiving an event --- regolith/watcher.go | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/regolith/watcher.go b/regolith/watcher.go index fe6d8939..00046449 100644 --- a/regolith/watcher.go +++ b/regolith/watcher.go @@ -1,9 +1,7 @@ package regolith import ( - "math" "path/filepath" - "sync" "time" "github.com/Bedrock-OSS/go-burrito/burrito" @@ -20,11 +18,10 @@ import ( type DirWatcher struct { watcher *fsnotify.Watcher root string - kind string // Whether the watched directory is "RP", "BP", or "data". - mu sync.Mutex // Used for locking during deboucning. - timers map[string]*time.Timer // Stores event path -> debounce timer. - interruption chan string // See RunContext. - errors chan error // See RunContext. + kind string // Whether the watched directory is "RP", "BP", or "data". + debounce <-chan time.Time // Debounce timer + interruption chan string // See RunContext. + errors chan error // See RunContext. } // NewDirWatcher creates a new directory watcher. @@ -43,7 +40,6 @@ func NewDirWatcher( watcher: watcher, root: root, kind: kind, - timers: make(map[string]*time.Timer), interruption: interruption, errors: errors, } @@ -60,7 +56,8 @@ func NewDirWatcher( // Start starts the file watching loop and blocks the goroutine until it // receives an event. Once it does, it sends a message to interruption channel -// then resumes blocking the goroutine. +// then creates a debounce timer of 100ms. In this duration, all events are +// silently ignored. func (d *DirWatcher) start() { for { select { @@ -74,24 +71,13 @@ func (d *DirWatcher) start() { if !ok { return } - if event.Op.Has(fsnotify.Chmod) { + if d.debounce != nil || event.Op.Has(fsnotify.Chmod) { continue } - - d.mu.Lock() - timer, exists := d.timers[event.Name] - d.mu.Unlock() - - if !exists { - timer = time.AfterFunc(math.MaxInt64, func() { d.interruption <- d.kind }) - timer.Stop() - - d.mu.Lock() - d.timers[event.Name] = timer - d.mu.Unlock() - } - - timer.Reset(100 * time.Millisecond) + d.interruption <- d.kind + d.debounce = time.After(100 * time.Millisecond) + case <-d.debounce: + d.debounce = nil } } } From 74b3e02ed574be6dce993ee4383d8c515f2602c8 Mon Sep 17 00:00:00 2001 From: arexon Date: Mon, 30 Sep 2024 00:24:06 +0300 Subject: [PATCH 09/17] Fix DirWatcher panic on Linux --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index c53d47b5..592de8cb 100644 --- a/go.mod +++ b/go.mod @@ -27,7 +27,7 @@ require ( cloud.google.com/go/iam v0.3.0 // indirect cloud.google.com/go/storage v1.21.0 // indirect github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20221202181307-76fa05c21b12 // indirect - github.com/arexon/fsnotify v0.0.0-20240925210538-c862c914df35 // indirect + github.com/arexon/fsnotify v0.0.0-20240929211932-1ebdc44d4bc2 // indirect github.com/aws/aws-sdk-go v1.43.25 // indirect github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect github.com/gammazero/deque v0.2.1 // indirect diff --git a/go.sum b/go.sum index 29ee03cb..949f62f7 100644 --- a/go.sum +++ b/go.sum @@ -67,8 +67,8 @@ github.com/alessio/shellescape v1.4.1/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPp github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20221202181307-76fa05c21b12 h1:npHgfD4Tl2WJS3AJaMUi5ynGDPUBfkg3U3fCzDyXZ+4= github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20221202181307-76fa05c21b12/go.mod h1:pSwJ0fSY5KhvocuWSx4fz3BA8OrA1bQn+K1Eli3BRwM= -github.com/arexon/fsnotify v0.0.0-20240925210538-c862c914df35 h1:LcWID+cU05U8/rovFXL+bB6eBhm4waCvzKCvaBHlJoo= -github.com/arexon/fsnotify v0.0.0-20240925210538-c862c914df35/go.mod h1:fMK1EJDCm6IfeqTBptyizpl356fZy33nWqFKELbFouQ= +github.com/arexon/fsnotify v0.0.0-20240929211932-1ebdc44d4bc2 h1:Y4fOHCKaIWeRXZ9+qqaB7UI0tjK/eMN6CZ9OCbY3FBY= +github.com/arexon/fsnotify v0.0.0-20240929211932-1ebdc44d4bc2/go.mod h1:fMK1EJDCm6IfeqTBptyizpl356fZy33nWqFKELbFouQ= github.com/arikkfir/go-getter v1.6.3-0.20220803164326-281b7670b734 h1:csFUhbcumnsC5d0SMF8CvtR6Z/i4UeNgOZ6xUaQUYas= github.com/arikkfir/go-getter v1.6.3-0.20220803164326-281b7670b734/go.mod h1:IZCrswsZPeWv9IkVnLElzRU/gz/QPi6pZHn4tv6vbwA= github.com/aws/aws-sdk-go v1.15.78/go.mod h1:E3/ieXAlvM0XWO57iftYVDLLvQ824smPP3ATZkfNZeM= From bd5b4e229b6a8b099f5cf1247fb2612ab613165c Mon Sep 17 00:00:00 2001 From: github-actions Date: Sun, 29 Sep 2024 21:24:45 +0000 Subject: [PATCH 10/17] Generated CREDITS.csv --- CREDITS.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CREDITS.csv b/CREDITS.csv index e1ac4e1f..fdbe7bf5 100644 --- a/CREDITS.csv +++ b/CREDITS.csv @@ -2,7 +2,7 @@ github.com/Bedrock-OSS/go-burrito/burrito,https://github.com/Bedrock-OSS/go-burr github.com/Bedrock-OSS/regolith,https://github.com/Bedrock-OSS/regolith/blob/HEAD/LICENSE,MIT github.com/alessio/shellescape,https://github.com/alessio/shellescape/blob/v1.4.1/LICENSE,MIT github.com/antlr/antlr4/runtime/Go/antlr/v4,https://github.com/antlr/antlr4/blob/76fa05c21b12/runtime/Go/antlr/v4/LICENSE,BSD-3-Clause -github.com/arexon/fsnotify,https://github.com/arexon/fsnotify/blob/c862c914df35/LICENSE,BSD-3-Clause +github.com/arexon/fsnotify,https://github.com/arexon/fsnotify/blob/1ebdc44d4bc2/LICENSE,BSD-3-Clause github.com/fatih/color,https://github.com/fatih/color/blob/v1.14.1/LICENSE.md,MIT github.com/gammazero/deque,https://github.com/gammazero/deque/blob/v0.2.1/LICENSE,MIT github.com/google/go-github/v39/github,https://github.com/google/go-github/blob/v39.2.0/LICENSE,BSD-3-Clause From 2ee43d6e096e41142df8a5d0f2ba2319825f6441 Mon Sep 17 00:00:00 2001 From: arexon Date: Sat, 12 Oct 2024 21:14:04 +0300 Subject: [PATCH 11/17] Fix file watcher not listening to events after `data` is copied/moved back from `tmp` --- regolith/export.go | 5 ++++ regolith/filter.go | 29 ++++++++++--------- regolith/filter_profile.go | 14 ++++----- regolith/main_functions.go | 18 ++++++------ regolith/watcher.go | 59 ++++++++++++++++++++++++++------------ 5 files changed, 77 insertions(+), 48 deletions(-) diff --git a/regolith/export.go b/regolith/export.go index c875d3a0..fa7e3d46 100644 --- a/regolith/export.go +++ b/regolith/export.go @@ -275,6 +275,7 @@ func ExportProject(ctx RunContext) error { MeasureEnd() // List the names of the filters that opt-in to the data export process var exportedFilterNames []string + shouldRestartWatchingData := false for filter := range profile.Filters { filter := profile.Filters[filter] usingDataPath, err := filter.IsUsingDataExport(dotRegolithPath, ctx) @@ -298,6 +299,7 @@ func ExportProject(ctx RunContext) error { } // Add the filter name to the list of paths to export exportedFilterNames = append(exportedFilterNames, filter.GetId()) + shouldRestartWatchingData = true } } // The root of the data path cannot be deleted because the @@ -364,6 +366,9 @@ func ExportProject(ctx RunContext) error { } return mainError } + if shouldRestartWatchingData { + ctx.shouldRestartWatchingData <- struct{}{} + } } MeasureStart("Export - MoveOrCopy") if IsExperimentEnabled(SizeTimeCheck) { diff --git a/regolith/filter.go b/regolith/filter.go index 25ebacfb..a621c789 100644 --- a/regolith/filter.go +++ b/regolith/filter.go @@ -23,16 +23,18 @@ type RunContext struct { DotRegolithPath string Settings map[string]interface{} - // interruptionChannel is a channel that is used to notify about changes + // interruption is a channel that is used to notify about changes // in the source files, in order to trigger a restart of the program in // the watch mode. The string send to the channel is the name of the source // of the change ("rp", "bp" or "data"), which may be used to handle // some interruptions differently. - interruptionChannel chan string + interruption chan string - // fileWatchingErrorChannel is used to pass any errors that may occur during + // fileWatchingError is used to pass any errors that may occur during // file watching. - fileWatchingErrorChannel chan error + fileWatchingError chan error + + shouldRestartWatchingData chan struct{} } // GetProfile returns the Profile structure from the context. @@ -48,7 +50,7 @@ func (c *RunContext) GetProfile() (Profile, error) { // IsInWatchMode returns a value that shows whether the context is in the // watch mode. func (c *RunContext) IsInWatchMode() bool { - return c.interruptionChannel == nil + return c.interruption == nil } // StartWatchingSourceFiles causes the Context to start goroutines that watch @@ -58,27 +60,28 @@ func (c *RunContext) StartWatchingSourceFiles() error { // closing the channels somewhere. Currently the watching goroutines yield // their messages until the end of the program. Sending to a closed channel // would cause panic. - if c.interruptionChannel != nil { + if c.interruption != nil { return burrito.WrappedError("Files are already being watched.") } - c.interruptionChannel = make(chan string) - c.fileWatchingErrorChannel = make(chan error) + c.interruption = make(chan string) + c.fileWatchingError = make(chan error) + c.shouldRestartWatchingData = make(chan struct{}) if c.Config.ResourceFolder != "" { - err := NewDirWatcher(c.Config.ResourceFolder, "rp", c.interruptionChannel, c.fileWatchingErrorChannel) + err := NewDirWatcher(c.Config.ResourceFolder, "rp", c.interruption, c.fileWatchingError, c.shouldRestartWatchingData) if err != nil { return burrito.WrapError(err, "Could not create resource pack watcher.") } } if c.Config.BehaviorFolder != "" { - err := NewDirWatcher(c.Config.BehaviorFolder, "bp", c.interruptionChannel, c.fileWatchingErrorChannel) + err := NewDirWatcher(c.Config.BehaviorFolder, "bp", c.interruption, c.fileWatchingError, c.shouldRestartWatchingData) if err != nil { return burrito.WrapError(err, "Could not create behavior pack watcher.") } } if c.Config.DataPath != "" { - err := NewDirWatcher(c.Config.DataPath, "data", c.interruptionChannel, c.fileWatchingErrorChannel) + err := NewDirWatcher(c.Config.DataPath, "data", c.interruption, c.fileWatchingError, c.shouldRestartWatchingData) if err != nil { return burrito.WrapError(err, "Could not create data watcher.") } @@ -91,11 +94,11 @@ func (c *RunContext) StartWatchingSourceFiles() error { // unless the source of the interruption is on the list of ignored sources. // This function does not block. func (c *RunContext) IsInterrupted(ignoredSource ...string) bool { - if c.interruptionChannel == nil { + if c.interruption == nil { return false } select { - case source := <-c.interruptionChannel: + case source := <-c.interruption: for _, ignored := range ignoredSource { if ignored == source { return false diff --git a/regolith/filter_profile.go b/regolith/filter_profile.go index 51777ee6..89f5167a 100644 --- a/regolith/filter_profile.go +++ b/regolith/filter_profile.go @@ -10,13 +10,13 @@ type ProfileFilter struct { func (f *ProfileFilter) Run(context RunContext) (bool, error) { Logger.Infof("Running %q nested profile...", f.Profile) return RunProfileImpl(RunContext{ - Profile: f.Profile, - AbsoluteLocation: context.AbsoluteLocation, - Config: context.Config, - Parent: &context, - interruptionChannel: context.interruptionChannel, - DotRegolithPath: context.DotRegolithPath, - Settings: f.Settings, + Profile: f.Profile, + AbsoluteLocation: context.AbsoluteLocation, + Config: context.Config, + Parent: &context, + interruption: context.interruption, + DotRegolithPath: context.DotRegolithPath, + Settings: f.Settings, }) } diff --git a/regolith/main_functions.go b/regolith/main_functions.go index 29d14470..43d5cd10 100644 --- a/regolith/main_functions.go +++ b/regolith/main_functions.go @@ -312,11 +312,11 @@ func Watch(profileName string, debug bool) error { } Logger.Info("Press Ctrl+C to stop watching.") select { - case <-context.interruptionChannel: + case <-context.interruption: // AwaitInterruption locks the goroutine with the interruption channel until // the Config is interrupted and returns the interruption message. Logger.Warn("Restarting...") - case err := <-context.fileWatchingErrorChannel: + case err := <-context.fileWatchingError: if err != nil { return burrito.WrapError(err, "Encountered an error during file watching") } @@ -380,13 +380,13 @@ func ApplyFilter(filterName string, filterArgs []string, debug bool) error { // Create run context path, _ := filepath.Abs(".") runContext := RunContext{ - Config: config, - Parent: nil, - Profile: "[dynamic profile]", - DotRegolithPath: dotRegolithPath, - interruptionChannel: nil, - AbsoluteLocation: path, - Settings: filterRunner.GetSettings(), + Config: config, + Parent: nil, + Profile: "[dynamic profile]", + DotRegolithPath: dotRegolithPath, + interruption: nil, + AbsoluteLocation: path, + Settings: filterRunner.GetSettings(), } // Check the filter err = filterRunner.Check(runContext) diff --git a/regolith/watcher.go b/regolith/watcher.go index 00046449..e4e364a9 100644 --- a/regolith/watcher.go +++ b/regolith/watcher.go @@ -16,12 +16,13 @@ import ( // // Fork patch: https://github.com/arexon/fsnotify/blob/main/fsnotify.go#L481 type DirWatcher struct { - watcher *fsnotify.Watcher - root string - kind string // Whether the watched directory is "RP", "BP", or "data". - debounce <-chan time.Time // Debounce timer - interruption chan string // See RunContext. - errors chan error // See RunContext. + watcher *fsnotify.Watcher + recursiveRoot string + kind string // Whether the watched directory is "RP", "BP", or "data". + debounce <-chan time.Time // Debounce timer + interruption chan string // See RunContext. + errors chan error // See RunContext. + shouldRestartData chan struct{} } // NewDirWatcher creates a new directory watcher. @@ -30,6 +31,7 @@ func NewDirWatcher( kind string, interruption chan string, errors chan error, + shouldRestartData chan struct{}, ) error { watcher, err := fsnotify.NewWatcher() if err != nil { @@ -37,27 +39,30 @@ func NewDirWatcher( } d := &DirWatcher{ - watcher: watcher, - root: root, - kind: kind, - interruption: interruption, - errors: errors, + watcher: watcher, + // We have to manually signal to fsnotify that it should recursively watch this + // path by using "/..." or "\...". + recursiveRoot: filepath.Join(root, "..."), + kind: kind, + interruption: interruption, + errors: errors, + shouldRestartData: shouldRestartData, } - // We have to manually signal to fsnotify that it should recursively watch this - // path by using "/..." or "\...". - recursiveRoot := filepath.Join(root, "...") - if err := d.watcher.Add(recursiveRoot); err != nil { - return burrito.WrapErrorf(err, "Could not start watching `%f`", root) + if err := d.watcher.Add(d.recursiveRoot); err != nil { + return burrito.WrapErrorf(err, "Could not start watching `%s`", root) } go d.start() return nil } // Start starts the file watching loop and blocks the goroutine until it -// receives an event. Once it does, it sends a message to interruption channel -// then creates a debounce timer of 100ms. In this duration, all events are -// silently ignored. +// receives either: +// - an event, in which it sends a message to interruption channel then +// creates a debounce timer of 100ms. In this duration, all events are +// silently ignored. +// - an restart channel signal, and if the watcher is for the "data" +// folder, it will restart watching the folder again. func (d *DirWatcher) start() { for { select { @@ -78,6 +83,22 @@ func (d *DirWatcher) start() { d.debounce = time.After(100 * time.Millisecond) case <-d.debounce: d.debounce = nil + case <-d.shouldRestartData: + if d.kind == "rp" || d.kind == "bp" { + // Basically "forward" the signal to another listener until it reaches the + // one for "data". + d.shouldRestartData <- struct{}{} + continue + } + d.watcher.Close() + watcher, err := fsnotify.NewWatcher() + if err != nil { + d.errors <- burrito.WrapError(err, "Could not begin restarting file watching for data folder") + } + d.watcher = watcher + if err := d.watcher.Add(d.recursiveRoot); err != nil { + d.errors <- burrito.WrapErrorf(err, "Could not start watching the data folder") + } } } } From 4124cb046de96a8dc04df4c1679ca1681122f4ad Mon Sep 17 00:00:00 2001 From: arexon Date: Sat, 12 Oct 2024 21:16:19 +0300 Subject: [PATCH 12/17] Fix typos in comment --- regolith/watcher.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/regolith/watcher.go b/regolith/watcher.go index e4e364a9..67b92bec 100644 --- a/regolith/watcher.go +++ b/regolith/watcher.go @@ -58,11 +58,11 @@ func NewDirWatcher( // Start starts the file watching loop and blocks the goroutine until it // receives either: -// - an event, in which it sends a message to interruption channel then -// creates a debounce timer of 100ms. In this duration, all events are +// - an event, in which it will send a message to interruption channel then +// create a debounce timer of 100ms. In this duration, all events are // silently ignored. -// - an restart channel signal, and if the watcher is for the "data" -// folder, it will restart watching the folder again. +// - a restart channel signal, and if the watcher is for the "data" folder, it +// will restart watching the folder again. func (d *DirWatcher) start() { for { select { From ad507c75becea16c7c559bebfa20a0a55b0e2a60 Mon Sep 17 00:00:00 2001 From: arexon Date: Mon, 14 Oct 2024 17:07:43 +0300 Subject: [PATCH 13/17] Use a single DirWatcher instance and restart upon exporting --- regolith/export.go | 5 -- regolith/filter.go | 31 ++++++------- regolith/profile.go | 6 +++ regolith/watcher.go | 108 +++++++++++++++++++++++++------------------- 4 files changed, 81 insertions(+), 69 deletions(-) diff --git a/regolith/export.go b/regolith/export.go index fa7e3d46..c875d3a0 100644 --- a/regolith/export.go +++ b/regolith/export.go @@ -275,7 +275,6 @@ func ExportProject(ctx RunContext) error { MeasureEnd() // List the names of the filters that opt-in to the data export process var exportedFilterNames []string - shouldRestartWatchingData := false for filter := range profile.Filters { filter := profile.Filters[filter] usingDataPath, err := filter.IsUsingDataExport(dotRegolithPath, ctx) @@ -299,7 +298,6 @@ func ExportProject(ctx RunContext) error { } // Add the filter name to the list of paths to export exportedFilterNames = append(exportedFilterNames, filter.GetId()) - shouldRestartWatchingData = true } } // The root of the data path cannot be deleted because the @@ -366,9 +364,6 @@ func ExportProject(ctx RunContext) error { } return mainError } - if shouldRestartWatchingData { - ctx.shouldRestartWatchingData <- struct{}{} - } } MeasureStart("Export - MoveOrCopy") if IsExperimentEnabled(SizeTimeCheck) { diff --git a/regolith/filter.go b/regolith/filter.go index a621c789..06dd0c49 100644 --- a/regolith/filter.go +++ b/regolith/filter.go @@ -1,6 +1,8 @@ package regolith -import "github.com/Bedrock-OSS/go-burrito/burrito" +import ( + "github.com/Bedrock-OSS/go-burrito/burrito" +) type FilterDefinition struct { Id string `json:"-"` @@ -34,7 +36,7 @@ type RunContext struct { // file watching. fileWatchingError chan error - shouldRestartWatchingData chan struct{} + fileWatchingStage chan string } // GetProfile returns the Profile structure from the context. @@ -50,7 +52,7 @@ func (c *RunContext) GetProfile() (Profile, error) { // IsInWatchMode returns a value that shows whether the context is in the // watch mode. func (c *RunContext) IsInWatchMode() bool { - return c.interruption == nil + return c.interruption != nil } // StartWatchingSourceFiles causes the Context to start goroutines that watch @@ -66,27 +68,22 @@ func (c *RunContext) StartWatchingSourceFiles() error { c.interruption = make(chan string) c.fileWatchingError = make(chan error) - c.shouldRestartWatchingData = make(chan struct{}) + c.fileWatchingStage = make(chan string) + var roots []string if c.Config.ResourceFolder != "" { - err := NewDirWatcher(c.Config.ResourceFolder, "rp", c.interruption, c.fileWatchingError, c.shouldRestartWatchingData) - if err != nil { - return burrito.WrapError(err, "Could not create resource pack watcher.") - } + roots = append(roots, c.Config.ResourceFolder) } if c.Config.BehaviorFolder != "" { - err := NewDirWatcher(c.Config.BehaviorFolder, "bp", c.interruption, c.fileWatchingError, c.shouldRestartWatchingData) - if err != nil { - return burrito.WrapError(err, "Could not create behavior pack watcher.") - } + roots = append(roots, c.Config.BehaviorFolder) } if c.Config.DataPath != "" { - err := NewDirWatcher(c.Config.DataPath, "data", c.interruption, c.fileWatchingError, c.shouldRestartWatchingData) - if err != nil { - return burrito.WrapError(err, "Could not create data watcher.") - } + roots = append(roots, c.Config.DataPath) + } + err := NewDirWatcher(roots, c.Config, c.interruption, c.fileWatchingError, c.fileWatchingStage) + if err != nil { + return err } - return nil } diff --git a/regolith/profile.go b/regolith/profile.go index d22b00f8..80a75970 100644 --- a/regolith/profile.go +++ b/regolith/profile.go @@ -142,10 +142,16 @@ start: // Export files Logger.Info("Moving files to target directory.") start := time.Now() + if context.IsInWatchMode() { + context.fileWatchingStage <- "pause" + } err = ExportProject(context) if err != nil { return burrito.WrapError(err, exportProjectError) } + if context.IsInWatchMode() { + context.fileWatchingStage <- "restart" + } if context.IsInterrupted("data") { goto start } diff --git a/regolith/watcher.go b/regolith/watcher.go index 67b92bec..9c58f9d4 100644 --- a/regolith/watcher.go +++ b/regolith/watcher.go @@ -2,6 +2,7 @@ package regolith import ( "path/filepath" + "strings" "time" "github.com/Bedrock-OSS/go-burrito/burrito" @@ -16,89 +17,102 @@ import ( // // Fork patch: https://github.com/arexon/fsnotify/blob/main/fsnotify.go#L481 type DirWatcher struct { - watcher *fsnotify.Watcher - recursiveRoot string - kind string // Whether the watched directory is "RP", "BP", or "data". - debounce <-chan time.Time // Debounce timer - interruption chan string // See RunContext. - errors chan error // See RunContext. - shouldRestartData chan struct{} + watcher *fsnotify.Watcher + roots []string + config *Config + debounce <-chan time.Time + interruption chan string + errors chan error + stage <-chan string } -// NewDirWatcher creates a new directory watcher. func NewDirWatcher( - root string, - kind string, + roots []string, + config *Config, interruption chan string, errors chan error, - shouldRestartData chan struct{}, + stage <-chan string, ) error { + d := &DirWatcher{ + roots: roots, + config: config, + interruption: interruption, + errors: errors, + stage: stage, + } + err := d.watch() + if err != nil { + return err + } + go d.start() + return nil +} + +func (d *DirWatcher) watch() error { watcher, err := fsnotify.NewWatcher() if err != nil { return burrito.WrapError(err, "Could not initialize directory watching") } - - d := &DirWatcher{ - watcher: watcher, + d.watcher = watcher + for _, root := range d.roots { // We have to manually signal to fsnotify that it should recursively watch this // path by using "/..." or "\...". - recursiveRoot: filepath.Join(root, "..."), - kind: kind, - interruption: interruption, - errors: errors, - shouldRestartData: shouldRestartData, - } - - if err := d.watcher.Add(d.recursiveRoot); err != nil { - return burrito.WrapErrorf(err, "Could not start watching `%s`", root) + recursiveRoot := filepath.Join(root, "...") + if err := d.watcher.Add(recursiveRoot); err != nil { + return burrito.WrapErrorf(err, "Could not start watching `%s`", root) + } } - go d.start() return nil } -// Start starts the file watching loop and blocks the goroutine until it -// receives either: -// - an event, in which it will send a message to interruption channel then -// create a debounce timer of 100ms. In this duration, all events are -// silently ignored. -// - a restart channel signal, and if the watcher is for the "data" folder, it -// will restart watching the folder again. func (d *DirWatcher) start() { + paused := false for { select { case err, ok := <-d.watcher.Errors: if !ok { + if paused { + continue + } return } d.errors <- err return case event, ok := <-d.watcher.Events: if !ok { + if paused { + continue + } return } if d.debounce != nil || event.Op.Has(fsnotify.Chmod) { continue } - d.interruption <- d.kind + if isInDir(event.Name, d.config.ResourceFolder) { + d.interruption <- "rp" + } else if isInDir(event.Name, d.config.BehaviorFolder) { + d.interruption <- "bp" + } else if isInDir(event.Name, d.config.DataPath) { + d.interruption <- "data" + } d.debounce = time.After(100 * time.Millisecond) case <-d.debounce: d.debounce = nil - case <-d.shouldRestartData: - if d.kind == "rp" || d.kind == "bp" { - // Basically "forward" the signal to another listener until it reaches the - // one for "data". - d.shouldRestartData <- struct{}{} - continue - } - d.watcher.Close() - watcher, err := fsnotify.NewWatcher() - if err != nil { - d.errors <- burrito.WrapError(err, "Could not begin restarting file watching for data folder") - } - d.watcher = watcher - if err := d.watcher.Add(d.recursiveRoot); err != nil { - d.errors <- burrito.WrapErrorf(err, "Could not start watching the data folder") + case stage := <-d.stage: + switch stage { + case "pause": + d.watcher.Close() + paused = true + case "restart": + if err := d.watch(); err != nil { + d.errors <- err + } + paused = false } } } } + +func isInDir(path string, root string) bool { + return strings.HasPrefix(path, filepath.Clean(root)) +} From 7e61890a1c4d259b3baf6dfab98aa7eb9c468b48 Mon Sep 17 00:00:00 2001 From: arexon Date: Mon, 14 Oct 2024 17:10:34 +0300 Subject: [PATCH 14/17] Remove redundant check Restarting the file watcher is done internally in DirWatcher now. --- regolith/filter.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/regolith/filter.go b/regolith/filter.go index 06dd0c49..4b9b53d4 100644 --- a/regolith/filter.go +++ b/regolith/filter.go @@ -58,14 +58,6 @@ func (c *RunContext) IsInWatchMode() bool { // StartWatchingSourceFiles causes the Context to start goroutines that watch // for changes in the source files and report that to the func (c *RunContext) StartWatchingSourceFiles() error { - // TODO - if you want to be able to restart the watcher, you need to handle - // closing the channels somewhere. Currently the watching goroutines yield - // their messages until the end of the program. Sending to a closed channel - // would cause panic. - if c.interruption != nil { - return burrito.WrappedError("Files are already being watched.") - } - c.interruption = make(chan string) c.fileWatchingError = make(chan error) c.fileWatchingStage = make(chan string) From 524aa9937312231c7da8ebf081d43a24ed9de4eb Mon Sep 17 00:00:00 2001 From: arexon Date: Mon, 14 Oct 2024 17:12:53 +0300 Subject: [PATCH 15/17] Handle setting `roots` up internally in DirWatcher --- regolith/filter.go | 13 +------------ regolith/watcher.go | 11 ++++++++++- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/regolith/filter.go b/regolith/filter.go index 4b9b53d4..95a435da 100644 --- a/regolith/filter.go +++ b/regolith/filter.go @@ -61,18 +61,7 @@ func (c *RunContext) StartWatchingSourceFiles() error { c.interruption = make(chan string) c.fileWatchingError = make(chan error) c.fileWatchingStage = make(chan string) - - var roots []string - if c.Config.ResourceFolder != "" { - roots = append(roots, c.Config.ResourceFolder) - } - if c.Config.BehaviorFolder != "" { - roots = append(roots, c.Config.BehaviorFolder) - } - if c.Config.DataPath != "" { - roots = append(roots, c.Config.DataPath) - } - err := NewDirWatcher(roots, c.Config, c.interruption, c.fileWatchingError, c.fileWatchingStage) + err := NewDirWatcher(c.Config, c.interruption, c.fileWatchingError, c.fileWatchingStage) if err != nil { return err } diff --git a/regolith/watcher.go b/regolith/watcher.go index 9c58f9d4..0fdd2bff 100644 --- a/regolith/watcher.go +++ b/regolith/watcher.go @@ -27,12 +27,21 @@ type DirWatcher struct { } func NewDirWatcher( - roots []string, config *Config, interruption chan string, errors chan error, stage <-chan string, ) error { + var roots []string + if config.ResourceFolder != "" { + roots = append(roots, config.ResourceFolder) + } + if config.BehaviorFolder != "" { + roots = append(roots, config.BehaviorFolder) + } + if config.DataPath != "" { + roots = append(roots, config.DataPath) + } d := &DirWatcher{ roots: roots, config: config, From 0646a67c692f38f44cce745aa8113825ed684b9f Mon Sep 17 00:00:00 2001 From: arexon Date: Mon, 14 Oct 2024 18:46:56 +0300 Subject: [PATCH 16/17] Update comment in DirWatcher --- regolith/watcher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/regolith/watcher.go b/regolith/watcher.go index 0fdd2bff..25dec9c7 100644 --- a/regolith/watcher.go +++ b/regolith/watcher.go @@ -9,7 +9,7 @@ import ( "github.com/arexon/fsnotify" ) -// DirWatcher handles watching for changes in a specific directory (e.g. RP). +// DirWatcher handles watching for changes in a multiple root directories. // // fsnotify doesn't *officially* support recursive file watching yet. Windows // and and Linux are supported, but not macOS. For now, this implementation uses From e66dcdb11a4a7d3be84572c27a73aababa3ee09f Mon Sep 17 00:00:00 2001 From: Nusiq Date: Mon, 14 Oct 2024 20:49:38 +0200 Subject: [PATCH 17/17] Preventing a potential issue of Regolith getting stuck in watch mode in case of failing to run the 'ExportProject' function. --- regolith/profile.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/regolith/profile.go b/regolith/profile.go index 80a75970..3517b11c 100644 --- a/regolith/profile.go +++ b/regolith/profile.go @@ -146,12 +146,14 @@ start: context.fileWatchingStage <- "pause" } err = ExportProject(context) - if err != nil { - return burrito.WrapError(err, exportProjectError) - } if context.IsInWatchMode() { + // We need to restart the watcher before error handling. See: + // https://github.com/Bedrock-OSS/regolith/pull/297#issuecomment-2411981894 context.fileWatchingStage <- "restart" } + if err != nil { + return burrito.WrapError(err, exportProjectError) + } if context.IsInterrupted("data") { goto start }