habitat
habitat copied to clipboard
Bump notify from 4.0.17 to 5.0.0
Bumps notify from 4.0.17 to 5.0.0.
Release notes
Sourced from notify's releases.
notify 5.0.0
5.0.0 (2022-08-28)
For a list of changes when upgrading from v4 see https://github.com/notify-rs/notify/blob/HEAD/UPGRADING_V4_TO_V5.md.
Differences to 5.0.0-pre.16:
- FIX: update minimum walkdir version to 2.2.2 #[432]
- CHANGE: add
need_rescan
function toEvent
, allowing easier detection when a rescan is required #435- FIX: debouncer-mini: change crossbeam feature to
crossbeam
, to allow passthrough with notify re-exports #429- DOCS: improve v5-to-v5 upgrade docs #431
- DOCS: file back v4 changelog into main #437
- DOCS: cleanups and link fixes
#431: notify-rs/notify#431 #432: notify-rs/notify#432 #437: notify-rs/notify#437 #435: notify-rs/notify#435 #429: notify-rs/notify#429
Changelog
Sourced from notify's changelog.
notify 5.0.0 (2022-08-28)
For a list of changes when upgrading from v4 see https://github.com/notify-rs/notify/blob/main/UPGRADING_V4_TO_V5.md.
Differences to 5.0.0-pre.16:
- FIX: update minimum walkdir version to 2.2.2 #[432]
- CHANGE: add
need_rescan
function toEvent
, allowing easier detection when a rescan is required #435- FIX: debouncer-mini: change crossbeam feature to
crossbeam
, to allow passthrough with notify re-exports #429- DOCS: improve v5-to-v5 upgrade docs #431
- DOCS: file back v4 changelog into main #437
- DOCS: cleanups and link fixes
#431: notify-rs/notify#431 #432: notify-rs/notify#432 #437: notify-rs/notify#437 #435: notify-rs/notify#435 #429: notify-rs/notify#429
5.0.0-pre.16 (2022-08-12)
- CHANGE: require config for watcher creation and unify config #426
- CHANGE: fsevent: use RenameMode::Any for renaming events #371
- FEATURE: re-add debouncer as new crate and fixup CI #286
- FEATURE: allow disabling crossbeam-channel dependency #425
- FIX: PollWatcher panic after delete-and-recreate #406
- MISC: rework pollwatcher internally #409
- DOCS: cleanup all docs towards v5 #395
#395: notify-rs/notify#395 #406: notify-rs/notify#406 #409: notify-rs/notify#409 #425: notify-rs/notify#425 #286: notify-rs/notify#286 #426: notify-rs/notify#426 #371: notify-rs/notify#371
5.0.0-pre.15 (2022-04-30)
- CHANGE: raise MSRV to 1.56! #396 and #402
- FEATURE: add support for pseudo filesystems like sysfs/procfs #396
- FIX: Fix builds on (Free)BSD due to changes in kqueue fix release #399
#396: notify-rs/notify#396 #399: notify-rs/notify#399 #402: notify-rs/notify#402
5.0.0-pre.14 (2022-03-13)
- CHANGE: upgrade mio to 0.8 #386
... (truncated)
Commits
d985ae1
prepare 5.0.0a83279f
improve upgrade docs and fix Config link65da37b
file back v4 history into changelog1fbf8fa
add accessor for whether rescan is required on Event17580f6
fixup rebase gunk54465e9
fixup optional crossbeam feature selection in debouncer-mini94f1680
update minimum walkdir version to 2.2.2 (#432)4d16a54
fixup doc links post initial debouncer-mini released698f90
fixup moved readme due to reorg2f91e15
fix typo in readme- Additional commits viewable in compare view
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase
.
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
-
@dependabot rebase
will rebase this PR -
@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it -
@dependabot merge
will merge this PR after your CI passes on it -
@dependabot squash and merge
will squash and merge this PR after your CI passes on it -
@dependabot cancel merge
will cancel a previously requested merge and block automerging -
@dependabot reopen
will reopen this PR if it is closed -
@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually -
@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) -
@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) -
@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Deploy Preview for chef-habitat processing.
Name | Link |
---|---|
Latest commit | 15023820cf532b00ee4bfd1940cb6d02e319e13c |
Latest deploy log | https://app.netlify.com/sites/chef-habitat/deploys/63ea45869fa2ff0008169525 |
Hello dependabot[bot]! Thanks for the pull request!
Here is what will happen next:
- Your PR will be reviewed by the maintainers.
- If everything looks good, one of them will approve it, and your PR will be merged.
Thank you for contributing!
Fair point.
I’ve been delivered not one but two thanksgiving turkeys this afternoon.
This likely explains a mystery bug I’ve been hitting in my testing today and finally had to dig into. https://github.com/Stebalien/tempfile/blob/1a40687e06eb656044e3d2dffa1379f04b3ef3fd/src/lib.rs#L30
This is something I’ll need to account for as I work through responding to the different events. https://github.com/notify-rs/notify/pull/166
I don’t think either is terrible, especially given that they’re known now.
Will incorporate the feed back below and push a point in time check in of my changes. Need to start heading towards my mom’s place.
From: Matt Wrock @.> Date: Tuesday, November 22, 2022 at 4:19 PM To: habitat-sh/habitat @.> Cc: Jason Heath @.>, Assign @.> Subject: Re: [habitat-sh/habitat] Bump notify from 4.0.17 to 5.0.0 (PR #8615)
@mwrock commented on this pull request.
In components/sup/src/manager/file_watcher.rshttps://github.com/habitat-sh/habitat/pull/8615#discussion_r1029828052:
-
#[derive(Clone, Debug, PartialEq)]
-
struct NotifyEvent {
-
path: PathBuf,
-
kind: NotifyEventKind,
-
// Logging the fact that a particular path was ignored. Ignoring a given
-
// path may be the correct thing to do so the purpose of logging that a
-
// particular path was ignored is to highlight that the path was ignored in
-
// case the decision made today to ignore the event needs to be changed
-
// due to design changes, lessons learned (bugs) or notify crate updates.
-
fn log_ignored_path(&mut self, notify_event: &Event, path_buf: &PathBuf) {
-
info!("IGNORING {:?} of {:?}", path_buf, notify_event)
this is probably something you want to log at TRACE level. INFO would be way too noisy.
— Reply to this email directly, view it on GitHubhttps://github.com/habitat-sh/habitat/pull/8615#pullrequestreview-1190784381, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAAHD6CKX7STWJQVPYWLUNTWJU2EFANCNFSM6AAAAAAQA73KFE. You are receiving this because you were assigned.Message ID: @.***>
A newer version of notify exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged.
Here's the changelog for 5.1.0.
notify 5.1.0 (2023-01-15)
CHANGE: switch from winapi to windows-sys https://github.com/notify-rs/notify/pull/457 FIX: kqueue-backend: batch file-watching together to improve performance https://github.com/notify-rs/notify/pull/454 DOCS: include license file in crate again https://github.com/notify-rs/notify/pull/461 DOCS: typo and examples fixups
How would you like to handle? For us, its really all about that change on the windows crate as since we don't use kqueue. I can argue for and against both finishing the upgrade work and bumping later vs bumping to 5.1.0 now. Given the nature of the change I think I would break towards upgrading now but you have a much better perspective on what that change will mean for windows.
How would you like to handle? For us, its really all about that change on the windows crate as since we don't use kqueue. I can argue for and against both finishing the upgrade work and bumping later vs bumping to 5.1.0 now. Given the nature of the change I think I would break towards upgrading now but you have a much better perspective on what that change will mean for windows.
I would go ahead and bump and if there is no refactoring needed and the tests pass then we can just take it. Otherwise we will probably want to do separately unless it is something obvious and trivial.
How would you like to handle? For us, its really all about that change on the windows crate as since we don't use kqueue. I can argue for and against both finishing the upgrade work and bumping later vs bumping to 5.1.0 now. Given the nature of the change I think I would break towards upgrading now but you have a much better perspective on what that change will mean for windows.
I would go ahead and bump and if there is no refactoring needed and the tests pass then we can just take it. Otherwise we will probably want to do separately unless it is something obvious and trivial.
Bumped and pushed. Made sure that only the notify dependency changed. Verified that cargo test --package habitat_sup --lib
is passing on windows and linux
Still making my way through the tests but wanted to drop this one comment now.
There are a lot of tests and parts of tests that are relying on knowledge of the private API and implementation of the file watcher. For instance a lot of checking on the watched file state and other tests calling methods like preclude_directories and the ends_with functions. I think we really just want to have tests that call the public API: new, run, single_iteration and validate that certain events were called or not called.
I think you are already testing that the events are being called and I see pretty thorough testing on the error conditions of new
. With that I think a lot of the other tests are repetitive and can be removed.
It looks like Path
does not transform a /
to a \
and therefore you would need to check for both on windows since /
is a legal substitute on windows. However, thinking more about this, I am inclined to eliminate the folder checking validation alltogether. There are only two users of the file watcher. First is user_config_watcher.rs which will only ever watch the user.toml file. The second user is the peer watcher. Users can pass in whatever path on the cli and therefore it is possible to pass a folder name. However, the current implementation has no validation for this. It would just result in nothing ever happening.
It looks like
Path
does not transform a/
to a\
and therefore you would need to check for both on windows since/
is a legal substitute on windows. However, thinking more about this, I am inclined to eliminate the folder checking validation alltogether. There are only two users of the file watcher. First is user_config_watcher.rs which will only ever watch the user.toml file. The second user is the peer watcher. Users can pass in whatever path on the cli and therefore it is possible to pass a folder name. However, the current implementation has no validation for this. It would just result in nothing ever happening.
The latest version of the code does this. (Just updating to keep track where we are)
Still making my way through the tests but wanted to drop this one comment now.
There are a lot of tests and parts of tests that are relying on knowledge of the private API and implementation of the file watcher. For instance a lot of checking on the watched file state and other tests calling methods like preclude_directories and the ends_with functions. I think we really just want to have tests that call the public API: new, run, single_iteration and validate that certain events were called or not called.
I think you are already testing that the events are being called and I see pretty thorough testing on the error conditions of
new
. With that I think a lot of the other tests are repetitive and can be removed.
Most recent version of the code should address all of this. (Just updating to keep track where we are)