Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track files replaced by move #10

Merged
merged 4 commits into from
Jun 6, 2017
Merged

Conversation

dougkoch
Copy link
Member

Adds a watch back to any file that has been replaced by a move. We look for a fsnotify.Remove event and retry watcher.Add() until it returns an error code of 0. Once a max number of retries has been exceeded, it is assumed that the file was removed.

Fixes: #9

@bazel-io
Copy link
Member

Can one of the admins verify this patch?

@mprobst
Copy link
Contributor

mprobst commented May 31, 2017

Alternative approach: run the build, then drop and re-add all watches. This usually gives enough time for the file to re-appear.

@dougkoch
Copy link
Member Author

dougkoch commented Jun 5, 2017

@mprobst Turns out that a file overwritten by a mv re-appears quickly enough that a watch can be added as soon as the remove event is received. My retry loop (removed in my latest commit) was unnecessary.
@achew22 Ready for review

ibazel/main.go Outdated
@@ -59,6 +59,25 @@ ibazel build //path/to/my/buildable:target
`)
}

type SourceEventProcessor struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this into its own file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

ibazel/main.go Outdated
SourceFileWatcher *fsnotify.Watcher
}

func (s SourceEventProcessor) Listen() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a method that creates a SourceEventProcessor and creates the channel then calls

go s.Listen()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

ibazel/main.go Outdated
@@ -107,7 +132,7 @@ func main() {
switch state {
case WAIT:
select {
case <-sourceFileWatcher.Events:
case <-sourceFileEvents:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you refer to the channel as being attached to the object instead of in the function scope?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

@achew22 achew22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your submission!

@achew22 achew22 merged commit 331dfc8 into bazelbuild:master Jun 6, 2017
clintharrison pushed a commit to clintharrison/bazel-watcher that referenced this pull request Sep 28, 2018
This test reproduces the write behavior of vim with `backupcopy` set to
both `yes` and `no`. `yes` is the default on Linux, and bazelbuild#10 fixed this
case. Unforunately, on macOS, `yes` is not the default, and using `no`
will cause ibazel to stop watching the file after a single file save
when a kqueue-based file watcher is used.
clintharrison pushed a commit to clintharrison/bazel-watcher that referenced this pull request Sep 28, 2018
This test reproduces the write behavior of vim with `backupcopy` set to
both `yes` and `no`. `yes` is the default on Linux, and bazelbuild#10 fixed this
case. Unforunately, on macOS, `yes` is not the default, and using `no`
will cause ibazel to stop watching the file after a single file save
when a kqueue-based file watcher is used.
achew22 pushed a commit that referenced this pull request Sep 30, 2018
* Add test reproducing vim backupcopy=no and kqueue failure

This test reproduces the write behavior of vim with `backupcopy` set to
both `yes` and `no`. `yes` is the default on Linux, and #10 fixed this
case. Unforunately, on macOS, `yes` is not the default, and using `no`
will cause ibazel to stop watching the file after a single file save
when a kqueue-based file watcher is used.

* Readd files when they are moved to overcome limitations in kqueue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants