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

Release 5.0! #249

Closed
tokahuke opened this issue Jun 10, 2020 · 33 comments · Fixed by #438
Closed

Release 5.0! #249

tokahuke opened this issue Jun 10, 2020 · 33 comments · Fixed by #438

Comments

@tokahuke
Copy link

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

@JohnTitor
Copy link
Member

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

@pksunkara
Copy link

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

@0xpr03
Copy link
Member

0xpr03 commented Sep 13, 2020

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

@pksunkara
Copy link

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

@dragonnn
Copy link

dragonnn commented Oct 2, 2020

@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

@0xpr03
Copy link
Member

0xpr03 commented Mar 6, 2021

@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
Copy link
Member

0xpr03 commented Mar 21, 2021

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.

@xanderio
Copy link
Contributor

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

@lulujun2233
Copy link

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

@jyn514
Copy link

jyn514 commented Jan 27, 2022

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.

@0xpr03
Copy link
Member

0xpr03 commented Jan 27, 2022

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

@parasyte
Copy link

parasyte commented Mar 1, 2022

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.

@0xpr03
Copy link
Member

0xpr03 commented Mar 13, 2022

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

@samuelcolvin
Copy link
Contributor

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.

@0xpr03
Copy link
Member

0xpr03 commented May 17, 2022

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
Copy link
Member

0xpr03 commented May 17, 2022

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.

@pksunkara
Copy link

pksunkara commented May 17, 2022

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

@0xpr03
Copy link
Member

0xpr03 commented May 17, 2022

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 ?

@jyn514
Copy link

jyn514 commented May 17, 2022

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.

@0xpr03
Copy link
Member

0xpr03 commented May 17, 2022

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

@pksunkara
Copy link

pksunkara commented May 17, 2022

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.

@samuelcolvin
Copy link
Contributor

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

@samuelcolvin
Copy link
Contributor

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

@0xpr03
Copy link
Member

0xpr03 commented May 17, 2022

@samuelcolvin thanks

Things to do definitely for v5:

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

@tripplet
Copy link

tripplet commented Jul 2, 2022

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 😉.

@0xpr03
Copy link
Member

0xpr03 commented Aug 12, 2022

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.

@samuelcolvin
Copy link
Contributor

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

@0xpr03
Copy link
Member

0xpr03 commented Aug 12, 2022

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
Copy link
Member

0xpr03 commented Aug 14, 2022

5.0.0-pre.16 is out

@samuelcolvin
Copy link
Contributor

samuelcolvin commented Aug 14, 2022

Heres the diff for anyone else who finds it helpful: 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)

@0xpr03
Copy link
Member

0xpr03 commented Aug 14, 2022

Changes I encountered:

I feel like I could improve the changelog for that.. I kinda relied on the examples. Definitely something for v5 changelogs

@0xpr03
Copy link
Member

0xpr03 commented Aug 30, 2022

notify 5.0.0 and debouncer-mini 0.2.0 are out

@samuelcolvin
Copy link
Contributor

Thanks so much and congratulations @0xpr03 🎉!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.