public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents
@ 2020-09-04 12:33 Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 01/11] tools/systemd/tm_editor: remove TMChanges optimization Dominik Csapak
                   ` (11 more replies)
  0 siblings, 12 replies; 15+ messages in thread
From: Dominik Csapak @ 2020-09-04 12:33 UTC (permalink / raw)
  To: pbs-devel

implements Dates and more special words for calendar events
and fixes some small bugs

not implemented for calendarevents are Timezones, Unix Timestamps and
the ~ operator for the last days of the month

changes from v1:
* do not change DateTimeValues to i32 but fail for not
  correctly convertable entries
  (this makes years < 0 invalid but it's ok)
* remove the TMChanges bitflags completely (its not faster, but more
  complicated)

Dominik Csapak (11):
  tools/systemd/tm_editor: remove TMChanges optimization
  tools/systemd/time: let libc normalize time for us
  tools/systemd/time: move continue out of the if/else
  tools/systemd/time: convert the resulting timestamp into an option
  tools/systemd/tm_editor: remove reset_time from add_days and document it
  tools/systemd/parse_time: error out on invalid ranges
  tools/systemd/time: fix selection for multiple options
  tools/systemd/tm_editor: move conversion of the year into getter and setter
  tools/systemd/tm_editor: add setter/getter for months/years/days
  tools/systemd/time: fix signed conversion
  tools/systemd/time: enable dates for calendarevents

 src/api2/admin/sync.rs          |   3 +-
 src/bin/proxmox-backup-proxy.rs |   9 +-
 src/tools/systemd/parse_time.rs |  80 +++++++++++++-
 src/tools/systemd/time.rs       | 150 ++++++++++++++++++--------
 src/tools/systemd/tm_editor.rs  | 186 +++++++++++---------------------
 5 files changed, 249 insertions(+), 179 deletions(-)

-- 
2.20.1





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 01/11] tools/systemd/tm_editor: remove TMChanges optimization
  2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
@ 2020-09-04 12:33 ` Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 02/11] tools/systemd/time: let libc normalize time for us Dominik Csapak
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2020-09-04 12:33 UTC (permalink / raw)
  To: pbs-devel

while it was correct, there was no measurable speed gain
(a benchmark yielded 2.8 ms for a spec that did not find a timestamp either way)
so remove it for simpler code

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/systemd/time.rs      | 24 ++++++++----------------
 src/tools/systemd/tm_editor.rs | 33 +--------------------------------
 2 files changed, 9 insertions(+), 48 deletions(-)

diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index 773a1509..2a1bf8a3 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -172,12 +172,10 @@ pub fn compute_next_event(
             count += 1;
         }
 
