serialport-rs icon indicating copy to clipboard operation
serialport-rs copied to clipboard

Update to nix-0.28

Open neumann-paulmann opened this issue 1 year ago • 8 comments

While at it, use std::os::fd::owned::OwnedFd instead of custom implementation.

Fixes #175

neumann-paulmann avatar Apr 03 '24 07:04 neumann-paulmann

Hi. This looks like a MSRV issue. The changelog state that it's currently at 1.36, which is ancient while I'm developing on 1.76 currently. The latest nix supposedly has a MSRV of 1.69. Would it be possible to raise the MSRV of serialport to 1.69 as well?

neumann-paulmann avatar Apr 05 '24 06:04 neumann-paulmann

Hmm, this does not seem MSRV-related to me, though. Here is a run with 1.76.0: https://github.com/serialport/serialport-rs/actions/runs/8535157043/job/23396028026?pr=176#step:7:42

The MSRV is set to 1.59.0, where have you found the 1.36.0 reference? Changing the MSRV has implications related to the supported platforms. What do you think @sirhcel ?

eldruin avatar Apr 05 '24 07:04 eldruin

You're right. I was looking at the first issues like here: https://github.com/serialport/serialport-rs/actions/runs/8535157043/job/23396025571?pr=176#step:7:32. I, however, cannot reproduce the other issues on my machine with above version. The library builds and all tests run fine:

~/RustroverProjects/serialport-rs> cargo build                                                                                                                                                                                  2024-04-05T10:06:49
   Compiling serialport v4.3.1-alpha.0 (/home/neumann/RustroverProjects/serialport-rs)
    Finished dev [unoptimized + debuginfo] target(s) in 0.73s
~/RustroverProjects/serialport-rs> git status                                                                                                                                                                                   2024-04-05T10:06:56
Auf Branch nix-0.28-2
Ihr Branch ist auf demselben Stand wie 'github/nix-0.28-2'.

Unversionierte Dateien:
  (benutzen Sie "git add <Datei>...", um die Änderungen zum Commit vorzumerken)
        .idea/

nichts zum Commit vorgemerkt, aber es gibt unversionierte Dateien
(benutzen Sie "git add" zum Versionieren)
~/RustroverProjects/serialport-rs> cargo clean                                                                                                                                                                                  2024-04-05T10:07:11
     Removed 2445 files, 801.1MiB total
~/RustroverProjects/serialport-rs> cargo build --all-features                                                                                                                                                                   2024-04-05T10:08:08
   Compiling proc-macro2 v1.0.79
   Compiling unicode-ident v1.0.12
   Compiling libc v0.2.153
   Compiling pkg-config v0.3.30
   Compiling thiserror v1.0.58
   Compiling cfg_aliases v0.1.1
   Compiling serde v1.0.197
   Compiling bitflags v2.5.0
   Compiling cfg-if v1.0.0
   Compiling scopeguard v1.2.0
   Compiling nix v0.28.0
   Compiling libudev-sys v0.1.4
   Compiling quote v1.0.35
   Compiling syn v2.0.58
   Compiling libudev v0.3.0
   Compiling thiserror-impl v1.0.58
   Compiling serde_derive v1.0.197
   Compiling unescaper v0.1.4
   Compiling serialport v4.3.1-alpha.0 (/home/neumann/RustroverProjects/serialport-rs)
    Finished dev [unoptimized + debuginfo] target(s) in 5.82s
~/RustroverProjects/serialport-rs> cargo test --all-features                                                                                                                                                                    2024-04-05T10:08:18
   Compiling version_check v0.9.4
   Compiling syn v1.0.109
   Compiling autocfg v1.2.0
   Compiling heck v0.4.1
   Compiling os_str_bytes v6.6.1
   Compiling hashbrown v0.12.3
   Compiling textwrap v0.16.1
   Compiling strsim v0.10.0
   Compiling termcolor v1.4.1
   Compiling bitflags v1.3.2
   Compiling once_cell v1.19.0
   Compiling assert_hex v0.4.1
   Compiling atty v0.2.14
   Compiling clap_lex v0.2.4
   Compiling proc-macro-error-attr v1.0.4
   Compiling proc-macro-error v1.0.4
   Compiling indexmap v1.9.3
   Compiling clap_derive v3.2.25
   Compiling clap v3.2.25
   Compiling serialport v4.3.1-alpha.0 (/home/neumann/RustroverProjects/serialport-rs)
    Finished test [unoptimized + debuginfo] target(s) in 6.66s
     Running unittests src/lib.rs (target/debug/deps/serialport-65059b8e45381439)

