habitat icon indicating copy to clipboard operation
habitat copied to clipboard

Bump notify from 4.0.17 to 5.0.0

Open dependabot[bot] opened this issue 2 years ago • 2 comments

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 to Event, 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 to Event, 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.0
  • a83279f improve upgrade docs and fix Config link
  • 65da37b file back v4 history into changelog
  • 1fbf8fa add accessor for whether rescan is required on Event
  • 17580f6 fixup rebase gunk
  • 54465e9 fixup optional crossbeam feature selection in debouncer-mini
  • 94f1680 update minimum walkdir version to 2.2.2 (#432)
  • 4d16a54 fixup doc links post initial debouncer-mini release
  • d698f90 fixup moved readme due to reorg
  • 2f91e15 fix typo in readme
  • Additional commits viewable in compare view

Dependabot compatibility score

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)

dependabot[bot] avatar Aug 31 '22 04:08 dependabot[bot]

Deploy Preview for chef-habitat processing.

Name Link
Latest commit 15023820cf532b00ee4bfd1940cb6d02e319e13c
Latest deploy log https://app.netlify.com/sites/chef-habitat/deploys/63ea45869fa2ff0008169525

netlify[bot] avatar Aug 31 '22 04:08 netlify[bot]

Hello dependabot[bot]! Thanks for the pull request!

Here is what will happen next:

  1. Your PR will be reviewed by the maintainers.
  2. If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

chef-expeditor[bot] avatar Aug 31 '22 04:08 chef-expeditor[bot]

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: @.***>

jasonheath avatar Nov 22 '22 21:11 jasonheath

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.

dependabot[bot] avatar Jan 30 '23 04:01 dependabot[bot]

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.

jasonheath avatar Jan 30 '23 15:01 jasonheath

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.

mwrock avatar Jan 30 '23 19:01 mwrock

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

jasonheath avatar Jan 30 '23 20:01 jasonheath

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.

mwrock avatar Jan 30 '23 21:01 mwrock

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.

mwrock avatar Jan 31 '23 23:01 mwrock

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)

jasonheath avatar Feb 07 '23 11:02 jasonheath

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)

jasonheath avatar Feb 07 '23 11:02 jasonheath