diff --git a/.gitattributes b/.gitattributes index 32f1001b..33380d70 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1 +1,2 @@ go.sum linguist-generated +*.go text eol=lf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..6578a9e5 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,47 @@ +name: CI +on: + push: + branches: + - preveil + pull_request: + +jobs: + + test: + name: Unit tests + + runs-on: windows-latest + + steps: + - name: Set up Go 1.13 + uses: actions/setup-go@v2 + with: + go-version: 1.13 + + - name: Check out code into the Go module directory + if: github.event_name == 'push' + uses: actions/checkout@v2 + + - name: Checkout pr's head commit + if: github.event_name == 'pull_request' + uses: actions/checkout@v2 + with: + # on pull_request, we don't want to build on + # the merged commit + ref: ${{ github.event.pull_request.head.sha }} + + - name: Run unit tests + run: make test + + golangci: + name: Linter + runs-on: windows-latest + steps: + - uses: actions/checkout@v2 + - name: golangci-lint + uses: golangci/golangci-lint-action@v2 + with: + # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version + version: v1.40.0 + + args: --config golangci.yml ./... diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..85bdb41f --- /dev/null +++ b/Makefile @@ -0,0 +1,57 @@ +define USAGE +USAGE: +> make [ + test: run all unit tests + test-unit: run all unit tests + lint: run golangci-linter + + clean: clean test cache and other deps + + help: print this help message +] +endef +export USAGE + +SHELL := /bin/bash + +BIN := $(PWD)/bin +GO := go +GOTEST := $(BIN)/gotest +LINT := $(BIN)/golangci-lint + +export PATH := $(PATH):$(BIN) + +help: + @echo "$$USAGE" + +usage: + @echo "$$USAGE" + +sense: + @echo "$$USAGE" + +install-dep-tools: + GOBIN=$(BIN) $(GO) get -u github.com/rakyll/gotest + @make install-golangci-lint + +install-golangci-lint: + # binary will be $(BIN) + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(BIN) v1.40.0 + +test: test-unit + +test-unit: install-dep-tools testclean + $(GOTEST) ./... -v -race + +testclean: + $(GO) clean -testcache || true + +clean: testclean + rm -rf $(BIN)/* + $(GO) clean -cache + $(GO) clean -modcache + +lint: install-dep-tools + $(LINT) run --config golangci.yml ./... + +ci: test lint diff --git a/README.md b/README.md index b2629e52..5922b114 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,5 @@ +![](https://github.com/PreVeil/fsnotify/workflows/CI/badge.svg) + # File system notifications for Go [![GoDoc](https://godoc.org/github.com/fsnotify/fsnotify?status.svg)](https://godoc.org/github.com/fsnotify/fsnotify) [![Go Report Card](https://goreportcard.com/badge/github.com/fsnotify/fsnotify)](https://goreportcard.com/report/github.com/fsnotify/fsnotify) @@ -27,7 +29,7 @@ Please see [the documentation](https://godoc.org/github.com/fsnotify/fsnotify) a ## API stability -fsnotify is a fork of [howeyc/fsnotify](https://godoc.org/github.com/howeyc/fsnotify) with a new API as of v1.0. The API is based on [this design document](http://goo.gl/MrYxyA). +fsnotify is a fork of [howeyc/fsnotify](https://godoc.org/github.com/howeyc/fsnotify) with a new API as of v1.0. The API is based on [this design document](http://goo.gl/MrYxyA). All [releases](https://github.com/fsnotify/fsnotify/releases) are tagged based on [Semantic Versioning](http://semver.org/). Further API changes are [planned](https://github.com/fsnotify/fsnotify/milestones), and will be tagged with a new major revision number. diff --git a/fsnotify.go b/fsnotify.go index 89cab046..e5b57f0e 100644 --- a/fsnotify.go +++ b/fsnotify.go @@ -15,8 +15,10 @@ import ( // Event represents a single file system notification. type Event struct { - Name string // Relative path to the file or directory. - Op Op // File operation that triggered the event. + Name string // Relative path to the file or directory. + Op Op // File operation that triggered the event. + OldName string + ID uint64 } // Op describes a set of file operations. @@ -59,7 +61,11 @@ func (op Op) String() string { // String returns a string representation of the event in the form // "file: REMOVE|WRITE|..." func (e Event) String() string { - return fmt.Sprintf("%q: %s", e.Name, e.Op.String()) + return fmt.Sprintf("newpath=%s, oldpath=%s, op=%s, eventID=%d", e.Name, e.OldName, e.Op.String(), e.ID) +} + +func (e Event) GetPath() string { + return e.Name } // Common errors that can be reported by a watcher diff --git a/fsnotify_test.go b/fsnotify_test.go index f9771d9d..d90b18de 100644 --- a/fsnotify_test.go +++ b/fsnotify_test.go @@ -7,23 +7,31 @@ package fsnotify import ( + "fmt" "os" "testing" "time" ) func TestEventStringWithValue(t *testing.T) { + formatEvent := func(name, oldName string, op Op, id uint64) string { + return fmt.Sprintf("newpath=%s, oldpath=%s, op=%s, eventID=%d", name, oldName, op.String(), id) + } for opMask, expectedString := range map[Op]string{ - Chmod | Create: `"/usr/someFile": CREATE|CHMOD`, - Rename: `"/usr/someFile": RENAME`, - Remove: `"/usr/someFile": REMOVE`, - Write | Chmod: `"/usr/someFile": WRITE|CHMOD`, + Chmod | Create: formatEvent("/usr/someFile", "", Create|Chmod, 0), + Rename: formatEvent("/usr/someFile", "", Rename, 0), + Remove: formatEvent("/usr/someFile", "", Remove, 0), + Write | Chmod: formatEvent("/usr/someFile", "", Write|Chmod, 0), } { - event := Event{Name: "/usr/someFile", Op: opMask} + event := Event{Name: "/usr/someFile", Op: opMask, OldName: "", ID: 0} if event.String() != expectedString { t.Fatalf("Expected %s, got: %v", expectedString, event.String()) } - + } + renameEvent := Event{Name: "/usr/someFile_Rename", Op: Rename, OldName: "/usr/someFile", ID: 123} + renameEventExpectedString := formatEvent("/usr/someFile_Rename", "/usr/someFile", Rename, 123) + if renameEvent.String() != renameEventExpectedString { + t.Fatalf("Expected %s, got: %v", renameEventExpectedString, renameEvent.String()) } } diff --git a/golangci.yml b/golangci.yml new file mode 100644 index 00000000..e034f9d9 --- /dev/null +++ b/golangci.yml @@ -0,0 +1,23 @@ +run: + deadline: 2m + +# all available settings of specific linters +linters-settings: + nolintlint: + # Enable to require an explanation of nonzero length after each nolint directive. Default is false. + require-explanation: true + +linters: + disable-all: true + enable: + - gofmt + - goimports + - gosimple + - ineffassign + - misspell + - govet + - nolintlint + - whitespace + - dupl + - rowserrcheck + - unparam diff --git a/integration_test.go b/integration_test.go index 7096344d..3064fe62 100644 --- a/integration_test.go +++ b/integration_test.go @@ -13,6 +13,7 @@ import ( "path" "path/filepath" "runtime" + "strconv" "sync/atomic" "testing" "time" @@ -73,13 +74,6 @@ func addWatch(t *testing.T, watcher *Watcher, dir string) { func TestFsnotifyMultipleOperations(t *testing.T) { watcher := newWatcher(t) - // Receive errors on the error channel on a separate goroutine - go func() { - for err := range watcher.Errors { - t.Fatalf("error received: %s", err) - } - }() - // Create directory to watch testDir := tempMkdir(t) defer os.RemoveAll(testDir) @@ -185,6 +179,10 @@ func TestFsnotifyMultipleOperations(t *testing.T) { t.Log("event channel closed") case <-time.After(2 * time.Second): t.Fatal("event stream was not closed after 2 seconds") + case err = <-watcher.Errors: + if err != nil { + t.Fatalf("watcher error received: %s", err) + } } } @@ -633,6 +631,182 @@ func TestFsnotifyRename(t *testing.T) { t.Fatal("fsnotify rename events have not been received after 500 ms") } + // Try closing the fsnotify instance + t.Log("calling Close()") + watcher.Close() + t.Log("waiting for the event channel to become closed...") + select { + case <-done: + t.Log("event channel closed") + case <-time.After(2 * time.Second): + t.Fatal("event stream was not closed after 2 seconds") + } + os.Remove(testFileRenamed) +} + +func TestFsnotifyMultipleRenames(t *testing.T) { + watcher := newWatcher(t) + + // Create directory to watch + testDir := tempMkdir(t) + defer os.RemoveAll(testDir) + + addWatch(t, watcher, testDir) + + // Receive errors on the error channel on a separate goroutine + go func() { + for err := range watcher.Errors { + t.Errorf("error received: %s", err) + } + }() + + // Receive events on the event channel on a separate goroutine + eventstream := watcher.Events + var renameReceived counter + done := make(chan bool) + go func() { + for event := range eventstream { + // Only count rename events + // Checks if the oldname attribute is set up accordingly + if event.Op&Rename == Rename { + newName, err := filepath.Rel(testDir, event.Name) + if err != nil { + t.Errorf("error received: %s", err) + } + oldName, err := filepath.Rel(testDir, event.OldName) + if err != nil { + t.Errorf("error received: %s", err) + } + if newName != oldName+"Renamed" { + t.Errorf("rename order messed up: %s", event) + } + renameReceived.increment() + } + } + done <- true + }() + + // Define rename action : create a file and rename it + // This should add at least one event to the fsnotify event queue + numberOfFiles := 100 + testFileNameCommon := "TestFsnotifyEvents.testfile" + renameAction := func(t *testing.T, count string) { + testFile := filepath.Join(testDir, testFileNameCommon+count) + f, err := os.OpenFile(testFile, os.O_WRONLY|os.O_CREATE, 0666) + if err != nil { + t.Fatalf("creating test file failed: %s", err) + } + f.Close() + + addWatch(t, watcher, testFile) + + testFileRenamed := filepath.Join(testDir, testFileNameCommon+count+"Renamed") + if err := testRename(testFile, testFileRenamed); err != nil { + t.Fatalf("rename failed: %s", err) + } + } + + // Creates #numberofFiles files and rename them + // Avoid buffer overflow by adding a short wait time + for i := 0; i < numberOfFiles; i++ { + go renameAction(t, strconv.Itoa(i)) + time.Sleep(5 * time.Millisecond) + } + // We expect this event to be received almost immediately, but let's wait 1000 ms to be sure + time.Sleep(1000 * time.Millisecond) + if renameReceived.value() == 0 { + t.Fatal("fsnotify rename events have not been received after 500 ms") + } + if renameReceived.value() != int32(numberOfFiles) { + t.Fatal("watcher missed a fsnotify rename event") + } + + // Try closing the fsnotify instance + t.Log("calling Close()") + watcher.Close() + t.Log("waiting for the event channel to become closed...") + select { + case <-done: + t.Log("event channel closed") + case <-time.After(2 * time.Second): + t.Fatal("event stream was not closed after 2 seconds") + } + for i := 0; i < numberOfFiles; i++ { + testFileRenamed := filepath.Join(testDir, testFileNameCommon+strconv.Itoa(i)+"Renamed") + os.Remove(testFileRenamed) + } +} + +// This test checks if the OldName attribute is set up for a rename event. +func TestFsnotifyRenameEventAttributes(t *testing.T) { + watcher := newWatcher(t) + + // Create directory to watch + testDir := tempMkdir(t) + defer os.RemoveAll(testDir) + + addWatch(t, watcher, testDir) + + // Receive errors on the error channel on a separate goroutine + go func() { + for err := range watcher.Errors { + t.Errorf("error received: %s", err) + } + }() + + testFile := filepath.Join(testDir, "TestFsnotifyEvents.testfile") + testFileRenamed := filepath.Join(testDir, "TestFsnotifyEvents.testfileRenamed") + + // Receive events on the event channel on a separate goroutine + eventstream := watcher.Events + var renameReceived counter + done := make(chan bool) + go func() { + for event := range eventstream { + // Only count relevant events + if event.Name == filepath.Clean(testDir) || event.Name == filepath.Clean(testFile) || event.Name == filepath.Clean(testFileRenamed) { + if event.Op&Rename == Rename { + renameReceived.increment() + } + t.Logf("event received: %s", event) + if event.Name == filepath.Clean(testFile) { + if event.OldName != filepath.Clean(testFileRenamed) { + t.Logf("unexpected old name for a file in rename event: %s", event) + } + } + } else { + t.Logf("unexpected event received: %s", event) + } + } + done <- true + }() + + // Create a file + // This should add at least one event to the fsnotify event queue + var f *os.File + f, err := os.OpenFile(testFile, os.O_WRONLY|os.O_CREATE, 0666) + if err != nil { + t.Fatalf("creating test file failed: %s", err) + } + f.Sync() + + f.WriteString("data") + f.Sync() + f.Close() + + // Add a watch for testFile + addWatch(t, watcher, testFile) + + if err := testRename(testFile, testFileRenamed); err != nil { + t.Fatalf("rename failed: %s", err) + } + + // We expect this event to be received almost immediately, but let's wait 500 ms to be sure + time.Sleep(500 * time.Millisecond) + if renameReceived.value() == 0 { + t.Fatal("fsnotify rename events have not been received after 500 ms") + } + // Try closing the fsnotify instance t.Log("calling Close()") watcher.Close() @@ -836,15 +1010,6 @@ func TestRemovalOfWatch(t *testing.T) { t.Fatalf("Could not remove the watch: %v\n", err) } - go func() { - select { - case ev := <-watcher.Events: - t.Fatalf("We received event: %v\n", ev) - case <-time.After(500 * time.Millisecond): - t.Log("No event received, as expected.") - } - }() - time.Sleep(200 * time.Millisecond) // Modify the file outside of the watched dir f, err := os.Open(testFileAlreadyExists) @@ -858,6 +1023,16 @@ func TestRemovalOfWatch(t *testing.T) { t.Fatalf("chmod failed: %s", err) } time.Sleep(400 * time.Millisecond) + + for { + select { + case ev := <-watcher.Events: + t.Fatalf("We received event: %v\n", ev) + case <-time.After(500 * time.Millisecond): + t.Log("No event received, as expected.") + return + } + } } func TestFsnotifyAttrib(t *testing.T) { diff --git a/windows.go b/windows.go index e3536e4b..d3d43703 100644 --- a/windows.go +++ b/windows.go @@ -15,6 +15,7 @@ import ( "runtime" "sync" "syscall" + "time" "unsafe" ) @@ -120,9 +121,21 @@ const ( sysFSQOVERFLOW = 0x4000 ) -func newEvent(name string, mask uint32) Event { - e := Event{Name: name} - if mask&sysFSCREATE == sysFSCREATE || mask&sysFSMOVEDTO == sysFSMOVEDTO { +func newEventID() uint64 { + return uint64(time.Now().UnixNano()) +} + +func newEvent(name string, mask uint32, oldName string) Event { + e := Event{ + Name: name, + OldName: oldName, + ID: newEventID(), + } + + if mask&sysFSMOVEDFROM == sysFSMOVEDFROM || mask&sysFSMOVEDTO == sysFSMOVEDTO || mask&sysFSMOVE == sysFSMOVE || mask&sysFSMOVESELF == sysFSMOVESELF { + e.Op |= Rename + } + if mask&sysFSCREATE == sysFSCREATE { e.Op |= Create } if mask&sysFSDELETE == sysFSDELETE || mask&sysFSDELETESELF == sysFSDELETESELF { @@ -131,9 +144,6 @@ func newEvent(name string, mask uint32) Event { if mask&sysFSMODIFY == sysFSMODIFY { e.Op |= Write } - if mask&sysFSMOVE == sysFSMOVE || mask&sysFSMOVESELF == sysFSMOVESELF || mask&sysFSMOVEDFROM == sysFSMOVEDFROM { - e.Op |= Rename - } if mask&sysFSATTRIB == sysFSATTRIB { e.Op |= Chmod } @@ -184,9 +194,13 @@ func (w *Watcher) wakeupReader() error { } func getDir(pathname string) (dir string, err error) { - attr, e := syscall.GetFileAttributes(syscall.StringToUTF16Ptr(pathname)) - if e != nil { - return "", os.NewSyscallError("GetFileAttributes", e) + name, err := syscall.UTF16PtrFromString(pathname) + if err != nil { + return "", err + } + attr, err := syscall.GetFileAttributes(name) + if err != nil { + return "", os.NewSyscallError("GetFileAttributes", err) } if attr&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 { dir = pathname @@ -198,7 +212,11 @@ func getDir(pathname string) (dir string, err error) { } func getIno(path string) (ino *inode, err error) { - h, e := syscall.CreateFile(syscall.StringToUTF16Ptr(path), + name, err := syscall.UTF16PtrFromString(path) + if err != nil { + return nil, err + } + h, e := syscall.CreateFile(name, syscall.FILE_LIST_DIRECTORY, syscall.FILE_SHARE_READ|syscall.FILE_SHARE_WRITE|syscall.FILE_SHARE_DELETE, nil, syscall.OPEN_EXISTING, @@ -303,11 +321,11 @@ func (w *Watcher) remWatch(pathname string) error { return fmt.Errorf("can't remove non-existent watch for: %s", pathname) } if pathname == dir { - w.sendEvent(watch.path, watch.mask&sysFSIGNORED) + w.sendEvent(watch.path, watch.mask&sysFSIGNORED, "") watch.mask = 0 } else { name := filepath.Base(pathname) - w.sendEvent(filepath.Join(watch.path, name), watch.names[name]&sysFSIGNORED) + w.sendEvent(filepath.Join(watch.path, name), watch.names[name]&sysFSIGNORED, "") delete(watch.names, name) } return w.startRead(watch) @@ -317,13 +335,13 @@ func (w *Watcher) remWatch(pathname string) error { func (w *Watcher) deleteWatch(watch *watch) { for name, mask := range watch.names { if mask&provisional == 0 { - w.sendEvent(filepath.Join(watch.path, name), mask&sysFSIGNORED) + w.sendEvent(filepath.Join(watch.path, name), mask&sysFSIGNORED, "") } delete(watch.names, name) } if watch.mask != 0 { if watch.mask&provisional == 0 { - w.sendEvent(watch.path, watch.mask&sysFSIGNORED) + w.sendEvent(watch.path, watch.mask&sysFSIGNORED, "") } watch.mask = 0 } @@ -356,7 +374,7 @@ func (w *Watcher) startRead(watch *watch) error { err := os.NewSyscallError("ReadDirectoryChanges", e) if e == syscall.ERROR_ACCESS_DENIED && watch.mask&provisional == 0 { // Watched directory was probably removed - if w.sendEvent(watch.path, watch.mask&sysFSDELETESELF) { + if w.sendEvent(watch.path, watch.mask&sysFSDELETESELF, "") { if watch.mask&sysFSONESHOT != 0 { watch.mask = 0 } @@ -420,6 +438,7 @@ func (w *Watcher) readEvents() { } switch e { + case nil: case syscall.ERROR_MORE_DATA: if watch == nil { w.Errors <- errors.New("ERROR_MORE_DATA has unexpectedly null lpOverlapped buffer") @@ -431,7 +450,7 @@ func (w *Watcher) readEvents() { } case syscall.ERROR_ACCESS_DENIED: // Watched directory was probably removed - w.sendEvent(watch.path, watch.mask&sysFSDELETESELF) + w.sendEvent(watch.path, watch.mask&sysFSDELETESELF, "") w.deleteWatch(watch) w.startRead(watch) continue @@ -441,21 +460,18 @@ func (w *Watcher) readEvents() { default: w.Errors <- os.NewSyscallError("GetQueuedCompletionPort", e) continue - case nil: } var offset uint32 for { if n == 0 { - w.Events <- newEvent("", sysFSQOVERFLOW) + w.Events <- newEvent("", sysFSQOVERFLOW, "") w.Errors <- errors.New("short read in readEvents()") break } // Point "raw" to the event in the buffer raw := (*syscall.FileNotifyInformation)(unsafe.Pointer(&watch.buf[offset])) - //buf := (*[syscall.MAX_PATH]uint16)(unsafe.Pointer(&raw.FileName)) - //name := syscall.UTF16ToString(buf[:raw.FileNameLength/2]) // https://stackoverflow.com/questions/51187973/how-to-create-an-array-or-a-slice-from-an-array-unsafe-pointer-in-golang // instead of using a fixed syscall.MAX_PATH buf, we create a buf that is the size of the path name @@ -468,7 +484,6 @@ func (w *Watcher) readEvents() { name := syscall.UTF16ToString(buf) fullname := filepath.Join(watch.path, name) - var mask uint64 switch raw.Action { case syscall.FILE_ACTION_REMOVED: @@ -476,7 +491,7 @@ func (w *Watcher) readEvents() { case syscall.FILE_ACTION_MODIFIED: mask = sysFSMODIFY case syscall.FILE_ACTION_RENAMED_OLD_NAME: - watch.rename = name + watch.rename = fullname case syscall.FILE_ACTION_RENAMED_NEW_NAME: if watch.names[watch.rename] != 0 { watch.names[name] |= watch.names[watch.rename] @@ -484,29 +499,40 @@ func (w *Watcher) readEvents() { mask = sysFSMOVESELF } } - - sendNameEvent := func() { - if w.sendEvent(fullname, watch.names[name]&mask) { + sendNameEvent := func(isRenameToEvent bool) { + if !isRenameToEvent && watch.rename != "" { + if w.sendEvent("", watch.names[name]&mask, watch.rename) { + if watch.names[name]&sysFSONESHOT != 0 { + delete(watch.names, name) + } + watch.rename = "" + } + } + if w.sendEvent(fullname, watch.names[name]&mask, watch.rename) { if watch.names[name]&sysFSONESHOT != 0 { delete(watch.names, name) } } + watch.rename = "" } - if raw.Action != syscall.FILE_ACTION_RENAMED_NEW_NAME { - sendNameEvent() - } - if raw.Action == syscall.FILE_ACTION_REMOVED { - w.sendEvent(fullname, watch.names[name]&sysFSIGNORED) - delete(watch.names, name) - } - if w.sendEvent(fullname, watch.mask&toFSnotifyFlags(raw.Action)) { - if watch.mask&sysFSONESHOT != 0 { - watch.mask = 0 + + if raw.Action != syscall.FILE_ACTION_RENAMED_OLD_NAME { + if raw.Action != syscall.FILE_ACTION_RENAMED_NEW_NAME { + sendNameEvent(false) + } + if raw.Action == syscall.FILE_ACTION_REMOVED { + w.sendEvent(fullname, watch.names[name]&sysFSIGNORED, "") + delete(watch.names, name) + } + if w.sendEvent(fullname, watch.mask&toFSnotifyFlags(raw.Action), watch.rename) { + if watch.mask&sysFSONESHOT != 0 { + watch.mask = 0 + } + } + if raw.Action == syscall.FILE_ACTION_RENAMED_NEW_NAME { + fullname = filepath.Join(watch.path, name) + sendNameEvent(true) } - } - if raw.Action == syscall.FILE_ACTION_RENAMED_NEW_NAME { - fullname = filepath.Join(watch.path, watch.rename) - sendNameEvent() } // Move to the next event in the buffer @@ -528,11 +554,11 @@ func (w *Watcher) readEvents() { } } -func (w *Watcher) sendEvent(name string, mask uint64) bool { +func (w *Watcher) sendEvent(name string, mask uint64, oldName string) bool { if mask == 0 { return false } - event := newEvent(name, uint32(mask)) + event := newEvent(name, uint32(mask), oldName) select { case ch := <-w.quit: w.quit <- ch