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

FileWatching: fix API problems with FDWatcher #42926

Merged
merged 1 commit into from
Nov 8, 2021
Merged

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Nov 3, 2021

  • add remaining FDEvents, and modernize this struct
  • add isopen function
  • do not deliver events after the FDWatcher is closed
  • do not delete events that have not been delivered
  • support running of these tests with Revise.jl
  • some threading problems still exist (where marked)

* add remaining FDEvents, and modernize this struct
* add `isopen` function
* do not deliver events after the FDWatcher is closed
* do not delete events that have not been delivered
* support running of these tests with Revise.jl
* some threading problems still exist (where marked)
Copy link
Member

@StefanKarpinski StefanKarpinski left a comment

Choose a reason for hiding this comment

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

My only comment is thanks for doing this, @vtjnash. Otherwise I'm not really qualified to review.

@vtjnash vtjnash merged commit 2539a67 into master Nov 8, 2021
@vtjnash vtjnash deleted the jn/FileWatcher-fixes branch November 8, 2021 19:43
vtjnash added a commit that referenced this pull request Nov 8, 2021
@vtjnash
Copy link
Member Author

vtjnash commented Nov 8, 2021

Whoops, seem to have a typo here, which will be fixed by 067ac5e (currently in #42962)

@DilumAluthge
Copy link
Member

DilumAluthge commented Nov 9, 2021

Any chance you could cherry-pick that fix into its own PR that we could merge quickly? I'm seeing most (if not all) CI jobs failing on master with Unhandled Task ERROR: type FDWatcher has no field notify, which I'm assuming is caused by this PR.

DilumAluthge pushed a commit that referenced this pull request Nov 9, 2021
@DilumAluthge
Copy link
Member

Any chance you could cherry-pick that fix into its own PR that we could merge quickly? I'm seeing most (if not all) CI jobs failing on master with Unhandled Task ERROR: type FDWatcher has no field notify, which I'm assuming is caused by this PR.

#43010

@tkf
Copy link
Member

tkf commented Nov 9, 2021

I get this error too when trying Pkg.precompile in 1.8.0-DEV.927:

(@v1.8) pkg> precompile
Unhandled Task ERROR: type FDWatcher has no field notify

aviatesk pushed a commit that referenced this pull request Nov 9, 2021
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
* add remaining FDEvents, and modernize this struct
* add `isopen` function
* do not deliver events after the FDWatcher is closed
* do not delete events that have not been delivered
* support running of these tests with Revise.jl
* note: some threading problems still exist (where marked)
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
* add remaining FDEvents, and modernize this struct
* add `isopen` function
* do not deliver events after the FDWatcher is closed
* do not delete events that have not been delivered
* support running of these tests with Revise.jl
* note: some threading problems still exist (where marked)
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
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