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

kqueue FileSystemWatcher doesn't handle subdirectories #1093

Closed
wants to merge 14 commits into from

Conversation

bratsche
Copy link
Contributor

@bratsche bratsche commented Jun 8, 2014

Reimplemented much of the kqueue-based file watcher so that watching subdirectories works.

https://bugzilla.xamarin.com/show_bug.cgi?id=16259

@migueldeicaza
Copy link
Contributor

Couple of comments:

  • Would be nice to remove the white space changes from the patch
  • Would be useful to have tests showing the fixed problem

@bratsche
Copy link
Contributor Author

bratsche commented Jun 9, 2014

Thanks @migueldeicaza. I have one other question: I'm calling fcntl in here, but I'm passing the F_GETPATH flag to it. This is a BSD-specific flag still (at least, it's on Mac and when I Googled I found it in a FreeBSD header as well). Is this something we'd want to put into the Mono.Posix Syscall class, or should it stay like this since it's not really portable?

@migueldeicaza
Copy link
Contributor

Leave it here, for two reasons:

(a) This is BSD specific, so it does not matter.
(b) We can not take a dependency on this assembly on Mono.Posix

@migueldeicaza
Copy link
Contributor

Any updates regarding adding test (if they only work on OSX for now, make them only work there) and the white space changes?

@migueldeicaza
Copy link
Contributor

Cody, any chance you can contribute the test suites as a Unit test?

@alexischr
Copy link
Contributor

@bratsche , the code doesn't seem to work in subdirectories that already exist (e.g. adding files to them does not trigger events). New subdirectories are correctly listened to. If you don't have time to look at it, give me a couple of hints and I'll try to fix it :)

@monoadmin
Copy link

Hello! I'm the build bot for the Mono project. I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done. Contributors can ignore this message.

@bratsche
Copy link
Contributor Author

@alexischr I finally got the creation/deletion tests working with that last commit. The main issue for the create test seemed to be that as soon as I created and setup the FSW I would create the file almost immediately and in most cases that was too soon. Thread.Sleep fixes it.

The delete test has the same issue and solution, but also needed to sleep between creating the file and setting up the FSW.

It sucks to have these sleeps in there slowing down our test suite, but I can't find a better solution. If you can, have at it. :)

@bratsche
Copy link
Contributor Author

@alexischr I added a test for changing files now. It works, but it's using Process.Start ("touch", path) so it's not portable. I couldn't get it to work any other way (the other ways I tried are commented out below). Maybe I was doing something wrong, or maybe there's another bug in StreamWriter or something? If you have any suggestions on this test I would appreciate them.

@bratsche
Copy link
Contributor Author

This is on master now.

@bratsche bratsche closed this Oct 28, 2014
AlexKnauth pushed a commit to AlexKnauth/mono that referenced this pull request Nov 2, 2023
…-unity-vprintf-redirect

Add ability to redirect mono stdout logging to unity's vprintf function.
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.

None yet

4 participants