notify icon indicating copy to clipboard operation
notify copied to clipboard

Release 5.0!

Open tokahuke opened this issue 5 years ago • 31 comments

Release candidate 2 is already 5 months in the air. Isn't is time to release it as "final" already?

tokahuke avatar Jun 10 '20 13:06 tokahuke

Before releasing, I'd like to resolve #248 and audit test cases as 2668c204525fc081bd6001acb1e8797ae520a43c removed most of them.

JohnTitor avatar Jul 18 '20 17:07 JohnTitor

What happened to debounced watcher in the pre releases? Is it being removed for 5.0?

pksunkara avatar Sep 12 '20 06:09 pksunkara

It was removed due to problems, so we would have to re-implement it for a real 5.0 release.

0xpr03 avatar Sep 13 '20 13:09 0xpr03

Ah, thanks for the answer. I would love to contribute since I kinda need it, but I am not knowledgable enough in this area. 😞

pksunkara avatar Sep 13 '20 18:09 pksunkara

@0xpr03 maybe using https://crates.io/crates/rjdebounce ? I used it outside notify in 5.0.0-pre3 and it works fine. I need 5.0.0 because I need to have an option to do things without using thread channels

dragonnn avatar Oct 02 '20 08:10 dragonnn

@0xpr03 maybe using https://crates.io/crates/rjdebounce ?

I don't think we want to add more dependencies just for debouncing. Also looking at the code this does not do what we want. The job of a debouncer it to record all events and only return you the ones that are the most important (create after write+delete) per file (notice directory deletions overriding subfiles..) after a timeout (or even without a timeout if something always receives an event => permanent write). Also we want to introduce the option of receiving all events and also the debounced version at the same time. Additionally we can't simply not run our edge code for X seconds as we have to catch all system events to handle recursive watching newely created files.

0xpr03 avatar Mar 06 '21 22:03 0xpr03

I need to have an option to do things without using thread channels

What exactly do you mean without threads ? Notify certainly doesn't support no-std and even without any debouncer we're using a thread underneath.

0xpr03 avatar Mar 21 '21 18:03 0xpr03

After #335 and #337 have been merged, cloud we get a new prerelease?

xanderio avatar Jul 18 '21 14:07 xanderio

Is there deadline or timeline to release 5.0 (with debouncer)?

lulujun2233 avatar Jan 19 '22 02:01 lulujun2233

I would find a release useful even without the debouncer, it would let me get rid of a duplicate mio dependency. Tokio has depended on mio 0.7 for a while now so any crate with both tokio and notify pulls in a duplicate mio dependency.

jyn514 avatar Jan 27 '22 18:01 jyn514

I don't think you want me to release v5 in the current state (not if I'm taking semver v5 serious). First of all #380 and #381 are things we'll want to resolve before releasing a v5, otherwise a v6 will be coming soon after, so you could also just use the current pre-release (required code changes would be the same). Then I'd personally expect a project like this to get back onto its tests, which got removed due to some bugs some time ago before I started to maintain this. (And also the v5 to be on-par regarding a debouncer to v4.) The third thing is that I don't own a macbook, and I don't intend to drop a thousand bucks on a machine just for that, so I'm not well equipped to actually test or develop this on all the OS we support. And last but not least it looks like I'm currently the sole (active/contributing/..) maintainer, which I never intended to be when offering to help out.**

If you just don't want duplicate mios in your dependency list, pin your repo to one of the v5-pre versions*, it's the same as releasing v5 in that regard (see above).

I can understand the frustration, but maybe this can shed some light into it.

P.S.: If you want a timeline: I'm busy until the mid of april with exams, so there probably won't be much happening before that.

*SemVer is never a guarantee, only a promise, so it's not like that'll make that much of a difference.

**This is not meant to be an attack on other members

0xpr03 avatar Jan 27 '22 23:01 0xpr03

If nothing else, we need a new release because the current version has a security advisory in a transitive dependency on the old version of mio.

warning[A003]: `net2` crate has been deprecated; use `socket2` instead
   ┌─ C:\Users\...\Cargo.lock:72:1
   │
