kkawakam / rustyline

Readline Implementation in Rust
https://crates.io/crates/rustyline/
MIT License
1.54k stars 176 forks source link

Use rustix instead of nix? #733

Open newpavlov opened 1 year ago

newpavlov commented 1 year ago

Both crates are quite similar to each other, but rusyline already pulls rustix via fd-lock. Thus migrating to rustix would make dependency tree of rustyline smaller. Additionally, rustix is developed a bit more actively and provides the nice linux_raw backend feature (i.e. much less errno stupidity).

gwenn commented 1 year ago

rustix doesn't seem to support select

newpavlov commented 1 year ago

Per select documentation:

All modern applications should instead use poll(2) or epoll(7)

Is there a reason why rustyline can not use one of those?

gwenn commented 1 year ago

Only because replxx does the same: https://github.com/AmokHuginnsson/replxx/blob/1f149bfe20bf6e49c1afd4154eaf0032c8c2fda2/src/terminal.cxx#L639

newpavlov commented 1 year ago

I think migration to rustix should be a good opportunity to remove dependence on select then, no?

gwenn commented 1 year ago

https://blessed.rs/crates#section-common-subsection-system

newpavlov commented 1 year ago

Do you want to say that because rustix is not on that (unofficial) list it's inferior to nix?

rustix is developed more actively, has a member of the rust-lang org in the list of maintainers, has the desirable linux_raw feature, and has almost twice more in the number of recent downloads than nix. The only "disadvantage" is that rustix, being a fork of nix, is a younger crate.

gwenn commented 12 months ago

Thus migrating to rustix would make dependency tree of rustyline smaller

Wrong:

rustyline % cargo tree --no-default-features
├── nix v0.27.1
│   ├── bitflags v2.4.0
│   ├── cfg-if v1.0.0
│   └── libc v0.2.148

vs

rustyline % cargo tree --features with-file-history
│   └── rustix v0.38.17
│       ├── bitflags v2.4.0
│       ├── errno v0.3.4
│       │   └── libc v0.2.148
│       └── libc v0.2.148

=> one more dependency: errno

newpavlov commented 12 months ago

As mentioned in the OP, most users pull rustix through fd-lock since with-file-history is a default feature.

Sigh... If you do not want to migrate to rustix, just say so and close this issue. I personally will not be happy about it, since in my project only rustyline pulls nix, while rustix is pulled by several other dependencies, but it's your crate and your decision. I can deal with it on my side either by leaving everything as-is or by eventually migrating to an alternative.

bkaestner commented 8 months ago

rustix doesn't seem to support select

If I get it correctly, then rustyline still uses select(2) internally? This will lead to panics at runtime on any application that have a file descriptor greater than FD_SETSIZE (see also https://github.com/nix-rust/nix/issues/1087).

FD_SETSIZE can be reached quite quickly when using sockets and threads, so please consider switching to nix::poll::poll instead. As someone who has been recently bitten in the arse by cross-language usage of select(2), I would be happy to provide a patch that replaces the nix::select::select call by nix::poll:poll.

gwenn commented 3 weeks ago

Just for your information, select seems to be mandatory on MacOS: See #802