running 1 test
test posix::tty::test_ttyport_into_raw_fd ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/test_serialport.rs (target/debug/deps/test_serialport-ddc0aa2fcb68f854)

running 6 tests
test test_configuring_ports ... ok
test test_duplicating_port_config ... ok
test test_opening_port ... ok
test test_opening_native_port ... ok
test test_listing_ports ... ok
test test_opening_found_ports ... ok

test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

     Running tests/test_try_clone.rs (target/debug/deps/test_try_clone-9d7e1a4c1730c099)

running 2 tests
test test_try_clone ... ok
test test_try_clone_move ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/test_tty.rs (target/debug/deps/test_tty-43b2dd8eb44f5847)

running 4 tests
test test_ttyport_pair ... ok
test test_ttyport_set_nonstandard_baud ... ok
test test_ttyport_set_standard_baud ... ok
test test_ttyport_timeout ... ok

test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.00s

   Doc-tests serialport

running 3 tests
test src/lib.rs - new (line 806) - compile ... ok
test src/posix/tty.rs - posix::tty::TTYPort::pair (line 228) ... ok
test src/posix/tty.rs - posix::tty::TTYPort (line 46) ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.17s

~/RustroverProjects/serialport-rs> git branch                                                                                                                                                                                   2024-04-05T10:08:31
  main
  nix-0.28
  nix-0.28-
* nix-0.28-2
~/RustroverProjects/serialport-rs>                         

neumann-paulmann avatar Apr 05 '24 08:04 neumann-paulmann

Hmm, this does not seem MSRV-related to me, though. [...]

No, this looks like an issue with the signedness of milliseconds in the explicit conversion from Duration. Both target types for the poll timeout support directly converting from Duration and we can work around it by using these direct conversions.

Ended up with bf7050c while looking into this. So there is just raising the MSRV left.

Changing the MSRV has implications related to the supported platforms. What do you think @sirhcel?

At a first glance at least when it comes to the Rust support shipped with Yocto. Dependents using older Yocto releases could still pin their dependency on us to a minor version and we could attempt to scan all publicly visible dependencies. And on the other hand there is also meta-rust-bin which provides the brand spanking new releases of Rust. Let me think and sleep over it.

sirhcel avatar Apr 09 '24 21:04 sirhcel

Did you have time to think about this @sirhcel ?

eldruin avatar May 22 '24 11:05 eldruin

I finally managed to. Just to get my perspective on this right: Is bumping our nix dependency tied to switching to OwnedFd? Or is the latter a drive-by catch happening in the context of bumping nix?

I'm fine with switching to OwnedFd with a minor release as this does not look like a breaking change to me as far as I'm overseeing this as of now.

When it comes to bumping nix, I would postpone this to the next major release as this could be a breaking change for dependents still relying on our MSRV being 1.59.0. I know this is subject to general discussion but we've got our MSRV for dev builds broken recently with our dependencies serialport -> clap -> os_str_bytes (see #186). I would like to spare dependents with tighter MSRV requirements this experience.

sirhcel avatar May 24 '24 07:05 sirhcel

I think that changes regarding the used file descriptor objects are necessary and that the custom implementation is no longer needed. I also agree to not rush this, since it might break things as you and others correctly pointed out. I currently do not have the time to look into this further. I might be able to do some work on this in two weeks or so when I'm done with my other stuff.

neumann-paulmann avatar May 24 '24 07:05 neumann-paulmann

Given the adoption of OwnedFd, could a std::os:fd::AsFd implementation be added as well?

max-heller avatar Sep 05 '24 15:09 max-heller

Hi folks, what's the latest on this effort?

I'm attempting to pull this into a project that's locked-in on nix 0.28.0 so this is blocking me from using serialport.

Is there anything I can do to help move this forward?

JonathonReinhart avatar Feb 13 '25 22:02 JonathonReinhart

I'm packaging serialport for Debian which ships nix in version 0.29.0. I forward-ported the changes of this pull request.

It would be nice to see support for more recent nix in the main branch. Thank you!

josch avatar Feb 17 '25 15:02 josch

I've made a new attempt at the nix upgrade in #263.

paolobarbolini avatar Apr 30 '25 07:04 paolobarbolini