72 │ net2 0.2.37 registry+https://github.com/rust-lang/crates.io-index
   │ ----------------------------------------------------------------- unmaintained advisory detected
   │
   = ID: RUSTSEC-2020-0016
   = Advisory: https://rustsec.org/advisories/RUSTSEC-2020-0016
   = The [`net2`](https://crates.io/crates/net2) crate has been deprecated
     and users are encouraged to considered [`socket2`](https://crates.io/crates/socket2) instead.
   = Announcement: https://github.com/deprecrated/net2-rs/commit/3350e3819adf151709047e93f25583a5df681091
   = Solution: No safe upgrade is available!
   = net2 v0.2.37
     ├── mio v0.6.23
     │   ├── mio-extras v2.0.6
     │   │   └── notify v4.0.17
     │   │       └── watchexec v1.17.1
     │   │           └── example v0.1.0
     │   └── notify v4.0.17 (*)
     └── miow v0.2.2
         └── mio v0.6.23 (*)

Even if #248 was backported to 4.0, that would be a huge win, IMHO.

parasyte avatar Mar 01 '22 10:03 parasyte

@parasyte if you want to do that and test it on all systems I'd be open for that.

0xpr03 avatar Mar 13 '22 21:03 0xpr03

Taken from #405, as that was marked as a duplicate:

Thanks so much for notify, it's great and I've had a really great experience wrapping it for the watchfiles python package.

Unfortunately, the continued use of pre-releases is making notify significantly harder to use than it should be.

Please could you release version 5.

In particular as per rust's dependency resolution logic on pre-releases:

Cargo allows "newer" pre-releases to be used automatically. For example, if 1.0.0-beta is published, then a requirement foo = "1.0.0-alpha" will allow updating to the beta version.

This means that we can't effectively lock the notify dependency to a specific pre-release.

As per samuelcolvin/watchfiles#144 and conda-forge/watchfiles-feedstock#5 this has just bitten us since conda forge decided to use 5.0.0-pre.15 which is incompatible with 5.0.0-pre.14 (which the package had been targeting) because PollWatcher::with_delay was replaced with PollWatcher::with_config. To make matters worse, this breaking change was not noted in the release notes.

Please, please can we get v5 released.

If there are missing features, they could be added in v5.1 or even v6. There's no cost to creating a release and then making further minor releases to add features or major releases to change behaviour.

samuelcolvin avatar May 17 '22 13:05 samuelcolvin

This means that we can't effectively lock the notify dependency to a specific pre-release

Ok that is actually bad and I didn't know of that, as I was always using lockfiles.

0xpr03 avatar May 17 '22 17:05 0xpr03

Ok so as I won't be having as much time to bring it into the shape as I'd like to, my current option is only to: A) Postpone this further or, B) possibly as wanted by people here, release v5 in the state it is, and possibly iterate to v6 quickly, depending on how quick some breaking change comes up.

I think it's worth a try. It's still FOSS and I'd hope no one is monitoring files with notify in pacemakers.

0xpr03 avatar May 17 '22 17:05 0xpr03

Depending on prereleases should always be in the format of pkg = '=1.0.0-alpha.1'. Please notice the =.

pksunkara avatar May 17 '22 17:05 pksunkara

The biggest breaking change is that there is no debouncer. I could hack one up in the next two weeks and we'll ship with that, making a migration at least something of a better experience.

@pksunkara does that actually work ? I'd have assumed, based on the linked docs, that it's simply ignored ?

0xpr03 avatar May 17 '22 17:05 0xpr03

I would love to have v5 in the state notify is currently in :) my understanding is that past releases didn't have a debouncer either.

jyn514 avatar May 17 '22 17:05 jyn514

v4 did, so the problem are people coming from v4 waiting for a stable v5

0xpr03 avatar May 17 '22 17:05 0xpr03

Linked docs is talking about the default behavior. But if you specify =, then yes the prerelease gets pinned.

This is how a lot of packages use prereleases. The issue here is that watchfiles package did not use it correctly.

pksunkara avatar May 17 '22 17:05 pksunkara

Thanks so much @pksunkara, this resolves my immediate problem.

samuelcolvin avatar May 17 '22 18:05 samuelcolvin

FWIW, there's pretty complete denounce logic in watchfiles (implemented in rust), see here. Feel free to use any of it.

samuelcolvin avatar May 17 '22 22:05 samuelcolvin

@samuelcolvin thanks

Things to do definitely for v5:

  • [x] Upgrade path from v4
  • [x] Overhaul documentation to reflect current state
  • [x] List of changes
  • [x] Remove configuration options or implement them
  • [X] Add some kind of debouncer (maybe just any event for files, most people don't need the actual event)

0xpr03 avatar May 17 '22 23:05 0xpr03

Like many others I'm also eagerly awaiting a release. While waiting I created my own debounce logic see in this gist It only depends on the timer crate to create a new debounced channel from the original channel given out by notify. It is definitely not optimal and can create many threads for the timers but seems to work for my use case watching only a few files but no guarantees 😉.

tripplet avatar Jul 02 '22 10:07 tripplet

The upcoming 5.0.0-pre.16 is the final RC. Please test this out. If no further issues arise I'll do a 5.0 stable release in two weeks.

0xpr03 avatar Aug 12 '22 14:08 0xpr03

Will do, please let us when it's released.

samuelcolvin avatar Aug 12 '22 14:08 samuelcolvin

With that said it is probably also time for new maintainers. My own use case for this crate was ~3 years ago, and I think somebody invested would fit that position much better. I think this crate is ultimately one of the many backbones of the rust ecosystem, which is why I don't want it to die.

0xpr03 avatar Aug 12 '22 14:08 0xpr03

5.0.0-pre.16 is out

0xpr03 avatar Aug 14 '22 12:08 0xpr03

Heres the diff for anyone else who finds it helpful: https://github.com/notify-rs/notify/compare/5.0.0-pre.15...5.0.0-pre.16

Changes I encountered:

  • PollWatcher::with_config has become PollWatcher::new
  • RecommendedWatcher::new's signature has changed to take a config argument.
  • The Config interface has changed somewhat, so you need to do Config::default().with_poll_interval(delay)

samuelcolvin avatar Aug 14 '22 13:08 samuelcolvin