Removal of `Copy` from `ReadFlag` (and co) in 0.6.2 breaks downstream dependencies

[mqudsi]

Hi, thanks for this crate, I'm using it my syngestures project to avoid having to interface with evdev C api directly.

The 0.6.2 release removed the Copy attribute from ReadFlag which broke cargo install syngestures (mqudsi/syngesture#16) but it took me a while to realize what happened because, as a binary project, I have Cargo.lock checked in to the repo and thus was using my pinned 0.6.1 version of your code.

Since rust automatically updates to the latest patch version with cargo install (and cargo package does not upload a copy of Cargo.lock), users attempting to install syngestures were automatically building against evdev-rs 0.6.2 (after it was released) instead of the 0.6.1 I had pinned. The problem is that the 0.6.2 release included a breaking change: the removal of the Copy attribute from ReadFlag (and maybe others), breaking rust semver versioning for 0.x projects.

I'm uploading a new version of syngestures to crates.io that hacks around the removal of both Copy and Clone from ReadFlag in a very ugly (but perfectly reasonable, given the api limitations) way, but if I may suggest one of these two things:

• It's not clear to me that the removal of Copy (and Clone) was intentional rather than unintended fallout from the upgrade to v2 of the bitflags crate; my guess is that you didn't mean to remove either of these attributes from ReadFlags (and any other flag structs) and the most straightforward thing to do here would be to manually add #[derive(Copy, Clone)] to all the bitflag declarations to get them back, published that as evdev-rs 0.6.3, and pretty please with a cherry on top yank evdev-rs 0.6.2 from crates.io for being an intended breaking change that removed core functionality,
• Alternatively, if for some reason you genuinely do not want your flags to have Copy and Clone, then please re-publish as 0.7.0 and yank 0.6.2 so existing code that specified a dependency on evdev-rs version 0.6.x would continue to work even with cargo install/without Cargo.lock, (but I'm reasonably sure this isn't your intention).

As a reference for why Copy (or at least Clone) is required, I'm copy-and-pasting an example of (probably idomatic?) downstream code that requires (some variant of) that functionality to function:

error[E0382]: use of moved value: `read_flag`
   --> src/main.rs:185:53
    |
179 |             let mut read_flag = ReadFlag::NORMAL;
    |                 ------------- move occurs because `read_flag` has type `ReadFlag`, which does not implement the `Copy` trait
180 |             'device: loop {
    |                      ---- inside of this loop
...
185 |                 let event = match device.next_event(read_flag) {
    |                                                     ^^^^^^^^^ value moved here, in previous iteration of loop
    |
note: these 2 reinitializations might get skipped
   --> src/main.rs:194:25
    |
194 |                         read_flag = ReadFlag::SYNC;
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^
...
199 |                         read_flag = ReadFlag::NORMAL;
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: verify that your loop breaking logic is correct
   --> src/main.rs:195:25
    |
162 |     for (device_path, gestures) in config.devices {
    |     ---------------------------------------------
...
180 |             'device: loop {
    |             -------------
...
195 |                         continue;
    |                         ^^^^^^^^ this `continue` advances the loop at line 180
...
202 |                                 Ok(()) => continue 'device,
    |                                           ^^^^^^^^^^^^^^^^ this `continue` advances the loop at line 180
...
205 |                                         continue;
    |                                         ^^^^^^^^ this `continue` advances the loop at line 200
...
208 |                                     break 'device;
    |                                     ^^^^^^^^^^^^^ this `break` exits the loop at line 180
...
215 |                         break;
    |                         ^^^^^ this `break` exits the loop at line 180
help: consider moving the expression out of the loop so it is only moved once
    |
180 ~             let mut value = device.next_event(read_flag);
181 ~             'device: loop {
182 |                 if SIGHUP.load(Ordering::Relaxed) {
...
185 |                 }
186 ~                 let event = match value {
    |

this code previously worked with 0.6.1, would theoretically work if you replace device.next_event(read_flag) with device.next_event(read_flag.Clone()) (except there is no ReadFlags::clone()) and required the ugly hack I pushed to work around this for now.

I was forced to work around the lack of Copy or Clone by doing the following:

diff --git a/src/main.rs b/src/main.rs
index d69d9bf..6c04fff 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -182,7 +182,10 @@ fn watch_devices<'scope>(
                     debug!("Threading exiting because SIGHUP was set.");
                     return;
                 }
-                let event = match device.next_event(read_flag) {
+                // Work around evdev-rs bug (?) removing `Copy` and `Clone` from `ReadFlag`
+                // https://github.com/ndesh26/evdev-rs/issues/116
+                let read_flag_clone = ReadFlag::from_bits(read_flag.bits()).unwrap();
+                let event = match device.next_event(read_flag_clone) {
                     Ok((ReadStatus::Success, event)) => event,
                     Ok((
                         ReadStatus::Sync,

which, as you can see, is far from ideal (though a step above using let read_flag_clone = unsafe { std::mem::transmute(*std::mem::transmute::<_, &u32>(&read_flag)) };!)

[ndesh26]

Thanks @mqudsi for highlighting the issue! I will reintroduce the traits and will release a new version of the crate.

[mqudsi]

Thanks for the quick response. I’m not 100% certain but I think v1 included all these derives that you’d want for backwards compatibility (and so they can be used in a struct that wanted to use them too):

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]

[ndesh26]

The new version has been released, and I have yanked the problematic version. I will close this issue. Please reopen if you are still running into the error.

[mqudsi]

Thank you, that's perfect.