-        if !all_days && t.changes.contains(TMChanges::WDAY) { // match day first
+        if !all_days { // match day first
             let day_num = t.day_num();
             let day = WeekDays::from_bits(1<<day_num).unwrap();
-            if event.days.contains(day) {
-                t.changes.remove(TMChanges::WDAY);
-            } else {
+            if !event.days.contains(day) {
                 if let Some(n) = ((day_num+1)..7)
                     .find(|d| event.days.contains(WeekDays::from_bits(1<<d).unwrap()))
                 {
@@ -193,11 +191,9 @@ pub fn compute_next_event(
         }
 
         // this day
-        if !event.hour.is_empty() && t.changes.contains(TMChanges::HOUR) {
+        if !event.hour.is_empty() {
             let hour = t.hour() as u32;
-            if DateTimeValue::list_contains(&event.hour, hour) {
-                t.changes.remove(TMChanges::HOUR);
-            } else {
+            if !DateTimeValue::list_contains(&event.hour, hour) {
                 if let Some(n) = DateTimeValue::find_next(&event.hour, hour) {
                     // test next hour
                     t.set_time(n as libc::c_int, 0, 0);
@@ -211,11 +207,9 @@ pub fn compute_next_event(
         }
 
         // this hour
-        if !event.minute.is_empty()  && t.changes.contains(TMChanges::MIN) {
+        if !event.minute.is_empty() {
             let minute = t.min() as u32;
-            if DateTimeValue::list_contains(&event.minute, minute) {
-                t.changes.remove(TMChanges::MIN);
-            } else {
+            if !DateTimeValue::list_contains(&event.minute, minute) {
                 if let Some(n) = DateTimeValue::find_next(&event.minute, minute) {
                     // test next minute
                     t.set_min_sec(n as libc::c_int, 0);
@@ -229,11 +223,9 @@ pub fn compute_next_event(
         }
 
         // this minute
-        if !event.second.is_empty() && t.changes.contains(TMChanges::SEC) {
+        if !event.second.is_empty() {
             let second = t.sec() as u32;
-            if DateTimeValue::list_contains(&event.second, second) {
-                t.changes.remove(TMChanges::SEC);
-            } else {
+            if !DateTimeValue::list_contains(&event.second, second) {
                 if let Some(n) = DateTimeValue::find_next(&event.second, second) {
                     // test next second
                     t.set_sec(n as libc::c_int);
diff --git a/src/tools/systemd/tm_editor.rs b/src/tools/systemd/tm_editor.rs
index ef8bcd2c..1e8e4d07 100644
--- a/src/tools/systemd/tm_editor.rs
+++ b/src/tools/systemd/tm_editor.rs
@@ -1,25 +1,10 @@
 use anyhow::Error;
-use bitflags::bitflags;
 
 use proxmox::tools::time::*;
 
-bitflags!{
-    #[derive(Default)]
-    pub struct TMChanges: u8 {
-        const SEC = 1;
-        const MIN = 2;
-        const HOUR = 4;
-        const MDAY = 8;
-        const MON = 16;
-        const YEAR = 32;
-        const WDAY = 64;
-    }
-}
-
 pub struct TmEditor {
     utc: bool,
     t: libc::tm,
-    pub changes: TMChanges,
 }
 
 fn is_leap_year(year: libc::c_int) -> bool {
@@ -45,7 +30,7 @@ impl TmEditor {
     pub fn new(epoch: i64, utc: bool) -> Result<Self, Error> {
         let mut t = if utc { gmtime(epoch)? } else { localtime(epoch)? };
         t.tm_year += 1900; // real years for clarity
-        Ok(Self { utc, t, changes: TMChanges::all() })
+        Ok(Self { utc, t })
     }
 
     pub fn into_epoch(mut self) -> Result<i64, Error> {
@@ -60,11 +45,9 @@ impl TmEditor {
             self.t.tm_hour = 0;
             self.t.tm_min = 0;
             self.t.tm_sec = 0;
-            self.changes.insert(TMChanges::HOUR|TMChanges::MIN|TMChanges::SEC);
         }
         self.t.tm_mday += days;
         self.t.tm_wday += days;
-        self.changes.insert(TMChanges::MDAY|TMChanges::WDAY);
         self.wrap_time();
     }
 
@@ -81,14 +64,12 @@ impl TmEditor {
         self.t.tm_hour = hour;
         self.t.tm_min = min;
         self.t.tm_sec = sec;
-        self.changes.insert(TMChanges::HOUR|TMChanges::MIN|TMChanges::SEC);
         self.wrap_time();
     }
 
     pub fn set_min_sec(&mut self, min: libc::c_int, sec: libc::c_int) {
         self.t.tm_min = min;
         self.t.tm_sec = sec;
-        self.changes.insert(TMChanges::MIN|TMChanges::SEC);
         self.wrap_time();
     }
 
@@ -98,14 +79,12 @@ impl TmEditor {
         if self.t.tm_sec >= 60 {
             self.t.tm_min += self.t.tm_sec / 60;
             self.t.tm_sec %= 60;
-            self.changes.insert(TMChanges::SEC|TMChanges::MIN);
         }
 
         // min: 0..59
         if self.t.tm_min >= 60 {
             self.t.tm_hour += self.t.tm_min / 60;
             self.t.tm_min %= 60;
-            self.changes.insert(TMChanges::MIN|TMChanges::HOUR);
        }
 
         // hour: 0..23
@@ -113,7 +92,6 @@ impl TmEditor {
             self.t.tm_mday += self.t.tm_hour / 24;
             self.t.tm_wday += self.t.tm_hour / 24;
             self.t.tm_hour %= 24;
-            self.changes.insert(TMChanges::HOUR|TMChanges::MDAY|TMChanges::WDAY);
         }
 
         // Translate to 0..($days_in_mon-1)
@@ -124,7 +102,6 @@ impl TmEditor {
 	    // Wrap one month
 	    self.t.tm_mday -= days_in_mon;
 	    self.t.tm_mon += 1;
-            self.changes.insert(TMChanges::MDAY|TMChanges::WDAY|TMChanges::MON);
         }
 
         // Translate back to 1..$days_in_mon
@@ -134,7 +111,6 @@ impl TmEditor {
         if self.t.tm_mon >= 12 {
             self.t.tm_year += self.t.tm_mon / 12;
             self.t.tm_mon %= 12;
-            self.changes.insert(TMChanges::MON|TMChanges::YEAR);
         }
 
         self.t.tm_wday %= 7;
@@ -142,43 +118,36 @@ impl TmEditor {
 
     pub fn set_sec(&mut self, v: libc::c_int) {
         self.t.tm_sec = v;
-        self.changes.insert(TMChanges::SEC);
         self.wrap_time();
     }
 
     pub fn set_min(&mut self, v: libc::c_int) {
         self.t.tm_min = v;
-        self.changes.insert(TMChanges::MIN);
         self.wrap_time();
     }
 
     pub fn set_hour(&mut self, v: libc::c_int) {
         self.t.tm_hour = v;
-        self.changes.insert(TMChanges::HOUR);
         self.wrap_time();
     }
 
     pub fn set_mday(&mut self, v: libc::c_int) {
         self.t.tm_mday = v;
-        self.changes.insert(TMChanges::MDAY);
         self.wrap_time();
     }
 
     pub fn set_mon(&mut self, v: libc::c_int) {
         self.t.tm_mon = v;
-        self.changes.insert(TMChanges::MON);
         self.wrap_time();
     }
 
     pub fn set_year(&mut self, v: libc::c_int) {
         self.t.tm_year = v;
-        self.changes.insert(TMChanges::YEAR);
         self.wrap_time();
     }
 
     pub fn set_wday(&mut self, v: libc::c_int) {
         self.t.tm_wday = v;
-        self.changes.insert(TMChanges::WDAY);
         self.wrap_time();
     }
 
-- 
2.20.1





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 02/11] tools/systemd/time: let libc normalize time for us
  2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 01/11] tools/systemd/tm_editor: remove TMChanges optimization Dominik Csapak
@ 2020-09-04 12:33 ` Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 03/11] tools/systemd/time: move continue out of the if/else Dominik Csapak
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2020-09-04 12:33 UTC (permalink / raw)
  To: pbs-devel

mktime/gmtime can normalize time and even can handle special timezone
cases like the fact that the time 2:30 on specific day/timezone combos
do not exists

we have to convert the signature of all functions that use
normalize_time since mktime/gmtime can return an EOVERFLOW
but if this happens there is no way we can find a good time anyway

since normalize_time will always set wday according to the rest of the
time, remove set_wday

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/systemd/time.rs      |  16 ++---
 src/tools/systemd/tm_editor.rs | 109 ++++++++-------------------------
 2 files changed, 34 insertions(+), 91 deletions(-)

diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index 2a1bf8a3..0d114eb8 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -180,11 +180,11 @@ pub fn compute_next_event(
                     .find(|d| event.days.contains(WeekDays::from_bits(1<<d).unwrap()))
                 {
                     // try next day
-                    t.add_days(n - day_num, true);
+                    t.add_days(n - day_num, true)?;
                     continue;
                 } else {
                     // try next week
-                    t.add_days(7 - day_num, true);
+                    t.add_days(7 - day_num, true)?;
                     continue;
                 }
             }
@@ -196,11 +196,11 @@ pub fn compute_next_event(
             if !DateTimeValue::list_contains(&event.hour, hour) {
                 if let Some(n) = DateTimeValue::find_next(&event.hour, hour) {
                     // test next hour
-                    t.set_time(n as libc::c_int, 0, 0);
+                    t.set_time(n as libc::c_int, 0, 0)?;
                     continue;
                 } else {
                     // test next day
-                    t.add_days(1, true);
+                    t.add_days(1, true)?;
                     continue;
                 }
             }
@@ -212,11 +212,11 @@ pub fn compute_next_event(
             if !DateTimeValue::list_contains(&event.minute, minute) {
                 if let Some(n) = DateTimeValue::find_next(&event.minute, minute) {
                     // test next minute
-                    t.set_min_sec(n as libc::c_int, 0);
+                    t.set_min_sec(n as libc::c_int, 0)?;
                     continue;
                 } else {
                     // test next hour
-                    t.set_time(t.hour() + 1, 0, 0);
+                    t.set_time(t.hour() + 1, 0, 0)?;
                     continue;
                 }
             }
@@ -228,11 +228,11 @@ pub fn compute_next_event(
             if !DateTimeValue::list_contains(&event.second, second) {
                 if let Some(n) = DateTimeValue::find_next(&event.second, second) {
                     // test next second
-                    t.set_sec(n as libc::c_int);
+                    t.set_sec(n as libc::c_int)?;
                     continue;
                 } else {
                     // test next min
-                    t.set_min_sec(t.min() + 1, 0);
+                    t.set_min_sec(t.min() + 1, 0)?;
                     continue;
                 }
             }
diff --git a/src/tools/systemd/tm_editor.rs b/src/tools/systemd/tm_editor.rs
index 1e8e4d07..4a14a975 100644
--- a/src/tools/systemd/tm_editor.rs
+++ b/src/tools/systemd/tm_editor.rs
@@ -7,24 +7,6 @@ pub struct TmEditor {
     t: libc::tm,
 }
 
-fn is_leap_year(year: libc::c_int) -> bool {
-    if year % 4 != 0  { return false; }
-    if year % 100 != 0 { return true; }
-    if year % 400 != 0  { return false; }
-    return true;
-}
-
-fn days_in_month(mon: libc::c_int, year: libc::c_int) -> libc::c_int {
-
-    let mon = mon % 12;
-
-    static MAP: &[libc::c_int] = &[31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31];
-
-    if mon == 1 && is_leap_year(year) { return 29; }
-
-    MAP[mon as usize]
-}
-
 impl TmEditor {
 
     pub fn new(epoch: i64, utc: bool) -> Result<Self, Error> {
@@ -39,8 +21,8 @@ impl TmEditor {
         Ok(epoch)
     }
 
-    pub fn add_days(&mut self, days: libc::c_int, reset_time: bool) {
-        if days == 0 { return; }
+    pub fn add_days(&mut self, days: libc::c_int, reset_time: bool) -> Result<(), Error> {
+        if days == 0 { return Ok(()); }
         if reset_time {
             self.t.tm_hour = 0;
             self.t.tm_min = 0;
@@ -48,7 +30,7 @@ impl TmEditor {
         }
         self.t.tm_mday += days;
         self.t.tm_wday += days;
-        self.wrap_time();
+        self.normalize_time()
     }
 
     pub fn hour(&self) -> libc::c_int { self.t.tm_hour }
@@ -60,95 +42,56 @@ impl TmEditor {
         (self.t.tm_wday + 6) % 7
     }
 
-    pub fn set_time(&mut self, hour: libc::c_int, min: libc::c_int, sec: libc::c_int) {
+    pub fn set_time(&mut self, hour: libc::c_int, min: libc::c_int, sec: libc::c_int) -> Result<(), Error> {
         self.t.tm_hour = hour;
         self.t.tm_min = min;
         self.t.tm_sec = sec;
-        self.wrap_time();
+        self.normalize_time()
     }
 
-    pub fn set_min_sec(&mut self, min: libc::c_int, sec: libc::c_int) {
+    pub fn set_min_sec(&mut self, min: libc::c_int, sec: libc::c_int) -> Result<(), Error> {
         self.t.tm_min = min;
         self.t.tm_sec = sec;
-        self.wrap_time();
+        self.normalize_time()
     }
 
-    fn wrap_time(&mut self) {
-
-        // sec: 0..59
-        if self.t.tm_sec >= 60 {
-            self.t.tm_min += self.t.tm_sec / 60;
-            self.t.tm_sec %= 60;
-        }
-
-        // min: 0..59
-        if self.t.tm_min >= 60 {
-            self.t.tm_hour += self.t.tm_min / 60;
-            self.t.tm_min %= 60;
-       }
-
-        // hour: 0..23
-        if self.t.tm_hour >= 24 {
-            self.t.tm_mday += self.t.tm_hour / 24;
-            self.t.tm_wday += self.t.tm_hour / 24;
-            self.t.tm_hour %= 24;
-        }
-
-        // Translate to 0..($days_in_mon-1)
-        self.t.tm_mday -= 1;
-        loop {
-	    let days_in_mon = days_in_month(self.t.tm_mon, self.t.tm_year);
-	    if self.t.tm_mday < days_in_mon { break; }
-	    // Wrap one month
-	    self.t.tm_mday -= days_in_mon;
-	    self.t.tm_mon += 1;
-        }
-
-        // Translate back to 1..$days_in_mon
-        self.t.tm_mday += 1;
-
-        // mon: 0..11
-        if self.t.tm_mon >= 12 {
-            self.t.tm_year += self.t.tm_mon / 12;
-            self.t.tm_mon %= 12;
+    fn normalize_time(&mut self) -> Result<(), Error> {
+        // libc normalizes it for us
+        if self.utc {
+            timegm(&mut self.t)?;
+        } else {
+            timelocal(&mut self.t)?;
         }
-
-        self.t.tm_wday %= 7;
+        Ok(())
     }
 
-    pub fn set_sec(&mut self, v: libc::c_int) {
+    pub fn set_sec(&mut self, v: libc::c_int) -> Result<(), Error> {
         self.t.tm_sec = v;
-        self.wrap_time();
+        self.normalize_time()
     }
 
-    pub fn set_min(&mut self, v: libc::c_int) {
+    pub fn set_min(&mut self, v: libc::c_int) -> Result<(), Error> {
         self.t.tm_min = v;
-        self.wrap_time();
+        self.normalize_time()
     }
 
-    pub fn set_hour(&mut self, v: libc::c_int) {
+    pub fn set_hour(&mut self, v: libc::c_int) -> Result<(), Error> {
         self.t.tm_hour = v;
-        self.wrap_time();
+        self.normalize_time()
     }
 
-    pub fn set_mday(&mut self, v: libc::c_int) {
+    pub fn set_mday(&mut self, v: libc::c_int) -> Result<(), Error> {
         self.t.tm_mday = v;
-        self.wrap_time();
+        self.normalize_time()
     }
 
-    pub fn set_mon(&mut self, v: libc::c_int) {
+    pub fn set_mon(&mut self, v: libc::c_int) -> Result<(), Error> {
         self.t.tm_mon = v;
-        self.wrap_time();
+        self.normalize_time()
     }
 
-    pub fn set_year(&mut self, v: libc::c_int) {
+    pub fn set_year(&mut self, v: libc::c_int) -> Result<(), Error> {
         self.t.tm_year = v;
-        self.wrap_time();
+        self.normalize_time()
     }
-
-    pub fn set_wday(&mut self, v: libc::c_int) {
-        self.t.tm_wday = v;
-        self.wrap_time();
-    }
-
 }
-- 
2.20.1





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 03/11] tools/systemd/time: move continue out of the if/else
  2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 01/11] tools/systemd/tm_editor: remove TMChanges optimization Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 02/11] tools/systemd/time: let libc normalize time for us Dominik Csapak
@ 2020-09-04 12:33 ` Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 04/11] tools/systemd/time: convert the resulting timestamp into an option Dominik Csapak
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2020-09-04 12:33 UTC (permalink / raw)
  To: pbs-devel

will be called anyway

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/systemd/time.rs | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index 0d114eb8..8bb2a8f7 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -181,12 +181,11 @@ pub fn compute_next_event(
                 {
                     // try next day
                     t.add_days(n - day_num, true)?;
-                    continue;
                 } else {
                     // try next week
                     t.add_days(7 - day_num, true)?;
-                    continue;
                 }
+                continue;
             }
         }
 
@@ -197,12 +196,11 @@ pub fn compute_next_event(
                 if let Some(n) = DateTimeValue::find_next(&event.hour, hour) {
                     // test next hour
                     t.set_time(n as libc::c_int, 0, 0)?;
-                    continue;
                 } else {
                     // test next day
                     t.add_days(1, true)?;
-                    continue;
                 }
+                continue;
             }
         }
 
@@ -213,12 +211,11 @@ pub fn compute_next_event(
                 if let Some(n) = DateTimeValue::find_next(&event.minute, minute) {
                     // test next minute
                     t.set_min_sec(n as libc::c_int, 0)?;
-                    continue;
                 } else {
                     // test next hour
                     t.set_time(t.hour() + 1, 0, 0)?;
-                    continue;
                 }
+                continue;
             }
         }
 
@@ -229,12 +226,11 @@ pub fn compute_next_event(
                 if let Some(n) = DateTimeValue::find_next(&event.second, second) {
                     // test next second
                     t.set_sec(n as libc::c_int)?;
-                    continue;
                 } else {
                     // test next min
                     t.set_min_sec(t.min() + 1, 0)?;
-                    continue;
                 }
+                continue;
             }
         }
 
-- 
2.20.1





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 04/11] tools/systemd/time: convert the resulting timestamp into an option
  2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
                   ` (2 preceding siblings ...)
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 03/11] tools/systemd/time: move continue out of the if/else Dominik Csapak
@ 2020-09-04 12:33 ` Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 05/11] tools/systemd/tm_editor: remove reset_time from add_days and document it Dominik Csapak
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2020-09-04 12:33 UTC (permalink / raw)
  To: pbs-devel

we want to use dates for the calendarspec, and with that there are some
impossible combinations that cannot be detected during parsing
(e.g. some datetimes do not exist in some timezones, and the timezone
can change after setting the schedule)

so finding no timestamp is not an error anymore but a valid result

we omit logging in that case (since it is not an error anymore)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/admin/sync.rs          |  3 ++-
 src/bin/proxmox-backup-proxy.rs |  9 ++++++---
 src/tools/systemd/time.rs       | 16 ++++++++++------
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/api2/admin/sync.rs b/src/api2/admin/sync.rs
index ff1e5ebf..bea52a0a 100644
--- a/src/api2/admin/sync.rs
+++ b/src/api2/admin/sync.rs
@@ -57,7 +57,8 @@ pub fn list_sync_jobs(
         job.next_run = (|| -> Option<i64> {
             let schedule = job.schedule.as_ref()?;
             let event = parse_calendar_event(&schedule).ok()?;
-            compute_next_event(&event, last, false).ok()
+            // ignore errors
+            compute_next_event(&event, last, false).unwrap_or_else(|_| None)
         })();
     }
 
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index dd081dfe..2b43c5ac 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -301,7 +301,8 @@ async fn schedule_datastore_garbage_collection() {
         };
 
         let next = match compute_next_event(&event, last, false) {
-            Ok(next) => next,
+            Ok(Some(next)) => next,
+            Ok(None) => continue,
             Err(err) => {
                 eprintln!("compute_next_event for '{}' failed - {}", event_str, err);
                 continue;
@@ -412,7 +413,8 @@ async fn schedule_datastore_prune() {
         };
 
         let next = match compute_next_event(&event, last, false) {
-            Ok(next) => next,
+            Ok(Some(next)) => next,
+            Ok(None) => continue,
             Err(err) => {
                 eprintln!("compute_next_event for '{}' failed - {}", event_str, err);
                 continue;
@@ -520,7 +522,8 @@ async fn schedule_datastore_sync_jobs() {
         };
 
         let next = match compute_next_event(&event, last, false) {
-            Ok(next) => next,
+            Ok(Some(next)) => next,
+            Ok(None) => continue,
             Err(err) => {
                 eprintln!("compute_next_event for '{}' failed - {}", event_str, err);
                 continue;
diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index 8bb2a8f7..2e99e289 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -1,4 +1,4 @@
-use anyhow::{bail, Error};
+use anyhow::Error;
 use bitflags::bitflags;
 
 pub use super::parse_time::*;
@@ -155,7 +155,7 @@ pub fn compute_next_event(
     event: &CalendarEvent,
     last: i64,
     utc: bool,
-) -> Result<i64, Error> {
+) -> Result<Option<i64>, Error> {
 
     let last = last + 1; // at least one second later
 
@@ -166,8 +166,9 @@ pub fn compute_next_event(
     let mut count = 0;
 
     loop {
-        if count > 1000 { // should not happen
-            bail!("unable to compute next calendar event");
+        // cancel after 1000 loops
+        if count > 1000 {
+            return Ok(None);
         } else {
             count += 1;
         }
@@ -235,13 +236,15 @@ pub fn compute_next_event(
         }
 
         let next = t.into_epoch()?;
-        return Ok(next)
+        return Ok(Some(next))
     }
 }
 
 #[cfg(test)]
 mod test {
 
+    use anyhow::bail;
+
     use super::*;
     use proxmox::tools::time::*;
 
@@ -268,7 +271,7 @@ mod test {
             };
 
             match compute_next_event(&event, last, true) {
-                Ok(next) => {
+                Ok(Some(next)) => {
                     if next == expect {
                         println!("next {:?} => {}", event, next);
                     } else {
@@ -276,6 +279,7 @@ mod test {
                               event, gmtime(next), gmtime(expect));
                     }
                 }
+                Ok(None) => bail!("next {:?} failed to find a timestamp", event),
                 Err(err) => bail!("compute next for '{}' failed - {}", v, err),
             }
 
-- 
2.20.1





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 05/11] tools/systemd/tm_editor: remove reset_time from add_days and document it
  2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
                   ` (3 preceding siblings ...)
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 04/11] tools/systemd/time: convert the resulting timestamp into an option Dominik Csapak
@ 2020-09-04 12:33 ` Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 06/11] tools/systemd/parse_time: error out on invalid ranges Dominik Csapak
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2020-09-04 12:33 UTC (permalink / raw)
  To: pbs-devel

we never passed 'false' to it anyway so remove it
(we can add it again if we should ever need it)

also remove the adding of wday (gets normalized anyway)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/systemd/time.rs      |  6 +++---
 src/tools/systemd/tm_editor.rs | 12 +++++-------
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index 2e99e289..c8cc8468 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -181,10 +181,10 @@ pub fn compute_next_event(
                     .find(|d| event.days.contains(WeekDays::from_bits(1<<d).unwrap()))
                 {
                     // try next day
-                    t.add_days(n - day_num, true)?;
+                    t.add_days(n - day_num,)?;
                 } else {
                     // try next week
-                    t.add_days(7 - day_num, true)?;
+                    t.add_days(7 - day_num)?;
                 }
                 continue;
             }
@@ -199,7 +199,7 @@ pub fn compute_next_event(
                     t.set_time(n as libc::c_int, 0, 0)?;
                 } else {
                     // test next day
-                    t.add_days(1, true)?;
+                    t.add_days(1)?;
                 }
                 continue;
             }
diff --git a/src/tools/systemd/tm_editor.rs b/src/tools/systemd/tm_editor.rs
index 4a14a975..b098f1cb 100644
--- a/src/tools/systemd/tm_editor.rs
+++ b/src/tools/systemd/tm_editor.rs
@@ -21,15 +21,13 @@ impl TmEditor {
         Ok(epoch)
     }
 
-    pub fn add_days(&mut self, days: libc::c_int, reset_time: bool) -> Result<(), Error> {
+    /// increases the day by 'days' and resets all smaller fields to their minimum
+    pub fn add_days(&mut self, days: libc::c_int) -> Result<(), Error> {
         if days == 0 { return Ok(()); }
-        if reset_time {
-            self.t.tm_hour = 0;
-            self.t.tm_min = 0;
-            self.t.tm_sec = 0;
-        }
+        self.t.tm_hour = 0;
+        self.t.tm_min = 0;
+        self.t.tm_sec = 0;
         self.t.tm_mday += days;
-        self.t.tm_wday += days;
         self.normalize_time()
     }
 
-- 
2.20.1





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 06/11] tools/systemd/parse_time: error out on invalid ranges
  2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
                   ` (4 preceding siblings ...)
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 05/11] tools/systemd/tm_editor: remove reset_time from add_days and document it Dominik Csapak
@ 2020-09-04 12:33 ` Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 07/11] tools/systemd/time: fix selection for multiple options Dominik Csapak
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2020-09-04 12:33 UTC (permalink / raw)
  To: pbs-devel

if the range is reverse (bigger..smaller) we will never find a value,
so error out during parsing

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/systemd/parse_time.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/tools/systemd/parse_time.rs b/src/tools/systemd/parse_time.rs
index 93657b21..051c4968 100644
--- a/src/tools/systemd/parse_time.rs
+++ b/src/tools/systemd/parse_time.rs
@@ -145,6 +145,9 @@ fn parse_date_time_comp(max: usize) -> impl Fn(&str) -> IResult<&str, DateTimeVa
         let (i, value) = parse_time_comp(max)(i)?;
 
         if let (i, Some(end)) = opt(preceded(tag(".."), parse_time_comp(max)))(i)? {
+            if value > end {
+                return Err(parse_error(i, "range start is bigger than end"));
+            }
             return Ok((i, DateTimeValue::Range(value, end)))
         }
 
-- 
2.20.1





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 07/11] tools/systemd/time: fix selection for multiple options
  2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
                   ` (5 preceding siblings ...)
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 06/11] tools/systemd/parse_time: error out on invalid ranges Dominik Csapak
@ 2020-09-04 12:33 ` Dominik Csapak
  2020-09-04 13:38   ` Dietmar Maurer
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 08/11] tools/systemd/tm_editor: move conversion of the year into getter and setter Dominik Csapak
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 15+ messages in thread
From: Dominik Csapak @ 2020-09-04 12:33 UTC (permalink / raw)
  To: pbs-devel

if we give multiple options/ranges for a value, e.g.
2,4,8
we always choose the biggest, instead of the smallest that is next

this happens because in DateTimeValue::find_next(value)
'next' can be set multiple times and we set it when the new
value was *bigger* than the last found 'next' value, when in reality
we have to choose the *smallest* next we can find

reverse the comparison operator to fix this

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/systemd/time.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index c8cc8468..69f5f5fb 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -54,7 +54,7 @@ impl DateTimeValue {
         let mut next: Option<u32> = None;
         let mut set_next = |v: u32| {
             if let Some(n) = next {
-                if v > n { next = Some(v); }
+                if v < n { next = Some(v); }
             } else {
                 next = Some(v);
             }
-- 
2.20.1





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 08/11] tools/systemd/tm_editor: move conversion of the year into getter and setter
  2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
                   ` (6 preceding siblings ...)
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 07/11] tools/systemd/time: fix selection for multiple options Dominik Csapak
@ 2020-09-04 12:33 ` Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 09/11] tools/systemd/tm_editor: add setter/getter for months/years/days Dominik Csapak
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2020-09-04 12:33 UTC (permalink / raw)
  To: pbs-devel

the tm struct contains the year - 1900 but we added that

if we want to use the libc normalization correctly, the tm struct
must have the correct year in it, else the computations for timezones,
etc. fail

instead add a getter that adds the years and a setter that subtracts it again

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/systemd/tm_editor.rs | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/tools/systemd/tm_editor.rs b/src/tools/systemd/tm_editor.rs
index b098f1cb..677e0520 100644
--- a/src/tools/systemd/tm_editor.rs
+++ b/src/tools/systemd/tm_editor.rs
@@ -10,13 +10,11 @@ pub struct TmEditor {
 impl TmEditor {
 
     pub fn new(epoch: i64, utc: bool) -> Result<Self, Error> {
-        let mut t = if utc { gmtime(epoch)? } else { localtime(epoch)? };
-        t.tm_year += 1900; // real years for clarity
+        let t = if utc { gmtime(epoch)? } else { localtime(epoch)? };
         Ok(Self { utc, t })
     }
 
     pub fn into_epoch(mut self) -> Result<i64, Error> {
-        self.t.tm_year -= 1900;
         let epoch = if self.utc { timegm(&mut self.t)? } else { timelocal(&mut self.t)? };
         Ok(epoch)
     }
@@ -31,6 +29,7 @@ impl TmEditor {
         self.normalize_time()
     }
 
+    pub fn year(&self) -> libc::c_int { self.t.tm_year + 1900 } // see man mktime
     pub fn hour(&self) -> libc::c_int { self.t.tm_hour }
     pub fn min(&self) -> libc::c_int { self.t.tm_min }
     pub fn sec(&self) -> libc::c_int { self.t.tm_sec }
@@ -89,7 +88,7 @@ impl TmEditor {
     }
 
     pub fn set_year(&mut self, v: libc::c_int) -> Result<(), Error> {
-        self.t.tm_year = v;
+        self.t.tm_year = v - 1900;
         self.normalize_time()
     }
 }
-- 
2.20.1





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 09/11] tools/systemd/tm_editor: add setter/getter for months/years/days
  2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
                   ` (7 preceding siblings ...)
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 08/11] tools/systemd/tm_editor: move conversion of the year into getter and setter Dominik Csapak
@ 2020-09-04 12:33 ` Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 10/11] tools/systemd/time: fix signed conversion Dominik Csapak
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2020-09-04 12:33 UTC (permalink / raw)
  To: pbs-devel

add_* are modeled after add_days

subtract one for set_mon to have a consistent interface for all fields
(i.e. getter/setter return/expect the 'real' number, not the ones
in the tm struct)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/systemd/tm_editor.rs | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/tools/systemd/tm_editor.rs b/src/tools/systemd/tm_editor.rs
index 677e0520..770bb280 100644
--- a/src/tools/systemd/tm_editor.rs
+++ b/src/tools/systemd/tm_editor.rs
@@ -19,6 +19,29 @@ impl TmEditor {
         Ok(epoch)
     }
 
+    /// increases the year by 'years' and resets all smaller fields to their minimum
+    pub fn add_years(&mut self, years: libc::c_int) -> Result<(), Error> {
+        if years == 0 { return Ok(()); }
+        self.t.tm_mon = 0;
+        self.t.tm_mday = 1;
+        self.t.tm_hour = 0;
+        self.t.tm_min = 0;
+        self.t.tm_sec = 0;
+        self.t.tm_year += years;
+        self.normalize_time()
+    }
+
+    /// increases the month by 'months' and resets all smaller fields to their minimum
+    pub fn add_months(&mut self, months: libc::c_int) -> Result<(), Error> {
+        if months == 0 { return Ok(()); }
+        self.t.tm_mday = 1;
+        self.t.tm_hour = 0;
+        self.t.tm_min = 0;
+        self.t.tm_sec = 0;
+        self.t.tm_mon += months;
+        self.normalize_time()
+    }
+
     /// increases the day by 'days' and resets all smaller fields to their minimum
     pub fn add_days(&mut self, days: libc::c_int) -> Result<(), Error> {
         if days == 0 { return Ok(()); }
@@ -30,6 +53,8 @@ impl TmEditor {
     }
 
     pub fn year(&self) -> libc::c_int { self.t.tm_year + 1900 } // see man mktime
+    pub fn month(&self) -> libc::c_int { self.t.tm_mon + 1 }
+    pub fn day(&self) -> libc::c_int { self.t.tm_mday }
     pub fn hour(&self) -> libc::c_int { self.t.tm_hour }
     pub fn min(&self) -> libc::c_int { self.t.tm_min }
     pub fn sec(&self) -> libc::c_int { self.t.tm_sec }
@@ -83,7 +108,7 @@ impl TmEditor {
     }
 
     pub fn set_mon(&mut self, v: libc::c_int) -> Result<(), Error> {
-        self.t.tm_mon = v;
+        self.t.tm_mon = v - 1;
         self.normalize_time()
     }
 
-- 
2.20.1





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 10/11] tools/systemd/time: fix signed conversion
  2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
                   ` (8 preceding siblings ...)
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 09/11] tools/systemd/tm_editor: add setter/getter for months/years/days Dominik Csapak
@ 2020-09-04 12:33 ` Dominik Csapak
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 11/11] tools/systemd/time: enable dates for calendarevents Dominik Csapak
  2020-09-04 13:36 ` [pbs-devel] applied: [PATCH proxmox-backup v2 00/11] implement " Dietmar Maurer
  11 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2020-09-04 12:33 UTC (permalink / raw)
  To: pbs-devel

instead of using 'as' and silently converting wrong,
use the TryInto trait and raise an error if we cannot convert

this should only happen if we have a negative year,
but this is expected (we do not want schedules from before the year 0)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/systemd/time.rs | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index 69f5f5fb..7717ecee 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -1,3 +1,5 @@
+use std::convert::TryInto;
+
 use anyhow::Error;
 use bitflags::bitflags;
 
@@ -174,17 +176,17 @@ pub fn compute_next_event(
         }
 
         if !all_days { // match day first
-            let day_num = t.day_num();
+            let day_num: u32 = t.day_num().try_into()?;
             let day = WeekDays::from_bits(1<<day_num).unwrap();
             if !event.days.contains(day) {
                 if let Some(n) = ((day_num+1)..7)
                     .find(|d| event.days.contains(WeekDays::from_bits(1<<d).unwrap()))
                 {
                     // try next day
-                    t.add_days(n - day_num,)?;
+                    t.add_days((n - day_num).try_into()?)?;
                 } else {
                     // try next week
-                    t.add_days(7 - day_num)?;
+                    t.add_days((7 - day_num).try_into()?)?;
                 }
                 continue;
             }
@@ -192,11 +194,11 @@ pub fn compute_next_event(
 
         // this day
         if !event.hour.is_empty() {
-            let hour = t.hour() as u32;
+            let hour = t.hour().try_into()?;
             if !DateTimeValue::list_contains(&event.hour, hour) {
                 if let Some(n) = DateTimeValue::find_next(&event.hour, hour) {
                     // test next hour
-                    t.set_time(n as libc::c_int, 0, 0)?;
+                    t.set_time(n.try_into()?, 0, 0)?;
                 } else {
                     // test next day
                     t.add_days(1)?;
@@ -207,11 +209,11 @@ pub fn compute_next_event(
 
         // this hour
         if !event.minute.is_empty() {
-            let minute = t.min() as u32;
+            let minute = t.min().try_into()?;
             if !DateTimeValue::list_contains(&event.minute, minute) {
                 if let Some(n) = DateTimeValue::find_next(&event.minute, minute) {
                     // test next minute
-                    t.set_min_sec(n as libc::c_int, 0)?;
+                    t.set_min_sec(n.try_into()?, 0)?;
                 } else {
                     // test next hour
                     t.set_time(t.hour() + 1, 0, 0)?;
@@ -222,11 +224,11 @@ pub fn compute_next_event(
 
         // this minute
         if !event.second.is_empty() {
-            let second = t.sec() as u32;
+            let second = t.sec().try_into()?;
             if !DateTimeValue::list_contains(&event.second, second) {
                 if let Some(n) = DateTimeValue::find_next(&event.second, second) {
                     // test next second
-                    t.set_sec(n as libc::c_int)?;
+                    t.set_sec(n.try_into()?)?;
                 } else {
                     // test next min
                     t.set_min_sec(t.min() + 1, 0)?;
-- 
2.20.1





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 11/11] tools/systemd/time: enable dates for calendarevents
  2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
                   ` (9 preceding siblings ...)
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 10/11] tools/systemd/time: fix signed conversion Dominik Csapak
@ 2020-09-04 12:33 ` Dominik Csapak
  2020-09-04 13:36 ` [pbs-devel] applied: [PATCH proxmox-backup v2 00/11] implement " Dietmar Maurer
  11 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2020-09-04 12:33 UTC (permalink / raw)
  To: pbs-devel

this implements parsing and calculating calendarevents that have a
basic date component (year-mon-day) with the usual syntax options
(*, ranges, lists)

and some special events:
monthly
yearly/annually (like systemd)
quarterly
semiannually,semi-annually (like systemd)

includes some regression tests

the ~ syntax for days (the last x days of the month) is not yet
implemented

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/systemd/parse_time.rs | 77 +++++++++++++++++++++++++++++++--
 src/tools/systemd/time.rs       | 70 +++++++++++++++++++++++++++++-
 2 files changed, 141 insertions(+), 6 deletions(-)

diff --git a/src/tools/systemd/parse_time.rs b/src/tools/systemd/parse_time.rs
index 051c4968..d30a1acc 100644
--- a/src/tools/systemd/parse_time.rs
+++ b/src/tools/systemd/parse_time.rs
@@ -186,6 +186,25 @@ fn parse_time_spec(i: &str) -> IResult<&str, (Vec<DateTimeValue>, Vec<DateTimeVa
     }
 }
 
+fn parse_date_spec(i: &str) -> IResult<&str, (Vec<DateTimeValue>, Vec<DateTimeValue>, Vec<DateTimeValue>)> {
+
+    // TODO: implement ~ for days (man systemd.time)
+    if let Ok((i, (year, month, day))) = tuple((
+        parse_date_time_comp_list(2200), // the upper limit for systemd, stay compatible
+        preceded(tag("-"), parse_date_time_comp_list(13)),
+        preceded(tag("-"), parse_date_time_comp_list(32)),
+    ))(i) {
+        Ok((i, (year, month, day)))
+    } else if let Ok((i, (month, day))) = tuple((
+        parse_date_time_comp_list(13),
+        preceded(tag("-"), parse_date_time_comp_list(32)),
+    ))(i) {
+        Ok((i, (Vec::new(), month, day)))
+    } else {
+        Err(parse_error(i, "invalid date spec"))
+    }
+}
+
 pub fn parse_calendar_event(i: &str) -> Result<CalendarEvent, Error> {
     parse_complete_line("calendar event", i, parse_calendar_event_incomplete)
 }
@@ -194,7 +213,7 @@ fn parse_calendar_event_incomplete(mut i: &str) -> IResult<&str, CalendarEvent>
 
     let mut has_dayspec = false;
     let mut has_timespec = false;
-    let has_datespec = false;
+    let mut has_datespec = false;
 
     let mut event = CalendarEvent::default();
 
@@ -231,8 +250,52 @@ fn parse_calendar_event_incomplete(mut i: &str) -> IResult<&str, CalendarEvent>
                     ..Default::default()
                 }));
             }
-            "monthly" | "yearly" | "quarterly" | "semiannually" => {
-                return Err(parse_error(i, "unimplemented date or time specification"));
+            "monthly" => {
+                return Ok(("", CalendarEvent {
+                    hour: vec![DateTimeValue::Single(0)],
+                    minute: vec![DateTimeValue::Single(0)],
+                    second: vec![DateTimeValue::Single(0)],
+                    day: vec![DateTimeValue::Single(1)],
+                    ..Default::default()
+                }));
+            }
+            "yearly" | "annually" => {
+                return Ok(("", CalendarEvent {
+                    hour: vec![DateTimeValue::Single(0)],
+                    minute: vec![DateTimeValue::Single(0)],
+                    second: vec![DateTimeValue::Single(0)],
+                    day: vec![DateTimeValue::Single(1)],
+                    month: vec![DateTimeValue::Single(1)],
+                    ..Default::default()
+                }));
+            }
+            "quarterly" => {
+                return Ok(("", CalendarEvent {
+                    hour: vec![DateTimeValue::Single(0)],
+                    minute: vec![DateTimeValue::Single(0)],
+                    second: vec![DateTimeValue::Single(0)],
+                    day: vec![DateTimeValue::Single(1)],
+                    month: vec![
+                        DateTimeValue::Single(1),
+                        DateTimeValue::Single(4),
+                        DateTimeValue::Single(7),
+                        DateTimeValue::Single(10),
+                    ],
+                    ..Default::default()
+                }));
+            }
+            "semiannually" | "semi-annually" => {
+                return Ok(("", CalendarEvent {
+                    hour: vec![DateTimeValue::Single(0)],
+                    minute: vec![DateTimeValue::Single(0)],
+                    second: vec![DateTimeValue::Single(0)],
+                    day: vec![DateTimeValue::Single(1)],
+                    month: vec![
+                        DateTimeValue::Single(1),
+                        DateTimeValue::Single(7),
+                    ],
+                    ..Default::default()
+                }));
             }
             _ => { /* continue */ }
         }
@@ -249,7 +312,13 @@ fn parse_calendar_event_incomplete(mut i: &str) -> IResult<&str, CalendarEvent>
         for range in range_list  { event.days.insert(range); }
     }
 
-    // todo: support date specs
+    if let (n, Some((year, month, day))) = opt(parse_date_spec)(i)? {
+        event.year = year;
+        event.month = month;
+        event.day = day;
+        has_datespec = true;
+        i = space0(n)?.0;
+    }
 
     if let (n, Some((hour, minute, second))) = opt(parse_time_spec)(i)? {
         event.hour = hour;
diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index 7717ecee..c30e86f9 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -103,14 +103,12 @@ pub struct CalendarEvent {
     pub minute: Vec<DateTimeValue>,
     /// the hour(s) this event should trigger
     pub hour: Vec<DateTimeValue>,
-/* FIXME: TODO
     /// the day(s) in a month this event should trigger
     pub day: Vec<DateTimeValue>,
     /// the month(s) in a year this event should trigger
     pub month: Vec<DateTimeValue>,
     /// the years(s) this event should trigger
     pub year: Vec<DateTimeValue>,
-*/
 }
 
 #[derive(Default)]
@@ -175,6 +173,45 @@ pub fn compute_next_event(
             count += 1;
         }
 
+        if !event.year.is_empty() {
+            let year: u32 = t.year().try_into()?;
+            if !DateTimeValue::list_contains(&event.year, year) {
+                if let Some(n) = DateTimeValue::find_next(&event.year, year) {
+                    t.add_years((n - year).try_into()?)?;
+                    continue;
+                } else {
+                    // if we have no valid year, we cannot find a correct timestamp
+                    return Ok(None);
+                }
+            }
+        }
+
+        if !event.month.is_empty() {
+            let month: u32 = t.month().try_into()?;
+            if !DateTimeValue::list_contains(&event.month, month) {
+                if let Some(n) = DateTimeValue::find_next(&event.month, month) {
+                    t.add_months((n - month).try_into()?)?;
+                } else {
+                    // if we could not find valid month, retry next year
+                    t.add_years(1)?;
+                }
+                continue;
+            }
+        }
+
+        if !event.day.is_empty() {
+            let day: u32 = t.day().try_into()?;
+            if !DateTimeValue::list_contains(&event.day, day) {
+                if let Some(n) = DateTimeValue::find_next(&event.day, day) {
+                    t.add_days((n - day).try_into()?)?;
+                } else {
+                    // if we could not find valid mday, retry next month
+                    t.add_months(1)?;
+                }
+                continue;
+            }
+        }
+
         if !all_days { // match day first
             let day_num: u32 = t.day_num().try_into()?;
             let day = WeekDays::from_bits(1<<day_num).unwrap();
@@ -288,6 +325,18 @@ mod test {
             Ok(expect)
         };
 
+        let test_never = |v: &'static str, last: i64| -> Result<(), Error> {
+            let event = match parse_calendar_event(v) {
+                Ok(event) => event,
+                Err(err) => bail!("parsing '{}' failed - {}", v, err),
+            };
+
+            match compute_next_event(&event, last, true)? {
+                None => Ok(()),
+                Some(next) => bail!("compute next for '{}' succeeded, but expected fail - result {}", v, next),
+            }
+        };
+
         const MIN: i64 = 60;
         const HOUR: i64 = 3600;
         const DAY: i64 = 3600*24;
@@ -355,6 +404,23 @@ mod test {
             n = test_value("1:0", n, THURSDAY_00_00 + i*DAY + HOUR)?;
         }
 
+        // test date functionality
+
+        test_value("2020-07-31", 0, JUL_31_2020)?;
+        test_value("02-28", 0, (31+27)*DAY)?;
+        test_value("02-29", 0, 2*365*DAY + (31+28)*DAY)?; // 1972-02-29
+        test_value("1965/5-01-01", -1, THURSDAY_00_00)?;
+        test_value("2020-7..9-2/2", JUL_31_2020, JUL_31_2020 + 2*DAY)?;
+        test_value("2020,2021-12-31", JUL_31_2020, DEC_31_2020)?;
+
+        test_value("monthly", 0, 31*DAY)?;
+        test_value("quarterly", 0, (31+28+31)*DAY)?;
+        test_value("semiannually", 0, (31+28+31+30+31+30)*DAY)?;
+        test_value("yearly", 0, (365)*DAY)?;
+
+        test_never("2021-02-29", 0)?;
+        test_never("02-30", 0)?;
+
         Ok(())
     }
 
-- 
2.20.1





^ permalink raw reply	[flat|nested] 15+ messages in thread

* [pbs-devel] applied: [PATCH proxmox-backup v2 00/11] implement dates for calendarevents
  2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
                   ` (10 preceding siblings ...)
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 11/11] tools/systemd/time: enable dates for calendarevents Dominik Csapak
@ 2020-09-04 13:36 ` Dietmar Maurer
  11 siblings, 0 replies; 15+ messages in thread
From: Dietmar Maurer @ 2020-09-04 13:36 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak

applied. 

> On 09/04/2020 2:33 PM Dominik Csapak <d.csapak@proxmox.com> wrote:
> 
>  
> implements Dates and more special words for calendar events
> and fixes some small bugs
> 
> not implemented for calendarevents are Timezones, Unix Timestamps and
> the ~ operator for the last days of the month
> 
> changes from v1:
> * do not change DateTimeValues to i32 but fail for not
>   correctly convertable entries
>   (this makes years < 0 invalid but it's ok)
> * remove the TMChanges bitflags completely (its not faster, but more
>   complicated)
> 
> Dominik Csapak (11):
>   tools/systemd/tm_editor: remove TMChanges optimization
>   tools/systemd/time: let libc normalize time for us
>   tools/systemd/time: move continue out of the if/else
>   tools/systemd/time: convert the resulting timestamp into an option
>   tools/systemd/tm_editor: remove reset_time from add_days and document it
>   tools/systemd/parse_time: error out on invalid ranges
>   tools/systemd/time: fix selection for multiple options
>   tools/systemd/tm_editor: move conversion of the year into getter and setter
>   tools/systemd/tm_editor: add setter/getter for months/years/days
>   tools/systemd/time: fix signed conversion
>   tools/systemd/time: enable dates for calendarevents
> 
>  src/api2/admin/sync.rs          |   3 +-
>  src/bin/proxmox-backup-proxy.rs |   9 +-
>  src/tools/systemd/parse_time.rs |  80 +++++++++++++-
>  src/tools/systemd/time.rs       | 150 ++++++++++++++++++--------
>  src/tools/systemd/tm_editor.rs  | 186 +++++++++++---------------------
>  5 files changed, 249 insertions(+), 179 deletions(-)
> 
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v2 07/11] tools/systemd/time: fix selection for multiple options
  2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 07/11] tools/systemd/time: fix selection for multiple options Dominik Csapak
@ 2020-09-04 13:38   ` Dietmar Maurer
  2020-09-07  6:27     ` Dominik Csapak
  0 siblings, 1 reply; 15+ messages in thread
From: Dietmar Maurer @ 2020-09-04 13:38 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak

already applied, but I wonder if we have a test case for this?

> On 09/04/2020 2:33 PM Dominik Csapak <d.csapak@proxmox.com> wrote:
> 
>  
> if we give multiple options/ranges for a value, e.g.
> 2,4,8
> we always choose the biggest, instead of the smallest that is next
> 
> this happens because in DateTimeValue::find_next(value)
> 'next' can be set multiple times and we set it when the new
> value was *bigger* than the last found 'next' value, when in reality
> we have to choose the *smallest* next we can find
> 
> reverse the comparison operator to fix this
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/tools/systemd/time.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
> index c8cc8468..69f5f5fb 100644
> --- a/src/tools/systemd/time.rs
> +++ b/src/tools/systemd/time.rs
> @@ -54,7 +54,7 @@ impl DateTimeValue {
>          let mut next: Option<u32> = None;
>          let mut set_next = |v: u32| {
>              if let Some(n) = next {
> -                if v > n { next = Some(v); }
> +                if v < n { next = Some(v); }
>              } else {
>                  next = Some(v);
>              }
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v2 07/11] tools/systemd/time: fix selection for multiple options
  2020-09-04 13:38   ` Dietmar Maurer
@ 2020-09-07  6:27     ` Dominik Csapak
  0 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2020-09-07  6:27 UTC (permalink / raw)
  To: Dietmar Maurer, Proxmox Backup Server development discussion

On 9/4/20 3:38 PM, Dietmar Maurer wrote:
> already applied, but I wonder if we have a test case for this?
> 

no, but i'll send one. thanks




^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-09-07  6:28 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04 12:33 [pbs-devel] [PATCH proxmox-backup v2 00/11] implement dates for calendarevents Dominik Csapak
2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 01/11] tools/systemd/tm_editor: remove TMChanges optimization Dominik Csapak
2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 02/11] tools/systemd/time: let libc normalize time for us Dominik Csapak
2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 03/11] tools/systemd/time: move continue out of the if/else Dominik Csapak
2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 04/11] tools/systemd/time: convert the resulting timestamp into an option Dominik Csapak
2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 05/11] tools/systemd/tm_editor: remove reset_time from add_days and document it Dominik Csapak
2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 06/11] tools/systemd/parse_time: error out on invalid ranges Dominik Csapak
2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 07/11] tools/systemd/time: fix selection for multiple options Dominik Csapak
2020-09-04 13:38   ` Dietmar Maurer
2020-09-07  6:27     ` Dominik Csapak
2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 08/11] tools/systemd/tm_editor: move conversion of the year into getter and setter Dominik Csapak
2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 09/11] tools/systemd/tm_editor: add setter/getter for months/years/days Dominik Csapak
2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 10/11] tools/systemd/time: fix signed conversion Dominik Csapak
2020-09-04 12:33 ` [pbs-devel] [PATCH proxmox-backup v2 11/11] tools/systemd/time: enable dates for calendarevents Dominik Csapak
2020-09-04 13:36 ` [pbs-devel] applied: [PATCH proxmox-backup v2 00/11] implement " Dietmar Maurer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal