fix: Make chrono a default-enabled optional feature. (#39)

This allows to turn chrono support off without actually affecting the ability to trash and restore items. `chrono` still has issues to dubious local-time support which relies on a c-library function that can cause undefined behaviour as it accesses an environment variable in a non-threadsafe fashion.
Byron · Jul 5, 2022 · 67244ba · 67244ba
1 parent 17d162f
commit 67244ba
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 30 deletions.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -35,6 +35,11 @@ jobs:
       with:
         command: test
         args: --verbose
+    - name: cargo test
+      uses: actions-rs/cargo@v1
+      with:
+        command: test
+        args: --verbose --no-default-features
     - name: cargo fmt
       uses: actions-rs/cargo@v1
       with:

diff --git a/Cargo.toml b/Cargo.toml
@@ -13,7 +13,7 @@ edition = "2018"
 include = ["src/**/*", "LICENSE.txt", "README.md", "CHANGELOG.md", "build.rs"]
 
 [features]
-default = ["coinit_apartmentthreaded"]
+default = ["coinit_apartmentthreaded", "chrono"]
 coinit_apartmentthreaded = []
 coinit_multithreaded = []
 coinit_disable_ole1dde = []
@@ -34,7 +34,7 @@ env_logger = "0.9"
 objc = "0.2.7"
 
 [target.'cfg(all(unix, not(target_os = "macos"), not(target_os = "ios"), not(target_os = "android")))'.dependencies]
-chrono = "0.4.9"
+chrono = { version = "0.4.9", optional = true }
 libc = "0.2.65"
 scopeguard = "1.0.0"
 url = "2.1.0"

diff --git a/src/freedesktop.rs b/src/freedesktop.rs
@@ -14,8 +14,7 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use chrono::{NaiveDateTime, TimeZone};
-use log::{debug, error, warn};
+use log::{debug, warn};
 
 use crate::{Error, TrashContext, TrashItem};
 
@@ -116,6 +115,7 @@ pub fn list() -> Result<Vec<TrashItem>, Error> {
                 continue;
             }
         };
+        #[cfg_attr(not(feature = "chrono"), allow(unused_labels))]
         'trash_item: for entry in read_dir {
             let info_entry = match entry {
                 Ok(entry) => entry,
@@ -150,6 +150,7 @@ pub fn list() -> Result<Vec<TrashItem>, Error> {
             let id = info_path.clone().into();
             let mut name = None;
             let mut original_parent: Option<PathBuf> = None;
+            #[cfg_attr(not(feature = "chrono"), allow(unused_mut))]
             let mut time_deleted = None;
 
             let info_reader = BufReader::new(info_file);
@@ -177,31 +178,39 @@ pub fn list() -> Result<Vec<TrashItem>, Error> {
                     let parent = full_path_utf8.parent().unwrap();
                     original_parent = Some(parent.into());
                 } else if key == "DeletionDate" {
-                    let parsed_time = NaiveDateTime::parse_from_str(value, "%Y-%m-%dT%H:%M:%S");
-                    let naive_local = match parsed_time {
-                        Ok(t) => t,
-                        Err(e) => {
-                            error!("Failed to parse the deletion date of the trash item {:?}. The deletion date was '{}'. Parse error was: {:?}", name, value, e);
-                            continue 'trash_item;
-                        }
-                    };
-                    let time = chrono::Local.from_local_datetime(&naive_local).earliest();
-                    match time {
-                        Some(time) => time_deleted = Some(time.timestamp()),
-                        None => {
-                            error!("Failed to convert the local time to a UTC time. Local time was {:?}", naive_local);
-                            continue 'trash_item;
+                    #[cfg(feature = "chrono")]
+                    {
+                        use chrono::{NaiveDateTime, TimeZone};
+                        let parsed_time = NaiveDateTime::parse_from_str(value, "%Y-%m-%dT%H:%M:%S");
+                        let naive_local = match parsed_time {
+                            Ok(t) => t,
+                            Err(e) => {
+                                log::error!("Failed to parse the deletion date of the trash item {:?}. The deletion date was '{}'. Parse error was: {:?}", name, value, e);
+                                continue 'trash_item;
+                            }
+                        };
+                        let time = chrono::Local.from_local_datetime(&naive_local).earliest();
+                        match time {
+                            Some(time) => time_deleted = Some(time.timestamp()),
+                            None => {
+                                log::error!("Failed to convert the local time to a UTC time. Local time was {:?}", naive_local);
+                                continue 'trash_item;
+                            }
                         }
                     }
                 }
             }
             if let Some(name) = name {
                 if let Some(original_parent) = original_parent {
-                    if let Some(time_deleted) = time_deleted {
-                        result.push(TrashItem { id, name, original_parent, time_deleted });
-                    } else {
+                    if time_deleted.is_none() {
                         warn!("Could not determine the deletion time of the trash item. (The `DeletionDate` field is probably missing from the info file.) The info file path is: '{:?}'", info_path);
                     }
+                    result.push(TrashItem {
+                        id,
+                        name,
+                        original_parent,
+                        time_deleted: time_deleted.unwrap_or(-1),
+                    });
                 } else {
                     warn!("Could not determine the original parent folder of the trash item. (The `Path` field is probably missing from the info file.) The info file path is: '{:?}'", info_path);
                 }
@@ -411,12 +420,19 @@ fn move_to_trash(
             Ok(mut file) => {
                 debug!("Successfully created {:?}", info_file_path);
                 // Write the info file before actually moving anything
-                let now = chrono::Local::now();
                 writeln!(file, "[Trash Info]")
                     .and_then(|_| {
                         let absolute_uri = encode_uri_path(src);
                         writeln!(file, "Path={}", absolute_uri).and_then(|_| {
-                            writeln!(file, "DeletionDate={}", now.format("%Y-%m-%dT%H:%M:%S"))
+                            #[cfg(feature = "chrono")]
+                            {
+                                let now = chrono::Local::now();
+                                writeln!(file, "DeletionDate={}", now.format("%Y-%m-%dT%H:%M:%S"))
+                            }
+                            #[cfg(not(feature = "chrono"))]
+                            {
+                                Ok(())
+                            }
                         })
                     })
                     .map_err(|e| fsys_err_to_unknown(&info_file_path, e))?;

diff --git a/src/lib.rs b/src/lib.rs
@@ -265,6 +265,7 @@ pub struct TrashItem {
 
     /// The number of non-leap seconds elapsed between the UNIX Epoch and the
     /// moment the file was deleted.
+    /// Without the "chrono" feature, this will be a negative number on linux only.
     pub time_deleted: i64,
 }
 

diff --git a/src/tests.rs b/src/tests.rs
@@ -73,13 +73,15 @@ mod os_limited {
                 Some(items) => {
                     assert_eq!(items.len(), batches);
                     for item in items {
-                        let diff = (item.time_deleted - actual_unix_deletion_time).abs();
-                        if diff > MAX_SECONDS_DIFFERENCE {
-                            panic!(
-                                "The deleted item does not have the timestamp that represents its deletion time. Expected: {}. Got: {}",
-                                actual_unix_deletion_time,
-                                item.time_deleted
-                            );
+                        if cfg!(feature = "chrono") {
+                            let diff = (item.time_deleted - actual_unix_deletion_time).abs();
+                            if diff > MAX_SECONDS_DIFFERENCE {
+                                panic!(
+                                    "The deleted item does not have the timestamp that represents its deletion time. Expected: {}. Got: {}",
+                                    actual_unix_deletion_time,
+                                    item.time_deleted
+                                );
+                            }
                         }
                     }
                 }