From 8432f5915d51ae485313cdd03ea0d9983e1425a9 Mon Sep 17 00:00:00 2001 From: GreekStapler Date: Sat, 7 Jan 2023 21:09:55 +0000 Subject: [PATCH 01/30] Fix for configured log level being ignored. --- src/agent/log.v | 29 +++++++++++++++-------------- src/cron/daemon/log.v | 29 +++++++++++++++-------------- src/web/logging.v | 29 +++++++++++++++-------------- 3 files changed, 45 insertions(+), 42 deletions(-) diff --git a/src/agent/log.v b/src/agent/log.v index cd59207..fcd8373 100644 --- a/src/agent/log.v +++ b/src/agent/log.v @@ -1,35 +1,36 @@ module agent -import log - -// log a message with the given level -pub fn (mut d AgentDaemon) log(msg string, level log.Level) { - lock d.logger { - d.logger.send_output(msg, level) - } -} - // lfatal create a log message with the fatal level pub fn (mut d AgentDaemon) lfatal(msg string) { - d.log(msg, log.Level.fatal) + lock d.logger { + d.logger.fatal(msg) + } } // lerror create a log message with the error level pub fn (mut d AgentDaemon) lerror(msg string) { - d.log(msg, log.Level.error) + lock d.logger { + d.logger.error(msg) + } } // lwarn create a log message with the warn level pub fn (mut d AgentDaemon) lwarn(msg string) { - d.log(msg, log.Level.warn) + lock d.logger { + d.logger.warn(msg) + } } // linfo create a log message with the info level pub fn (mut d AgentDaemon) linfo(msg string) { - d.log(msg, log.Level.info) + lock d.logger { + d.logger.info(msg) + } } // ldebug create a log message with the debug level pub fn (mut d AgentDaemon) ldebug(msg string) { - d.log(msg, log.Level.debug) + lock d.logger { + d.logger.debug(msg) + } } diff --git a/src/cron/daemon/log.v b/src/cron/daemon/log.v index 95a50e7..4f978fc 100644 --- a/src/cron/daemon/log.v +++ b/src/cron/daemon/log.v @@ -1,35 +1,36 @@ module daemon -import log - -// log reate a log message with the given level -pub fn (mut d Daemon) log(msg string, level log.Level) { - lock d.logger { - d.logger.send_output(msg, level) - } -} - // lfatal create a log message with the fatal level pub fn (mut d Daemon) lfatal(msg string) { - d.log(msg, log.Level.fatal) + lock d.logger { + d.logger.fatal(msg) + } } // lerror create a log message with the error level pub fn (mut d Daemon) lerror(msg string) { - d.log(msg, log.Level.error) + lock d.logger { + d.logger.error(msg) + } } // lwarn create a log message with the warn level pub fn (mut d Daemon) lwarn(msg string) { - d.log(msg, log.Level.warn) + lock d.logger { + d.logger.warn(msg) + } } // linfo create a log message with the info level pub fn (mut d Daemon) linfo(msg string) { - d.log(msg, log.Level.info) + lock d.logger { + d.logger.info(msg) + } } // ldebug create a log message with the debug level pub fn (mut d Daemon) ldebug(msg string) { - d.log(msg, log.Level.debug) + lock d.logger { + d.logger.debug(msg) + } } diff --git a/src/web/logging.v b/src/web/logging.v index 12b07d7..7ba649c 100644 --- a/src/web/logging.v +++ b/src/web/logging.v @@ -1,35 +1,36 @@ module web -import log - -// log reate a log message with the given level -pub fn (mut ctx Context) log(msg string, level log.Level) { - lock ctx.logger { - ctx.logger.send_output(msg, level) - } -} - // lfatal create a log message with the fatal level pub fn (mut ctx Context) lfatal(msg string) { - ctx.log(msg, log.Level.fatal) + lock ctx.logger { + ctx.logger.fatal(msg) + } } // lerror create a log message with the error level pub fn (mut ctx Context) lerror(msg string) { - ctx.log(msg, log.Level.error) + lock ctx.logger { + ctx.logger.error(msg) + } } // lwarn create a log message with the warn level pub fn (mut ctx Context) lwarn(msg string) { - ctx.log(msg, log.Level.warn) + lock ctx.logger { + ctx.logger.warn(msg) + } } // linfo create a log message with the info level pub fn (mut ctx Context) linfo(msg string) { - ctx.log(msg, log.Level.info) + lock ctx.logger { + ctx.logger.info(msg) + } } // ldebug create a log message with the debug level pub fn (mut ctx Context) ldebug(msg string) { - ctx.log(msg, log.Level.debug) + lock ctx.logger { + ctx.logger.debug(msg) + } } From 7490668b447422bdf910b72822e583270c3eb108 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Thu, 12 Jan 2023 12:26:12 +0100 Subject: [PATCH 02/30] wip --- src/cron/expression/c/expression.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 src/cron/expression/c/expression.h diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h new file mode 100644 index 0000000..079a72a --- /dev/null +++ b/src/cron/expression/c/expression.h @@ -0,0 +1,20 @@ +#include +#include + +typedef struct cron_expression { + uint8_t *minutes; + uint8_t *hours; + uint8_t *days; + uint8_t *months; + uint8_t minute_count; + uint8_t hour_count; + uint8_t day_count; + uint8_t month_count; +} CronExpression; + +/** + * Given a + */ +int ce_next(struct tm *out, struct tm *ref); + +int ce_parse(CronExpression *out, char *s); From c3f7f11686f1fc2cc4d04bbf52c2262a8237d825 Mon Sep 17 00:00:00 2001 From: Chewing_Bever Date: Thu, 12 Jan 2023 14:01:18 +0100 Subject: [PATCH 03/30] feat(cron): mostly written C expression parser --- .gitignore | 2 +- src/cron/expression/c/expression.h | 2 + src/cron/expression/c/parse.c | 179 +++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 src/cron/expression/c/parse.c diff --git a/.gitignore b/.gitignore index aaec9ef..daeb3d3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ -*.c +vieter.c /data/ # Build artifacts diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h index 079a72a..5b8e4f9 100644 --- a/src/cron/expression/c/expression.h +++ b/src/cron/expression/c/expression.h @@ -1,5 +1,7 @@ #include #include +#include +#include typedef struct cron_expression { uint8_t *minutes; diff --git a/src/cron/expression/c/parse.c b/src/cron/expression/c/parse.c new file mode 100644 index 0000000..17d416f --- /dev/null +++ b/src/cron/expression/c/parse.c @@ -0,0 +1,179 @@ +#include "expression.h" + +const uint8_t min[4] = {0, 0, 1, 1}; +const uint8_t max[4] = {59, 23, 31, 12}; + +typedef enum parse_error { + ParseOk = 0, + ParseInvalidExpression = 1, + ParseInvalidNumber = 2, + ParseOutOfRange = 3 +} ParseError; + +#define SAFE_ATOI(v,s,min,max) \ + int _##v = atoi(s); \ + if ((_##v) == 0 && strcmp((s), "0") != 0) { \ + return ParseInvalidNumber; \ + } \ + if (v < (min) || v > (max)) { \ + return ParseOutOfRange; \ + } \ + v = (uint8_t) (_##v); + +/** + * Given a range expression, produce a bit field defining what numbers in the + * min-max range the expression represents. The first bit (starting from the + * right) corresponds to min, the max - min + 1'th bit to max. All trailing bits + * after this should be ignored. The given bitfield is modified in-place, so + * multiple calls of this function can be performed on the same value to create + * the effect of ORing their values: + * + * A range expression has one of the following forms: + * + * - * + * - a + * - a-b + * - a/c + * - a-b/c + */ +ParseError ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { + // The * expression means "every possible value" + if (s[0] == '*') { + // A '*' is only valid on its own + if (s[1] != '\0') { + return ParseInvalidExpression; + } + + *out = ~0; + + return ParseOk; + } + + size_t slash_index = 0; + size_t dash_index = 0; + size_t i = 0; + + // We first iterate over the string to determine whether it contains a slash + // and/or a dash. We know the dash can only be valid if it appears before + // the slash. + while (s[i] != '\0' && slash_index == 0) { + if (s[i] == '/') { + slash_index = i; + + s[i] = '\0'; + } else if (s[i] == '-') { + dash_index = i; + + s[i] = '\0'; + } + + i++; + } + + // Parse the three possible numbers in the pattern + uint8_t start = 0; + uint8_t end = 0; + uint8_t interval = 1; + + SAFE_ATOI(start, s, min, max); + + if (dash_index > 0) { + SAFE_ATOI(end, &s[dash_index + 1], min, max); + } + + if (slash_index > 0) { + SAFE_ATOI(interval, &s[slash_index + 1], 1, max - min); + } + + // Single number doesn't need to loop + if (end == 0 && slash_index == 0) { + *out |= 1 << (start - min); + } else { + for (;start <= end; start += interval) { + *out |= 1 << (start - min); + start += interval; + } + } + + return ParseOk; +} + +ParseError ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { + *out = 0; + + char *next; + ParseError res; + + while ((next = strchr(s, ',')) != NULL) { + next[0] = '\0'; + res = ce_parse_range(out, s, min, max); + + if (res != ParseOk) { + return res; + } + + s = next + 1; + } + + // Make sure to parse the final range as well + res = ce_parse_range(out, s, min, max); + + if (res != ParseOk) { + return res; + } + + return ParseOk; +} + +ParseError ce_parse_expression(uint64_t *out, char *s) { + uint8_t part_count = 0; + + char *next; + ParseError res; + + // Skip leading spaces + while (s[0] == ' ') { + s++; + } + + while (part_count < 4 && (next = strchr(s, ' ')) != NULL) { + next[0] = '\0'; + res = ce_parse_part(&out[part_count], s, min[part_count], max[part_count]); + + if (res != ParseOk) { + return res; + } + + s = next + 1; + size_t offset = 1; + + // Skip multiple spaces + while (next[offset] == ' ') { + offset++; + } + s = next + offset; + + part_count++; + } + + // Parse final trailing part + if (part_count < 4 && s[0] != '\0') { + // Make sure to parse the final range as well + res = ce_parse_part(&out[part_count], s, min[part_count], max[part_count]); + + if (res != ParseOk) { + return res; + } + + part_count++; + } + + // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts + while (part_count < 4) { + // Expression is augmented with '*' expressions + out[part_count] = ~0; + part_count++; + } + + return ParseOk; +} From ee262a2fbc5a169164b57c3afc1f871e5ed21df2 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Thu, 12 Jan 2023 21:08:58 +0100 Subject: [PATCH 04/30] feat(cron): rest of parser in C --- src/cron/expression/c/expression.h | 9 ++++- src/cron/expression/c/parse.c | 55 ++++++++++++++++++++++++------ src/cron/expression/expression.c.v | 18 ++++++++++ src/cron/expression/v.mod | 0 4 files changed, 70 insertions(+), 12 deletions(-) create mode 100644 src/cron/expression/expression.c.v create mode 100644 src/cron/expression/v.mod diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h index 5b8e4f9..799a9a3 100644 --- a/src/cron/expression/c/expression.h +++ b/src/cron/expression/c/expression.h @@ -3,6 +3,13 @@ #include #include +typedef enum parse_error { + ParseOk = 0, + ParseInvalidExpression = 1, + ParseInvalidNumber = 2, + ParseOutOfRange = 3 +} ParseError; + typedef struct cron_expression { uint8_t *minutes; uint8_t *hours; @@ -19,4 +26,4 @@ typedef struct cron_expression { */ int ce_next(struct tm *out, struct tm *ref); -int ce_parse(CronExpression *out, char *s); +ParseError ce_parse_expression(CronExpression *out, char *s); diff --git a/src/cron/expression/c/parse.c b/src/cron/expression/c/parse.c index 17d416f..cb97373 100644 --- a/src/cron/expression/c/parse.c +++ b/src/cron/expression/c/parse.c @@ -3,13 +3,6 @@ const uint8_t min[4] = {0, 0, 1, 1}; const uint8_t max[4] = {59, 23, 31, 12}; -typedef enum parse_error { - ParseOk = 0, - ParseInvalidExpression = 1, - ParseInvalidNumber = 2, - ParseOutOfRange = 3 -} ParseError; - #define SAFE_ATOI(v,s,min,max) \ int _##v = atoi(s); \ if ((_##v) == 0 && strcmp((s), "0") != 0) { \ @@ -125,11 +118,41 @@ ParseError ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { return ParseOk; } -ParseError ce_parse_expression(uint64_t *out, char *s) { +uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { + uint8_t capacity = 8; + uint8_t size = 0; + + uint8_t *buf = malloc(capacity * sizeof(uint8_t)); + + for (uint8_t i = 0; i <= max - min; i++) { + if ((1 << i) & bf) { + // Resize buffer if needed + if (size == capacity) { + capacity *= 2; + buf = realloc(buf, capacity * sizeof(uint8_t)); + } + + buf[size] = min + i; + size++; + } + } + + // Resize buffer once more to remove any trailing unused bytes + if (size < capacity) { + buf = realloc(buf, size * sizeof(uint8_t)); + } + + *out = buf; + + return size; +} + +ParseError ce_parse_expression(CronExpression *out, char *s) { uint8_t part_count = 0; char *next; ParseError res; + uint64_t bfs[4]; // Skip leading spaces while (s[0] == ' ') { @@ -138,7 +161,7 @@ ParseError ce_parse_expression(uint64_t *out, char *s) { while (part_count < 4 && (next = strchr(s, ' ')) != NULL) { next[0] = '\0'; - res = ce_parse_part(&out[part_count], s, min[part_count], max[part_count]); + res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); if (res != ParseOk) { return res; @@ -159,7 +182,7 @@ ParseError ce_parse_expression(uint64_t *out, char *s) { // Parse final trailing part if (part_count < 4 && s[0] != '\0') { // Make sure to parse the final range as well - res = ce_parse_part(&out[part_count], s, min[part_count], max[part_count]); + res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); if (res != ParseOk) { return res; @@ -168,12 +191,22 @@ ParseError ce_parse_expression(uint64_t *out, char *s) { part_count++; } + // At least two parts need to be provided + if (part_count < 2) { + return ParseInvalidExpression; + } + // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts while (part_count < 4) { // Expression is augmented with '*' expressions - out[part_count] = ~0; + bfs[part_count] = ~0; part_count++; } + out->minute_count = bf_to_nums(&out->minutes, bfs[0], min[0], max[0]); + out->hour_count = bf_to_nums(&out->hours, bfs[1], min[1], max[1]); + out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); + out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); + return ParseOk; } diff --git a/src/cron/expression/expression.c.v b/src/cron/expression/expression.c.v new file mode 100644 index 0000000..ec39fd6 --- /dev/null +++ b/src/cron/expression/expression.c.v @@ -0,0 +1,18 @@ +module expression + +#flag -I @VMODROOT/c +#flag @VMODROOT/c/parse.o +#include "expression.h" + +pub struct C.CronExpression { + minutes &u8 + hours &u8 + days &u8 + months &u8 + minute_count u8 + hour_count u8 + day_count u8 + month_count u8 +} + +/* pub type CronExpression = C.CronExpression */ diff --git a/src/cron/expression/v.mod b/src/cron/expression/v.mod new file mode 100644 index 0000000..e69de29 From 85ea7166fbb887be54065548a7acc122e569053f Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Thu, 12 Jan 2023 21:31:32 +0100 Subject: [PATCH 05/30] feat(cron): next function in C --- src/cron/expression/c/expression.c | 90 ++++++++++++++++++++++++++++++ src/cron/expression/c/expression.h | 10 +++- 2 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 src/cron/expression/c/expression.c diff --git a/src/cron/expression/c/expression.c b/src/cron/expression/c/expression.c new file mode 100644 index 0000000..3f65b6a --- /dev/null +++ b/src/cron/expression/c/expression.c @@ -0,0 +1,90 @@ +#include "expression.h" + +const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}; + +int ce_next(SimpleTime *out, CronExpression *ce, SimpleTime *ref) { + // For all of these values, the rule is the following: if their value is + // the length of their respective array in the CronExpression object, that + // means we've looped back around. This means that the "bigger" value has + // to be incremented by one. For example, if the minutes have looped + // around, that means that the hour has to be incremented as well. + uint8_t month_index = 0; + uint8_t day_index = 0; + uint8_t hour_index = 0; + uint8_t minute_index = 0; + + // This chain is the same logic multiple times, namely that if a "bigger" + // value loops around, then the smaller value will always reset as well. + // For example, if we're going to a new day, the hour & minute will always + // be their smallest value again. + while (month_index < ce->month_count && ref->month > ce->months[month_index]) { + month_index++; + } + + if (month_index < ce->month_count && ref->month == ce->months[month_index]) { + while (day_index < ce->day_count && ref->day > ce->days[day_index]) { + day_index++; + } + + if (day_index < ce->days_count && ref->day == ce->days[day_index]) { + while (hour_index < ce->hour_count && ref->hour > ce->hours[hour_index]) { + hour_index++; + } + + if (hour_index < ce->hours_count && ref->hour == ce->hours[hour_index]) { + // Minute is the only value where we explicitely make sure we + // can't match sref's value exactly. This is to ensure we only + // return values in the future. + while (minute_index < ce->minute_count && ref->minute > ce->minutes[minute_index]) { + minute_index++; + } + } + } + } + + // Here, we increment the "bigger" values by one if the smaller ones loop + // around. The order is important, as it allows a sort-of waterfall effect + // to occur which updates all values if required. + if (minute_index == ce->minute_count && hour_index < ce->hour_count) { + hour_index++; + } + + if (hour_index == ce->hour_count && day_index < ce->day_count) { + day_index++; + } + + if (day_index == ce->day_count && month_index < ce->month_count) { + month_index++; + } + + out->minute = ce->minutes[minute_index % ce->minute_count]; + out->hour = ce->hours[hour_index % ce->hour_count]; + out->day = ce->days[day_index % ce->day_count]; + + // Sometimes, we end up with a day that does not exist within the selected + // month, e.g. day 30 in February. When this occurs, we reset day back to + // the smallest value & loop over to the next month that does have this + // day. + if (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { + out->day = ce->days[0]; + month_index++; + + while (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { + month_index++; + + if (month_index == 2 * ce->month_count) { + return 1; + } + } + } + + out->month = ce->months[month_index * ce->month_count]; + + if (month_index >= ce->month_count) { + out->year = ref->year + 1; + } else { + out->year = ref->year; + } + + return 0; +} diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h index 799a9a3..7abb189 100644 --- a/src/cron/expression/c/expression.h +++ b/src/cron/expression/c/expression.h @@ -21,9 +21,17 @@ typedef struct cron_expression { uint8_t month_count; } CronExpression; +typedef struct simple_time { + int year; + int month; + int day; + int hour; + int minute; +} SimpleTime; + /** * Given a */ -int ce_next(struct tm *out, struct tm *ref); +int ce_next(SimpleTime *out, CronExpression *ce, SimpleTime *ref); ParseError ce_parse_expression(CronExpression *out, char *s); From d8e3dcb34f3ac8d6c0708505e820d280f0120776 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Fri, 13 Jan 2023 19:03:58 +0100 Subject: [PATCH 06/30] chore: remove old cron daemon code --- src/cron/cli.v | 32 ----- src/cron/cron.v | 33 ----- src/cron/daemon/build.v | 115 ---------------- src/cron/daemon/daemon.v | 274 --------------------------------------- src/cron/daemon/log.v | 35 ----- src/main.v | 2 - 6 files changed, 491 deletions(-) delete mode 100644 src/cron/cli.v delete mode 100644 src/cron/cron.v delete mode 100644 src/cron/daemon/build.v delete mode 100644 src/cron/daemon/daemon.v delete mode 100644 src/cron/daemon/log.v diff --git a/src/cron/cli.v b/src/cron/cli.v deleted file mode 100644 index 16a3537..0000000 --- a/src/cron/cli.v +++ /dev/null @@ -1,32 +0,0 @@ -module cron - -import cli -import conf as vconf - -struct Config { -pub: - log_level string = 'WARN' - api_key string - address string - data_dir string - base_image string = 'archlinux:base-devel' - max_concurrent_builds int = 1 - api_update_frequency int = 15 - image_rebuild_frequency int = 1440 - // Replicates the behavior of the original cron system - global_schedule string = '0 3' -} - -// cmd returns the cli module that handles the cron daemon. -pub fn cmd() cli.Command { - return cli.Command{ - name: 'cron' - description: 'Start the cron service that periodically runs builds.' - execute: fn (cmd cli.Command) ! { - config_file := cmd.flags.get_string('config-file')! - conf := vconf.load(prefix: 'VIETER_', default_path: config_file)! - - cron(conf)! - } - } -} diff --git a/src/cron/cron.v b/src/cron/cron.v deleted file mode 100644 index f1d6b7b..0000000 --- a/src/cron/cron.v +++ /dev/null @@ -1,33 +0,0 @@ -module cron - -import log -import cron.daemon -import cron.expression -import os - -const log_file_name = 'vieter.cron.log' - -// cron starts a cron daemon & starts periodically scheduling builds. -pub fn cron(conf Config) ! { - // Configure logger - log_level := log.level_from_tag(conf.log_level) or { - return error('Invalid log level. The allowed values are FATAL, ERROR, WARN, INFO & DEBUG.') - } - - mut logger := log.Log{ - level: log_level - } - - log_file := os.join_path_single(conf.data_dir, cron.log_file_name) - logger.set_full_logpath(log_file) - logger.log_to_console_too() - - ce := expression.parse_expression(conf.global_schedule) or { - return error('Error while parsing global cron expression: $err.msg()') - } - - mut d := daemon.init_daemon(logger, conf.address, conf.api_key, conf.base_image, ce, - conf.max_concurrent_builds, conf.api_update_frequency, conf.image_rebuild_frequency)! - - d.run() -} diff --git a/src/cron/daemon/build.v b/src/cron/daemon/build.v deleted file mode 100644 index 42edc92..0000000 --- a/src/cron/daemon/build.v +++ /dev/null @@ -1,115 +0,0 @@ -module daemon - -import time -import sync.stdatomic -import build -import os - -const ( - build_empty = 0 - build_running = 1 - build_done = 2 -) - -// clean_finished_builds removes finished builds from the build slots & returns -// them. -fn (mut d Daemon) clean_finished_builds() []ScheduledBuild { - mut out := []ScheduledBuild{} - - for i in 0 .. d.atomics.len { - if stdatomic.load_u64(&d.atomics[i]) == daemon.build_done { - stdatomic.store_u64(&d.atomics[i], daemon.build_empty) - out << d.builds[i] - } - } - - return out -} - -// update_builds starts as many builds as possible. -fn (mut d Daemon) start_new_builds() { - now := time.now() - - for d.queue.len() > 0 { - elem := d.queue.peek() or { - d.lerror("queue.peek() unexpectedly returned an error. This shouldn't happen.") - - break - } - - if elem.timestamp < now { - sb := d.queue.pop() or { - d.lerror("queue.pop() unexpectedly returned an error. This shouldn't happen.") - - break - } - - // If this build couldn't be scheduled, no more will be possible. - if !d.start_build(sb) { - d.queue.insert(sb) - break - } - } else { - break - } - } -} - -// start_build starts a build for the given ScheduledBuild object. -fn (mut d Daemon) start_build(sb ScheduledBuild) bool { - for i in 0 .. d.atomics.len { - if stdatomic.load_u64(&d.atomics[i]) == daemon.build_empty { - stdatomic.store_u64(&d.atomics[i], daemon.build_running) - d.builds[i] = sb - - go d.run_build(i, sb) - - return true - } - } - - return false -} - -// run_build actually starts the build process for a given target. -fn (mut d Daemon) run_build(build_index int, sb ScheduledBuild) { - d.linfo('started build: $sb.target.url -> $sb.target.repo') - - // 0 means success, 1 means failure - mut status := 0 - - res := build.build_target(d.client.address, d.client.api_key, d.builder_images.last(), - &sb.target, false) or { - d.ldebug('build_target error: $err.msg()') - status = 1 - - build.BuildResult{} - } - - if status == 0 { - d.linfo('finished build: $sb.target.url -> $sb.target.repo; uploading logs...') - - build_arch := os.uname().machine - d.client.add_build_log(sb.target.id, res.start_time, res.end_time, build_arch, - res.exit_code, res.logs) or { - d.lerror('Failed to upload logs for build: $sb.target.url -> $sb.target.repo') - } - } else { - d.linfo('an error occured during build: $sb.target.url -> $sb.target.repo') - } - - stdatomic.store_u64(&d.atomics[build_index], daemon.build_done) -} - -// current_build_count returns how many builds are currently running. -fn (mut d Daemon) current_build_count() int { - mut res := 0 - - for i in 0 .. d.atomics.len { - if stdatomic.load_u64(&d.atomics[i]) == daemon.build_running { - res += 1 - } - } - - return res -} diff --git a/src/cron/daemon/daemon.v b/src/cron/daemon/daemon.v deleted file mode 100644 index b94dab8..0000000 --- a/src/cron/daemon/daemon.v +++ /dev/null @@ -1,274 +0,0 @@ -module daemon - -import time -import log -import datatypes { MinHeap } -import cron.expression { CronExpression, parse_expression } -import math -import build -import docker -import os -import client -import models { Target } - -const ( - // How many seconds to wait before retrying to update API if failed - api_update_retry_timeout = 5 - // How many seconds to wait before retrying to rebuild image if failed - rebuild_base_image_retry_timout = 30 -) - -struct ScheduledBuild { -pub: - target Target - timestamp time.Time -} - -// Overloaded operator for comparing ScheduledBuild objects -fn (r1 ScheduledBuild) < (r2 ScheduledBuild) bool { - return r1.timestamp < r2.timestamp -} - -pub struct Daemon { -mut: - client client.Client - base_image string - builder_images []string - global_schedule CronExpression - api_update_frequency int - image_rebuild_frequency int - // Targets currently loaded from API. - targets []Target - // At what point to update the list of targets. - api_update_timestamp time.Time - image_build_timestamp time.Time - queue MinHeap - // Which builds are currently running - builds []ScheduledBuild - // Atomic variables used to detect when a build has finished; length is the - // same as builds - atomics []u64 - logger shared log.Log -} - -// init_daemon initializes a new Daemon object. It renews the targets & -// populates the build queue for the first time. -pub fn init_daemon(logger log.Log, address string, api_key string, base_image string, global_schedule CronExpression, max_concurrent_builds int, api_update_frequency int, image_rebuild_frequency int) !Daemon { - mut d := Daemon{ - client: client.new(address, api_key) - base_image: base_image - global_schedule: global_schedule - api_update_frequency: api_update_frequency - image_rebuild_frequency: image_rebuild_frequency - atomics: []u64{len: max_concurrent_builds} - builds: []ScheduledBuild{len: max_concurrent_builds} - logger: logger - } - - // Initialize the targets & queue - d.renew_targets() - d.renew_queue() - if !d.rebuild_base_image() { - return error('The base image failed to build. The Vieter cron daemon cannot run without an initial builder image.') - } - - return d -} - -// run starts the actual daemon process. It runs builds when possible & -// periodically refreshes the list of targets to ensure we stay in sync. -pub fn (mut d Daemon) run() { - for { - finished_builds := d.clean_finished_builds() - - // Update the API's contents if needed & renew the queue - if time.now() >= d.api_update_timestamp { - d.renew_targets() - d.renew_queue() - } - // The finished builds should only be rescheduled if the API contents - // haven't been renewed. - else { - for sb in finished_builds { - d.schedule_build(sb.target) - } - } - - // TODO remove old builder images. - // This issue is less trivial than it sounds, because a build could - // still be running when the image has to be rebuilt. That would - // prevent the image from being removed. Therefore, we will need to - // keep track of a list or something & remove an image once we have - // made sure it isn't being used anymore. - if time.now() >= d.image_build_timestamp { - d.rebuild_base_image() - // In theory, executing this function here allows an old builder - // image to exist for at most image_rebuild_frequency minutes. - d.clean_old_base_images() - } - - // Schedules new builds when possible - d.start_new_builds() - - // If there are builds currently running, the daemon should refresh - // every second to clean up any finished builds & start new ones. - mut delay := time.Duration(1 * time.second) - - // Sleep either until we have to refresh the targets or when the next - // build has to start, with a minimum of 1 second. - if d.current_build_count() == 0 { - now := time.now() - delay = d.api_update_timestamp - now - - if d.queue.len() > 0 { - elem := d.queue.peek() or { - d.lerror("queue.peek() unexpectedly returned an error. This shouldn't happen.") - - // This is just a fallback option. In theory, queue.peek() - // should *never* return an error or none, because we check - // its len beforehand. - time.sleep(1) - continue - } - - time_until_next_job := elem.timestamp - now - - delay = math.min(delay, time_until_next_job) - } - } - - // We sleep for at least one second. This is to prevent the program - // from looping agressively when a cronjob can be scheduled, but - // there's no spots free for it to be started. - delay = math.max(delay, 1 * time.second) - - d.ldebug('Sleeping for ${delay}...') - - time.sleep(delay) - } -} - -// schedule_build adds the next occurence of the given targets build to the -// queue. -fn (mut d Daemon) schedule_build(target Target) { - ce := if target.schedule != '' { - parse_expression(target.schedule) or { - // TODO This shouldn't return an error if the expression is empty. - d.lerror("Error while parsing cron expression '$target.schedule' (id $target.id): $err.msg()") - - d.global_schedule - } - } else { - d.global_schedule - } - - // A target that can't be scheduled will just be skipped for now - timestamp := ce.next_from_now() or { - d.lerror("Couldn't calculate next timestamp from '$target.schedule'; skipping") - return - } - - d.queue.insert(ScheduledBuild{ - target: target - timestamp: timestamp - }) -} - -// renew_targets requests the newest list of targets from the server & replaces -// the old one. -fn (mut d Daemon) renew_targets() { - d.linfo('Renewing targets...') - - mut new_targets := d.client.get_all_targets() or { - d.lerror('Failed to renew targets. Retrying in ${daemon.api_update_retry_timeout}s...') - d.api_update_timestamp = time.now().add_seconds(daemon.api_update_retry_timeout) - - return - } - - // Filter out any targets that shouldn't run on this architecture - cur_arch := os.uname().machine - new_targets = new_targets.filter(it.arch.any(it.value == cur_arch)) - - d.targets = new_targets - - d.api_update_timestamp = time.now().add_seconds(60 * d.api_update_frequency) -} - -// renew_queue replaces the old queue with a new one that reflects the newest -// values in targets. -fn (mut d Daemon) renew_queue() { - d.linfo('Renewing queue...') - mut new_queue := MinHeap{} - - // Move any jobs that should have already started from the old queue onto - // the new one - now := time.now() - - // For some reason, using - // ```v - // for d.queue.len() > 0 && d.queue.peek() !.timestamp < now { - //``` - // here causes the function to prematurely just exit, without any errors or anything, very weird - // https://github.com/vlang/v/issues/14042 - for d.queue.len() > 0 { - elem := d.queue.pop() or { - d.lerror("queue.pop() returned an error. This shouldn't happen.") - continue - } - - if elem.timestamp < now { - new_queue.insert(elem) - } else { - break - } - } - - d.queue = new_queue - - // For each target in targets, parse their cron expression (or use the - // default one if not present) & add them to the queue - for target in d.targets { - d.schedule_build(target) - } -} - -// rebuild_base_image recreates the builder image. -fn (mut d Daemon) rebuild_base_image() bool { - d.linfo('Rebuilding builder image....') - - d.builder_images << build.create_build_image(d.base_image) or { - d.lerror('Failed to rebuild base image. Retrying in ${daemon.rebuild_base_image_retry_timout}s...') - d.image_build_timestamp = time.now().add_seconds(daemon.rebuild_base_image_retry_timout) - - return false - } - - d.image_build_timestamp = time.now().add_seconds(60 * d.image_rebuild_frequency) - - return true -} - -// clean_old_base_images tries to remove any old but still present builder -// images. -fn (mut d Daemon) clean_old_base_images() { - mut i := 0 - - mut dd := docker.new_conn() or { - d.lerror('Failed to connect to Docker socket.') - return - } - - defer { - dd.close() or {} - } - - for i < d.builder_images.len - 1 { - // For each builder image, we try to remove it by calling the Docker - // API. If the function returns an error or false, that means the image - // wasn't deleted. Therefore, we move the index over. If the function - // returns true, the array's length has decreased by one so we don't - // move the index. - dd.image_remove(d.builder_images[i]) or { i += 1 } - } -} diff --git a/src/cron/daemon/log.v b/src/cron/daemon/log.v deleted file mode 100644 index 95a50e7..0000000 --- a/src/cron/daemon/log.v +++ /dev/null @@ -1,35 +0,0 @@ -module daemon - -import log - -// log reate a log message with the given level -pub fn (mut d Daemon) log(msg string, level log.Level) { - lock d.logger { - d.logger.send_output(msg, level) - } -} - -// lfatal create a log message with the fatal level -pub fn (mut d Daemon) lfatal(msg string) { - d.log(msg, log.Level.fatal) -} - -// lerror create a log message with the error level -pub fn (mut d Daemon) lerror(msg string) { - d.log(msg, log.Level.error) -} - -// lwarn create a log message with the warn level -pub fn (mut d Daemon) lwarn(msg string) { - d.log(msg, log.Level.warn) -} - -// linfo create a log message with the info level -pub fn (mut d Daemon) linfo(msg string) { - d.log(msg, log.Level.info) -} - -// ldebug create a log message with the debug level -pub fn (mut d Daemon) ldebug(msg string) { - d.log(msg, log.Level.debug) -} diff --git a/src/main.v b/src/main.v index 1c8b816..ce9ec81 100644 --- a/src/main.v +++ b/src/main.v @@ -9,7 +9,6 @@ import console.schedule import console.man import console.aur import console.repos -import cron import agent fn main() { @@ -43,7 +42,6 @@ fn main() { commands: [ server.cmd(), targets.cmd(), - cron.cmd(), logs.cmd(), schedule.cmd(), man.cmd(), From 191ea1f2fe9932e2b2257839dcf1b44f6217b09e Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Thu, 12 Jan 2023 21:52:51 +0100 Subject: [PATCH 07/30] feat(cron): first step of replacing cron with C implementation --- Makefile | 2 +- src/build/queue.v | 8 +- src/cron/expression/c/expression.c | 36 +++++- src/cron/expression/c/expression.h | 30 +++-- src/cron/expression/c/parse.c | 39 ++++--- src/cron/expression/expression.c.v | 37 +++++-- src/cron/expression/expression.v | 146 +++++++------------------ src/cron/expression/expression_parse.v | 146 ------------------------- src/cron/expression/expression_test.v | 18 +-- 9 files changed, 157 insertions(+), 305 deletions(-) delete mode 100644 src/cron/expression/expression_parse.v diff --git a/Makefile b/Makefile index 4bd1edc..c71ff1f 100644 --- a/Makefile +++ b/Makefile @@ -81,7 +81,7 @@ fmt: .PHONY: test test: - $(V) test $(SRC_DIR) + $(V) -g test $(SRC_DIR) .PHONY: clean clean: diff --git a/src/build/queue.v b/src/build/queue.v index e74529c..e87024b 100644 --- a/src/build/queue.v +++ b/src/build/queue.v @@ -13,7 +13,7 @@ pub mut: // Next timestamp from which point this job is allowed to be executed timestamp time.Time // Required for calculating next timestamp after having pop'ed a job - ce CronExpression + ce &CronExpression = unsafe { nil } // Actual build config sent to the agent config BuildConfig // Whether this is a one-time job @@ -30,7 +30,7 @@ fn (r1 BuildJob) < (r2 BuildJob) bool { // for each architecture. Agents receive jobs from this queue. pub struct BuildJobQueue { // Schedule to use for targets without explicitely defined cron expression - default_schedule CronExpression + default_schedule &CronExpression // Base image to use for targets without defined base image default_base_image string mut: @@ -44,9 +44,9 @@ mut: } // new_job_queue initializes a new job queue -pub fn new_job_queue(default_schedule CronExpression, default_base_image string) BuildJobQueue { +pub fn new_job_queue(default_schedule &CronExpression, default_base_image string) BuildJobQueue { return BuildJobQueue{ - default_schedule: default_schedule + default_schedule: unsafe { default_schedule } default_base_image: default_base_image invalidated: map[int]time.Time{} } diff --git a/src/cron/expression/c/expression.c b/src/cron/expression/c/expression.c index 3f65b6a..c990b4f 100644 --- a/src/cron/expression/c/expression.c +++ b/src/cron/expression/c/expression.c @@ -1,8 +1,21 @@ #include "expression.h" +#include const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}; -int ce_next(SimpleTime *out, CronExpression *ce, SimpleTime *ref) { +struct cron_expression *ce_init() { + return malloc(sizeof(struct cron_expression)); +} + +void ce_free(struct cron_expression *ce) { + free(ce->months); + free(ce->days); + free(ce->hours); + free(ce->minutes); + free(ce); +} + +int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cron_simple_time *ref) { // For all of these values, the rule is the following: if their value is // the length of their respective array in the CronExpression object, that // means we've looped back around. This means that the "bigger" value has @@ -26,12 +39,12 @@ int ce_next(SimpleTime *out, CronExpression *ce, SimpleTime *ref) { day_index++; } - if (day_index < ce->days_count && ref->day == ce->days[day_index]) { + if (day_index < ce->day_count && ref->day == ce->days[day_index]) { while (hour_index < ce->hour_count && ref->hour > ce->hours[hour_index]) { hour_index++; } - if (hour_index < ce->hours_count && ref->hour == ce->hours[hour_index]) { + if (hour_index < ce->hour_count && ref->hour == ce->hours[hour_index]) { // Minute is the only value where we explicitely make sure we // can't match sref's value exactly. This is to ensure we only // return values in the future. @@ -88,3 +101,20 @@ int ce_next(SimpleTime *out, CronExpression *ce, SimpleTime *ref) { return 0; } + +int ce_next_from_now(struct cron_simple_time *out, struct cron_expression *ce) { + time_t t = time(NULL); + struct tm gm; + gmtime_r(&t, &gm); + + struct cron_simple_time ref = { + .year = gm.tm_year, + // tm_mon goes from 0 to 11 + .month = gm.tm_mon + 1, + .day = gm.tm_mday, + .hour = gm.tm_hour, + .minute = gm.tm_min + }; + + return ce_next(out, ce, &ref); +} diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h index 7abb189..91db5d0 100644 --- a/src/cron/expression/c/expression.h +++ b/src/cron/expression/c/expression.h @@ -3,14 +3,14 @@ #include #include -typedef enum parse_error { - ParseOk = 0, - ParseInvalidExpression = 1, - ParseInvalidNumber = 2, - ParseOutOfRange = 3 -} ParseError; +enum cron_parse_error { + CPEParseOk = 0, + CPEParseInvalidExpression = 1, + CPEParseInvalidNumber = 2, + CPEParseOutOfRange = 3 +}; -typedef struct cron_expression { +struct cron_expression { uint8_t *minutes; uint8_t *hours; uint8_t *days; @@ -19,19 +19,25 @@ typedef struct cron_expression { uint8_t hour_count; uint8_t day_count; uint8_t month_count; -} CronExpression; +}; -typedef struct simple_time { +struct cron_simple_time { int year; int month; int day; int hour; int minute; -} SimpleTime; +}; + +struct cron_expression *ce_init(); + +void cron_ce_free(struct cron_expression *ce); /** * Given a */ -int ce_next(SimpleTime *out, CronExpression *ce, SimpleTime *ref); +int cron_ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct ce_simple_time *ref); -ParseError ce_parse_expression(CronExpression *out, char *s); +int cron_ce_next_from_now(struct simple_time *out, struct cron_expression *ce); + +enum cron_parse_error cron_ce_parse_expression(struct cron_expression *out, char *s); diff --git a/src/cron/expression/c/parse.c b/src/cron/expression/c/parse.c index cb97373..b49b5dd 100644 --- a/src/cron/expression/c/parse.c +++ b/src/cron/expression/c/parse.c @@ -6,10 +6,10 @@ const uint8_t max[4] = {59, 23, 31, 12}; #define SAFE_ATOI(v,s,min,max) \ int _##v = atoi(s); \ if ((_##v) == 0 && strcmp((s), "0") != 0) { \ - return ParseInvalidNumber; \ + return CPEParseInvalidNumber; \ } \ if (v < (min) || v > (max)) { \ - return ParseOutOfRange; \ + return CPEParseOutOfRange; \ } \ v = (uint8_t) (_##v); @@ -29,17 +29,17 @@ const uint8_t max[4] = {59, 23, 31, 12}; * - a/c * - a-b/c */ -ParseError ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { +enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { // The * expression means "every possible value" if (s[0] == '*') { // A '*' is only valid on its own if (s[1] != '\0') { - return ParseInvalidExpression; + return CPEParseInvalidExpression; } *out = ~0; - return ParseOk; + return CPEParseOk; } size_t slash_index = 0; @@ -88,20 +88,20 @@ ParseError ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { } } - return ParseOk; + return CPEParseOk; } -ParseError ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { +enum cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { *out = 0; char *next; - ParseError res; + enum cron_parse_error res; while ((next = strchr(s, ',')) != NULL) { next[0] = '\0'; res = ce_parse_range(out, s, min, max); - if (res != ParseOk) { + if (res != CPEParseOk) { return res; } @@ -111,11 +111,11 @@ ParseError ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { // Make sure to parse the final range as well res = ce_parse_range(out, s, min, max); - if (res != ParseOk) { + if (res != CPEParseOk) { return res; } - return ParseOk; + return CPEParseOk; } uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { @@ -147,11 +147,14 @@ uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { return size; } -ParseError ce_parse_expression(CronExpression *out, char *s) { +enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) { + // The parsing functions modify the input string in-place + s = strdup(s); + uint8_t part_count = 0; char *next; - ParseError res; + enum cron_parse_error res; uint64_t bfs[4]; // Skip leading spaces @@ -159,11 +162,11 @@ ParseError ce_parse_expression(CronExpression *out, char *s) { s++; } - while (part_count < 4 && (next = strchr(s, ' ')) != NULL) { + while (part_count < 4 && ((next = strchr(s, ' ')) != NULL)) { next[0] = '\0'; res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - if (res != ParseOk) { + if (res != CPEParseOk) { return res; } @@ -184,7 +187,7 @@ ParseError ce_parse_expression(CronExpression *out, char *s) { // Make sure to parse the final range as well res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - if (res != ParseOk) { + if (res != CPEParseOk) { return res; } @@ -193,7 +196,7 @@ ParseError ce_parse_expression(CronExpression *out, char *s) { // At least two parts need to be provided if (part_count < 2) { - return ParseInvalidExpression; + return CPEParseInvalidExpression; } // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts @@ -208,5 +211,5 @@ ParseError ce_parse_expression(CronExpression *out, char *s) { out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); - return ParseOk; + return CPEParseOk; } diff --git a/src/cron/expression/expression.c.v b/src/cron/expression/expression.c.v index ec39fd6..fc97176 100644 --- a/src/cron/expression/expression.c.v +++ b/src/cron/expression/expression.c.v @@ -2,17 +2,36 @@ module expression #flag -I @VMODROOT/c #flag @VMODROOT/c/parse.o +#flag @VMODROOT/c/expression.o #include "expression.h" -pub struct C.CronExpression { - minutes &u8 - hours &u8 - days &u8 - months &u8 +pub struct C.cron_expression { + minutes &u8 + hours &u8 + days &u8 + months &u8 minute_count u8 - hour_count u8 - day_count u8 - month_count u8 + hour_count u8 + day_count u8 + month_count u8 } -/* pub type CronExpression = C.CronExpression */ +pub type CronExpression = C.cron_expression + +struct C.cron_simple_time { + year int + month int + day int + hour int + minute int +} + +fn C.ce_init() &C.cron_expression + +fn C.ce_free(ce &C.cron_expression) + +fn C.ce_next(out &C.cron_simple_time, ce &C.cron_expression, ref &C.cron_simple_time) int + +fn C.ce_next_from_now(out &C.cron_simple_time, ce &C.cron_expression) int + +fn C.ce_parse_expression(out &C.cron_expression, s &char) int diff --git a/src/cron/expression/expression.v b/src/cron/expression/expression.v index c3ff8c5..a51f562 100644 --- a/src/cron/expression/expression.v +++ b/src/cron/expression/expression.v @@ -2,123 +2,61 @@ module expression import time -pub struct CronExpression { - minutes []int - hours []int - days []int - months []int +pub fn parse_expression(exp string) !&CronExpression { + out := C.ce_init() + res := C.ce_parse_expression(out, exp.str) + + if res != 0 { + return error('yuhh') + } + + return out +} + +pub fn (ce &CronExpression) free() { + C.ce_free(ce) } -// next calculates the earliest time this cron expression is valid. It will -// always pick a moment in the future, even if ref matches completely up to the -// minute. This function conciously does not take gap years into account. pub fn (ce &CronExpression) next(ref time.Time) !time.Time { - // If the given ref matches the next cron occurence up to the minute, it - // will return that value. Because we always want to return a value in the - // future, we artifically shift the ref 60 seconds to make sure we always - // match in the future. A shift of 60 seconds is enough because the cron - // expression does not allow for accuracy smaller than one minute. - sref := ref - - // For all of these values, the rule is the following: if their value is - // the length of their respective array in the CronExpression object, that - // means we've looped back around. This means that the "bigger" value has - // to be incremented by one. For example, if the minutes have looped - // around, that means that the hour has to be incremented as well. - mut minute_index := 0 - mut hour_index := 0 - mut day_index := 0 - mut month_index := 0 - - // This chain is the same logic multiple times, namely that if a "bigger" - // value loops around, then the smaller value will always reset as well. - // For example, if we're going to a new day, the hour & minute will always - // be their smallest value again. - for month_index < ce.months.len && sref.month > ce.months[month_index] { - month_index++ + st := C.cron_simple_time{ + year: ref.year + month: ref.month + day: ref.day + hour: ref.hour + minute: ref.minute } - if month_index < ce.months.len && sref.month == ce.months[month_index] { - for day_index < ce.days.len && sref.day > ce.days[day_index] { - day_index++ - } + out := C.cron_simple_time{} + res := C.ce_next(&out, ce, &st) - if day_index < ce.days.len && ce.days[day_index] == sref.day { - for hour_index < ce.hours.len && sref.hour > ce.hours[hour_index] { - hour_index++ - } - - if hour_index < ce.hours.len && ce.hours[hour_index] == sref.hour { - // Minute is the only value where we explicitely make sure we - // can't match sref's value exactly. This is to ensure we only - // return values in the future. - for minute_index < ce.minutes.len && sref.minute >= ce.minutes[minute_index] { - minute_index++ - } - } - } - } - - // Here, we increment the "bigger" values by one if the smaller ones loop - // around. The order is important, as it allows a sort-of waterfall effect - // to occur which updates all values if required. - if minute_index == ce.minutes.len && hour_index < ce.hours.len { - hour_index += 1 - } - - if hour_index == ce.hours.len && day_index < ce.days.len { - day_index += 1 - } - - if day_index == ce.days.len && month_index < ce.months.len { - month_index += 1 - } - - mut minute := ce.minutes[minute_index % ce.minutes.len] - mut hour := ce.hours[hour_index % ce.hours.len] - mut day := ce.days[day_index % ce.days.len] - - // Sometimes, we end up with a day that does not exist within the selected - // month, e.g. day 30 in February. When this occurs, we reset day back to - // the smallest value & loop over to the next month that does have this - // day. - if day > time.month_days[ce.months[month_index % ce.months.len] - 1] { - day = ce.days[0] - month_index += 1 - - for day > time.month_days[ce.months[month_index & ce.months.len] - 1] { - month_index += 1 - - // If for whatever reason the day value ends up being something - // that can't be scheduled in any month, we have to make sure we - // don't create an infinite loop. - if month_index == 2 * ce.months.len { - return error('No schedulable moment.') - } - } - } - - month := ce.months[month_index % ce.months.len] - mut year := sref.year - - // If the month loops over, we need to increment the year. - if month_index >= ce.months.len { - year++ + if res != 0 { + return error('yuhh') } return time.new_time(time.Time{ - year: year - month: month - day: day - minute: minute - hour: hour + year: out.year + month: out.month + day: out.day + hour: out.hour + minute: out.minute }) } -// next_from_now returns the result of ce.next(ref) where ref is the result of -// time.now(). pub fn (ce &CronExpression) next_from_now() !time.Time { - return ce.next(time.now()) + out := C.cron_simple_time{} + res := C.ce_next_from_now(&out, ce) + + if res != 0 { + return error('yuhh') + } + + return time.new_time(time.Time{ + year: out.year + month: out.month + day: out.day + hour: out.hour + minute: out.minute + }) } // next_n returns the n next occurences of the expression, given a starting diff --git a/src/cron/expression/expression_parse.v b/src/cron/expression/expression_parse.v deleted file mode 100644 index 4aaec5b..0000000 --- a/src/cron/expression/expression_parse.v +++ /dev/null @@ -1,146 +0,0 @@ -module expression - -import bitfield - -// parse_range parses a given string into a range of sorted integers. Its -// result is a BitField with set bits for all numbers in the result. -fn parse_range(s string, min int, max int) !bitfield.BitField { - mut start := min - mut end := max - mut interval := 1 - mut bf := bitfield.new(max - min + 1) - - exps := s.split('/') - - if exps.len > 2 { - return error('Invalid expression.') - } - - if exps[0] != '*' { - dash_parts := exps[0].split('-') - - if dash_parts.len > 2 { - return error('Invalid expression.') - } - - start = dash_parts[0].int() - - // The builtin parsing functions return zero if the string can't be - // parsed into a number, so we have to explicitely check whether they - // actually entered zero or if it's an invalid number. - if start == 0 && dash_parts[0] != '0' { - return error('Invalid number.') - } - - // Check whether the start value is out of range - if start < min || start > max { - return error('Out of range.') - } - - if dash_parts.len == 2 { - end = dash_parts[1].int() - - if end == 0 && dash_parts[1] != '0' { - return error('Invalid number.') - } - - if end < start || end > max { - return error('Out of range.') - } - } - } - - if exps.len > 1 { - interval = exps[1].int() - - // interval being zero is always invalid, but we want to check why - // it's invalid for better error messages. - if interval == 0 { - if exps[1] != '0' { - return error('Invalid number.') - } else { - return error('Step size zero not allowed.') - } - } - - if interval > max - min { - return error('Step size too large.') - } - } - // Here, s solely consists of a number, so that's the only value we - // should return. - else if exps[0] != '*' && !exps[0].contains('-') { - bf.set_bit(start - min) - return bf - } - - for start <= end { - bf.set_bit(start - min) - start += interval - } - - return bf -} - -// bf_to_ints takes a BitField and converts it into the expected list of actual -// integers. -fn bf_to_ints(bf bitfield.BitField, min int) []int { - mut out := []int{} - - for i in 0 .. bf.get_size() { - if bf.get_bit(i) == 1 { - out << min + i - } - } - - return out -} - -// parse_part parses a given part of a cron expression & returns the -// corresponding array of ints. -fn parse_part(s string, min int, max int) ![]int { - mut bf := bitfield.new(max - min + 1) - - for range in s.split(',') { - bf2 := parse_range(range, min, max)! - bf = bitfield.bf_or(bf, bf2) - } - - return bf_to_ints(bf, min) -} - -// parse_expression parses an entire cron expression string into a -// CronExpression object, if possible. -pub fn parse_expression(exp string) !CronExpression { - // The filter allows for multiple spaces between parts - mut parts := exp.split(' ').filter(it != '') - - if parts.len < 2 || parts.len > 4 { - return error('Expression must contain between 2 and 4 space-separated parts.') - } - - // For ease of use, we allow the user to only specify as many parts as they - // need. - for parts.len < 4 { - parts << '*' - } - - mut part_results := [][]int{} - - mins := [0, 0, 1, 1] - maxs := [59, 23, 31, 12] - - // This for loop allows us to more clearly propagate the error to the user. - for i, min in mins { - part_results << parse_part(parts[i], min, maxs[i]) or { - return error('An error occurred with part $i: $err.msg()') - } - } - - return CronExpression{ - minutes: part_results[0] - hours: part_results[1] - days: part_results[2] - months: part_results[3] - } -} diff --git a/src/cron/expression/expression_test.v b/src/cron/expression/expression_test.v index 82bf959..2b21b4b 100644 --- a/src/cron/expression/expression_test.v +++ b/src/cron/expression/expression_test.v @@ -4,6 +4,7 @@ import time { parse } fn util_test_time(exp string, t1_str string, t2_str string) ! { ce := parse_expression(exp)! + dump(ce) t1 := parse(t1_str)! t2 := parse(t2_str)! @@ -18,17 +19,18 @@ fn util_test_time(exp string, t1_str string, t2_str string) ! { fn test_next_simple() ! { // Very simple - util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! + /* util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! */ // Overlap to next day - util_test_time('0 3', '2002-01-01 03:00:00', '2002-01-02 03:00:00')! - util_test_time('0 3', '2002-01-01 04:00:00', '2002-01-02 03:00:00')! + mut exp := '0 3' + util_test_time(exp, '2002-01-01 03:00:00', '2002-01-02 03:00:00')! + util_test_time(exp, '2002-01-01 04:00:00', '2002-01-02 03:00:00')! - util_test_time('0 3/4', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! + /* util_test_time('0 3/4', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! */ - // Overlap to next month - util_test_time('0 3', '2002-11-31 04:00:00', '2002-12-01 03:00:00')! + /* // Overlap to next month */ + /* util_test_time('0 3', '2002-11-31 04:00:00', '2002-12-01 03:00:00')! */ - // Overlap to next year - util_test_time('0 3', '2002-12-31 04:00:00', '2003-01-01 03:00:00')! + /* // Overlap to next year */ + /* util_test_time('0 3', '2002-12-31 04:00:00', '2003-01-01 03:00:00')! */ } From 292e43944e90ebea156e3ab0f89e825f015b1e6b Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 14:18:41 +0100 Subject: [PATCH 08/30] feat(cron): pass original expression tests --- src/cron/expression/c/expression.c | 4 ++-- src/cron/expression/c/parse.c | 31 +++++++++++++++--------------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/cron/expression/c/expression.c b/src/cron/expression/c/expression.c index c990b4f..f9dc534 100644 --- a/src/cron/expression/c/expression.c +++ b/src/cron/expression/c/expression.c @@ -48,7 +48,7 @@ int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cro // Minute is the only value where we explicitely make sure we // can't match sref's value exactly. This is to ensure we only // return values in the future. - while (minute_index < ce->minute_count && ref->minute > ce->minutes[minute_index]) { + while (minute_index < ce->minute_count && ref->minute >= ce->minutes[minute_index]) { minute_index++; } } @@ -91,7 +91,7 @@ int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cro } } - out->month = ce->months[month_index * ce->month_count]; + out->month = ce->months[month_index % ce->month_count]; if (month_index >= ce->month_count) { out->year = ref->year + 1; diff --git a/src/cron/expression/c/parse.c b/src/cron/expression/c/parse.c index b49b5dd..e664dd8 100644 --- a/src/cron/expression/c/parse.c +++ b/src/cron/expression/c/parse.c @@ -80,10 +80,10 @@ enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_ // Single number doesn't need to loop if (end == 0 && slash_index == 0) { - *out |= 1 << (start - min); + *out |= ((uint64_t) 1) << (start - min); } else { for (;start <= end; start += interval) { - *out |= 1 << (start - min); + *out |= ((uint64_t) 1) << (start - min); start += interval; } } @@ -109,13 +109,7 @@ enum cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t } // Make sure to parse the final range as well - res = ce_parse_range(out, s, min, max); - - if (res != CPEParseOk) { - return res; - } - - return CPEParseOk; + return ce_parse_range(out, s, min, max); } uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { @@ -125,7 +119,7 @@ uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { uint8_t *buf = malloc(capacity * sizeof(uint8_t)); for (uint8_t i = 0; i <= max - min; i++) { - if ((1 << i) & bf) { + if (((uint64_t) 1 << i) & bf) { // Resize buffer if needed if (size == capacity) { capacity *= 2; @@ -150,11 +144,12 @@ uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) { // The parsing functions modify the input string in-place s = strdup(s); + char *orig_s = s; uint8_t part_count = 0; char *next; - enum cron_parse_error res; + enum cron_parse_error res = CPEParseOk; uint64_t bfs[4]; // Skip leading spaces @@ -167,10 +162,9 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); if (res != CPEParseOk) { - return res; + goto end; } - s = next + 1; size_t offset = 1; // Skip multiple spaces @@ -188,7 +182,7 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); if (res != CPEParseOk) { - return res; + goto end; } part_count++; @@ -196,7 +190,8 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) // At least two parts need to be provided if (part_count < 2) { - return CPEParseInvalidExpression; + res = CPEParseInvalidExpression; + goto end; } // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts @@ -211,5 +206,9 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); - return CPEParseOk; +end: + // s is cloned + free(orig_s); + + return res; } From fbc18386e262b8c35b6ad775dc3c52d1546c1f29 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 15:03:11 +0100 Subject: [PATCH 09/30] feat(cron): some bug fixes & formatting --- .editorconfig | 3 +- src/cron/expression/c/expression.c | 141 ++++++------ src/cron/expression/c/expression.h | 34 +-- src/cron/expression/c/parse.c | 318 ++++++++++++++------------ src/cron/expression/expression_test.v | 8 +- 5 files changed, 266 insertions(+), 238 deletions(-) diff --git a/.editorconfig b/.editorconfig index e23a3c7..e9c1e63 100644 --- a/.editorconfig +++ b/.editorconfig @@ -5,6 +5,5 @@ root = true end_of_line = lf insert_final_newline = true -[*.v] -# vfmt wants it :( +[*.{v,c,h}] indent_style = tab diff --git a/src/cron/expression/c/expression.c b/src/cron/expression/c/expression.c index f9dc534..f15e359 100644 --- a/src/cron/expression/c/expression.c +++ b/src/cron/expression/c/expression.c @@ -4,15 +4,15 @@ const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}; struct cron_expression *ce_init() { - return malloc(sizeof(struct cron_expression)); + return malloc(sizeof(struct cron_expression)); } void ce_free(struct cron_expression *ce) { - free(ce->months); - free(ce->days); - free(ce->hours); - free(ce->minutes); - free(ce); + free(ce->months); + free(ce->days); + free(ce->hours); + free(ce->minutes); + free(ce); } int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cron_simple_time *ref) { @@ -21,100 +21,101 @@ int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cro // means we've looped back around. This means that the "bigger" value has // to be incremented by one. For example, if the minutes have looped // around, that means that the hour has to be incremented as well. - uint8_t month_index = 0; - uint8_t day_index = 0; - uint8_t hour_index = 0; - uint8_t minute_index = 0; + uint8_t month_index = 0; + uint8_t day_index = 0; + uint8_t hour_index = 0; + uint8_t minute_index = 0; // This chain is the same logic multiple times, namely that if a "bigger" // value loops around, then the smaller value will always reset as well. // For example, if we're going to a new day, the hour & minute will always // be their smallest value again. - while (month_index < ce->month_count && ref->month > ce->months[month_index]) { - month_index++; - } + while (month_index < ce->month_count && ref->month > ce->months[month_index]) { + month_index++; + } - if (month_index < ce->month_count && ref->month == ce->months[month_index]) { - while (day_index < ce->day_count && ref->day > ce->days[day_index]) { - day_index++; - } + if (month_index < ce->month_count && ref->month == ce->months[month_index]) { + while (day_index < ce->day_count && ref->day > ce->days[day_index]) { + day_index++; + } - if (day_index < ce->day_count && ref->day == ce->days[day_index]) { - while (hour_index < ce->hour_count && ref->hour > ce->hours[hour_index]) { - hour_index++; - } + if (day_index < ce->day_count && ref->day == ce->days[day_index]) { + while (hour_index < ce->hour_count && ref->hour > ce->hours[hour_index]) { + hour_index++; + } - if (hour_index < ce->hour_count && ref->hour == ce->hours[hour_index]) { + if (hour_index < ce->hour_count && ref->hour == ce->hours[hour_index]) { // Minute is the only value where we explicitely make sure we // can't match sref's value exactly. This is to ensure we only // return values in the future. - while (minute_index < ce->minute_count && ref->minute >= ce->minutes[minute_index]) { - minute_index++; - } - } - } - } + while (minute_index < ce->minute_count && ref->minute >= ce->minutes[minute_index]) { + minute_index++; + } + } + } + } // Here, we increment the "bigger" values by one if the smaller ones loop // around. The order is important, as it allows a sort-of waterfall effect // to occur which updates all values if required. - if (minute_index == ce->minute_count && hour_index < ce->hour_count) { - hour_index++; - } + if (minute_index == ce->minute_count && hour_index < ce->hour_count) { + hour_index++; + } - if (hour_index == ce->hour_count && day_index < ce->day_count) { - day_index++; - } + if (hour_index == ce->hour_count && day_index < ce->day_count) { + day_index++; + } - if (day_index == ce->day_count && month_index < ce->month_count) { - month_index++; - } + if (day_index == ce->day_count && month_index < ce->month_count) { + month_index++; + } - out->minute = ce->minutes[minute_index % ce->minute_count]; - out->hour = ce->hours[hour_index % ce->hour_count]; - out->day = ce->days[day_index % ce->day_count]; + out->minute = ce->minutes[minute_index % ce->minute_count]; + out->hour = ce->hours[hour_index % ce->hour_count]; + out->day = ce->days[day_index % ce->day_count]; // Sometimes, we end up with a day that does not exist within the selected // month, e.g. day 30 in February. When this occurs, we reset day back to // the smallest value & loop over to the next month that does have this // day. - if (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { - out->day = ce->days[0]; - month_index++; + if (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { + out->day = ce->days[0]; + month_index++; - while (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { - month_index++; - - if (month_index == 2 * ce->month_count) { - return 1; - } - } - } + while (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { + month_index++; - out->month = ce->months[month_index % ce->month_count]; + // TODO find out if this can happen + if (month_index == 2 * ce->month_count) { + return 1; + } + } + } - if (month_index >= ce->month_count) { - out->year = ref->year + 1; - } else { - out->year = ref->year; - } + out->month = ce->months[month_index % ce->month_count]; - return 0; + if (month_index >= ce->month_count) { + out->year = ref->year + 1; + } else { + out->year = ref->year; + } + + return 0; } int ce_next_from_now(struct cron_simple_time *out, struct cron_expression *ce) { - time_t t = time(NULL); - struct tm gm; - gmtime_r(&t, &gm); + time_t t = time(NULL); + struct tm gm; + gmtime_r(&t, &gm); - struct cron_simple_time ref = { - .year = gm.tm_year, - // tm_mon goes from 0 to 11 - .month = gm.tm_mon + 1, - .day = gm.tm_mday, - .hour = gm.tm_hour, - .minute = gm.tm_min - }; + struct cron_simple_time ref = { + .year = gm.tm_year, + // tm_mon goes from 0 to 11 + .month = gm.tm_mon + 1, + .day = gm.tm_mday, + .hour = gm.tm_hour, + .minute = gm.tm_min + }; - return ce_next(out, ce, &ref); + return ce_next(out, ce, &ref); } diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h index 91db5d0..f6d8826 100644 --- a/src/cron/expression/c/expression.h +++ b/src/cron/expression/c/expression.h @@ -4,29 +4,29 @@ #include enum cron_parse_error { - CPEParseOk = 0, - CPEParseInvalidExpression = 1, - CPEParseInvalidNumber = 2, - CPEParseOutOfRange = 3 + CPEParseOk = 0, + CPEParseInvalidExpression = 1, + CPEParseInvalidNumber = 2, + CPEParseOutOfRange = 3 }; struct cron_expression { - uint8_t *minutes; - uint8_t *hours; - uint8_t *days; - uint8_t *months; - uint8_t minute_count; - uint8_t hour_count; - uint8_t day_count; - uint8_t month_count; + uint8_t *minutes; + uint8_t *hours; + uint8_t *days; + uint8_t *months; + uint8_t minute_count; + uint8_t hour_count; + uint8_t day_count; + uint8_t month_count; }; struct cron_simple_time { - int year; - int month; - int day; - int hour; - int minute; + int year; + int month; + int day; + int hour; + int minute; }; struct cron_expression *ce_init(); diff --git a/src/cron/expression/c/parse.c b/src/cron/expression/c/parse.c index e664dd8..cd23458 100644 --- a/src/cron/expression/c/parse.c +++ b/src/cron/expression/c/parse.c @@ -1,25 +1,28 @@ #include "expression.h" +// Allowed value ranges for the minute, hour, day and month field const uint8_t min[4] = {0, 0, 1, 1}; const uint8_t max[4] = {59, 23, 31, 12}; +// Convert a string a uint8_t value by parsing it using atoi and checking +// whether it's contained within the given range #define SAFE_ATOI(v,s,min,max) \ - int _##v = atoi(s); \ - if ((_##v) == 0 && strcmp((s), "0") != 0) { \ - return CPEParseInvalidNumber; \ - } \ - if (v < (min) || v > (max)) { \ - return CPEParseOutOfRange; \ - } \ - v = (uint8_t) (_##v); + int _##v = atoi(s); \ + if ((_##v) == 0 && strcmp((s), "0") != 0) { \ + return CPEParseInvalidNumber; \ + } \ + if (v < (min) || v > (max)) { \ + return CPEParseOutOfRange; \ + } \ + v = (uint8_t) (_##v); /** * Given a range expression, produce a bit field defining what numbers in the - * min-max range the expression represents. The first bit (starting from the - * right) corresponds to min, the max - min + 1'th bit to max. All trailing bits + * min-max range the expression represents. Bit 0 (starting from the + * right) corresponds to min, the bit max - min to max. All trailing bits * after this should be ignored. The given bitfield is modified in-place, so * multiple calls of this function can be performed on the same value to create - * the effect of ORing their values: + * the effect of ORing their values. * * A range expression has one of the following forms: * @@ -30,185 +33,210 @@ const uint8_t max[4] = {59, 23, 31, 12}; * - a-b/c */ enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { - // The * expression means "every possible value" - if (s[0] == '*') { - // A '*' is only valid on its own - if (s[1] != '\0') { - return CPEParseInvalidExpression; - } + // The * expression means "every possible value" + if (s[0] == '*') { + // A '*' is only valid on its own + if (s[1] != '\0') { + return CPEParseInvalidExpression; + } - *out = ~0; + *out = ~0; - return CPEParseOk; - } + return CPEParseOk; + } - size_t slash_index = 0; - size_t dash_index = 0; - size_t i = 0; + size_t slash_index = 0; + size_t dash_index = 0; + size_t i = 0; - // We first iterate over the string to determine whether it contains a slash - // and/or a dash. We know the dash can only be valid if it appears before - // the slash. - while (s[i] != '\0' && slash_index == 0) { - if (s[i] == '/') { - slash_index = i; + // We first iterate over the string to determine whether it contains a slash + // and/or a dash. We know the dash can only be valid if it appears before + // the slash. + while (s[i] != '\0' && slash_index == 0) { + if (s[i] == '/') { + slash_index = i; - s[i] = '\0'; - } else if (s[i] == '-') { - dash_index = i; + s[i] = '\0'; + } else if (s[i] == '-') { + dash_index = i; - s[i] = '\0'; - } + s[i] = '\0'; + } - i++; - } + i++; + } - // Parse the three possible numbers in the pattern - uint8_t start = 0; - uint8_t end = 0; - uint8_t interval = 1; + // Parse the three possible numbers in the pattern + uint8_t start = 0; + uint8_t end = max; + uint8_t interval = 1; - SAFE_ATOI(start, s, min, max); + SAFE_ATOI(start, s, min, max); - if (dash_index > 0) { - SAFE_ATOI(end, &s[dash_index + 1], min, max); - } + if (dash_index > 0) { + SAFE_ATOI(end, &s[dash_index + 1], min, max); + } - if (slash_index > 0) { - SAFE_ATOI(interval, &s[slash_index + 1], 1, max - min); - } + if (slash_index > 0) { + SAFE_ATOI(interval, &s[slash_index + 1], 1, max - min); + } - // Single number doesn't need to loop - if (end == 0 && slash_index == 0) { - *out |= ((uint64_t) 1) << (start - min); - } else { - for (;start <= end; start += interval) { - *out |= ((uint64_t) 1) << (start - min); - start += interval; - } - } + if (dash_index == 0 && slash_index == 0) { + *out |= ((uint64_t) 1) << (start - min); + } else { + while (start <= end) { + *out |= ((uint64_t) 1) << (start - min); + start += interval; + } + } - return CPEParseOk; + return CPEParseOk; } +/* + * Given an expression part, produce a bitfield defining what numbers in the + * min-max range the part represents. A part consists of one or more range + * expressions, separated by commas. + */ enum cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { - *out = 0; + *out = 0; - char *next; - enum cron_parse_error res; - - while ((next = strchr(s, ',')) != NULL) { - next[0] = '\0'; - res = ce_parse_range(out, s, min, max); + char *next; + enum cron_parse_error res; - if (res != CPEParseOk) { - return res; - } + while ((next = strchr(s, ',')) != NULL) { + next[0] = '\0'; + res = ce_parse_range(out, s, min, max); - s = next + 1; - } + if (res != CPEParseOk) { + return res; + } - // Make sure to parse the final range as well - return ce_parse_range(out, s, min, max); + s = next + 1; + } + + // Make sure to parse the final range as well + return ce_parse_range(out, s, min, max); } +/* + * Return how many bits are set in the bitfield, better known as popcount. I + * added my own implementation (taken from my algorithms course) as I don't want + * to be dependent on GCC-specific extensions. + */ +uint8_t uint64_t_popcount(uint64_t n) { + uint8_t c = 0; + + while (n != 0) { + // This sets the least significant bit to zero (very cool) + n &= n - 1; + c++; + } + + return c; +} + +/* + * Convert a bitfield into an array containing the numbers in the min-max range + * it represents. + */ uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { - uint8_t capacity = 8; - uint8_t size = 0; + // Each bit field only has `max - min + 1` meaningful bits. All other bits + // should be ignored, and can be any value. By shifting the bit field back and + // forth, we set these excessive bits to zero, ensuring popcount returns the + // correct value. + uint8_t excess_bits = 64 - (max - min + 1); + bf = (bf << excess_bits) >> excess_bits; + uint8_t size = uint64_t_popcount(bf); + uint8_t *buf = malloc(size * sizeof(uint8_t)); - uint8_t *buf = malloc(capacity * sizeof(uint8_t)); + uint8_t i = 0, j = 0; - for (uint8_t i = 0; i <= max - min; i++) { - if (((uint64_t) 1 << i) & bf) { - // Resize buffer if needed - if (size == capacity) { - capacity *= 2; - buf = realloc(buf, capacity * sizeof(uint8_t)); - } + while (j < size && i <= max - min) { + if (((uint64_t)1 << i) & bf) { + // Resize buffer if needed + buf[j] = min + i; + j++; + } - buf[size] = min + i; - size++; - } - } + i++; + } - // Resize buffer once more to remove any trailing unused bytes - if (size < capacity) { - buf = realloc(buf, size * sizeof(uint8_t)); - } + *out = buf; - *out = buf; - - return size; + return size; } +/* + * Parse a cron expression string into a cron_expression struct. + */ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) { - // The parsing functions modify the input string in-place - s = strdup(s); - char *orig_s = s; + // The parsing functions modify the input string in-place + s = strdup(s); + char *orig_s = s; - uint8_t part_count = 0; + uint8_t part_count = 0; - char *next; - enum cron_parse_error res = CPEParseOk; - uint64_t bfs[4]; + char *next; + enum cron_parse_error res = CPEParseOk; + uint64_t bfs[4]; - // Skip leading spaces - while (s[0] == ' ') { - s++; - } - - while (part_count < 4 && ((next = strchr(s, ' ')) != NULL)) { - next[0] = '\0'; - res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); + // Skip leading spaces + while (s[0] == ' ') { + s++; + } - if (res != CPEParseOk) { - goto end; - } + while (part_count < 4 && ((next = strchr(s, ' ')) != NULL)) { + next[0] = '\0'; + res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - size_t offset = 1; + if (res != CPEParseOk) { + goto end; + } - // Skip multiple spaces - while (next[offset] == ' ') { - offset++; - } - s = next + offset; + size_t offset = 1; - part_count++; - } + // Skip multiple spaces + while (next[offset] == ' ') { + offset++; + } + s = next + offset; - // Parse final trailing part - if (part_count < 4 && s[0] != '\0') { - // Make sure to parse the final range as well - res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); + part_count++; + } - if (res != CPEParseOk) { - goto end; - } + // Parse final trailing part + if (part_count < 4 && s[0] != '\0') { + res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - part_count++; - } + if (res != CPEParseOk) { + goto end; + } - // At least two parts need to be provided - if (part_count < 2) { - res = CPEParseInvalidExpression; - goto end; - } + part_count++; + } - // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts - while (part_count < 4) { - // Expression is augmented with '*' expressions - bfs[part_count] = ~0; - part_count++; - } + // At least two parts need to be provided + if (part_count < 2) { + res = CPEParseInvalidExpression; + goto end; + } - out->minute_count = bf_to_nums(&out->minutes, bfs[0], min[0], max[0]); - out->hour_count = bf_to_nums(&out->hours, bfs[1], min[1], max[1]); - out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); - out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); + // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts + while (part_count < 4) { + // Expression is augmented with '*' expressions + bfs[part_count] = ~0; + part_count++; + } + + out->minute_count = bf_to_nums(&out->minutes, bfs[0], min[0], max[0]); + out->hour_count = bf_to_nums(&out->hours, bfs[1], min[1], max[1]); + out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); + out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); end: - // s is cloned - free(orig_s); + // s is cloned + free(orig_s); - return res; + return res; } diff --git a/src/cron/expression/expression_test.v b/src/cron/expression/expression_test.v index 2b21b4b..448927a 100644 --- a/src/cron/expression/expression_test.v +++ b/src/cron/expression/expression_test.v @@ -22,15 +22,15 @@ fn test_next_simple() ! { /* util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! */ // Overlap to next day - mut exp := '0 3' + mut exp := '0 3 ' util_test_time(exp, '2002-01-01 03:00:00', '2002-01-02 03:00:00')! util_test_time(exp, '2002-01-01 04:00:00', '2002-01-02 03:00:00')! - /* util_test_time('0 3/4', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! */ + util_test_time('0 3/4', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! /* // Overlap to next month */ - /* util_test_time('0 3', '2002-11-31 04:00:00', '2002-12-01 03:00:00')! */ + util_test_time('0 3', '2002-11-31 04:00:00', '2002-12-01 03:00:00')! /* // Overlap to next year */ - /* util_test_time('0 3', '2002-12-31 04:00:00', '2003-01-01 03:00:00')! */ + util_test_time('0 3', '2002-12-31 04:00:00', '2003-01-01 03:00:00')! } From f63cbd77d3e943d7f463a54d362833910a2fcd61 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 16:52:30 +0100 Subject: [PATCH 10/30] refactor: make cron.expression into cron module --- CHANGELOG.md | 4 ++ src/build/queue.v | 10 ++--- src/console/schedule/schedule.v | 4 +- src/console/targets/targets.v | 4 +- src/cron/{expression => }/c/expression.c | 10 ++--- src/cron/c/expression.h | 40 +++++++++++++++++ src/cron/{expression => }/c/parse.c | 22 +++++----- src/cron/{expression => }/expression.c.v | 6 ++- src/cron/{expression => }/expression.v | 18 ++++---- src/cron/expression/c/expression.h | 43 ------------------- .../{expression => }/expression_parse_test.v | 2 +- src/cron/{expression => }/expression_test.v | 8 ++-- src/cron/{expression => }/v.mod | 0 src/server/log_removal.v | 4 +- src/server/server.v | 6 +-- 15 files changed, 92 insertions(+), 89 deletions(-) rename src/cron/{expression => }/c/expression.c (91%) create mode 100644 src/cron/c/expression.h rename src/cron/{expression => }/c/parse.c (92%) rename src/cron/{expression => }/expression.c.v (88%) rename src/cron/{expression => }/expression.v (69%) delete mode 100644 src/cron/expression/c/expression.h rename src/cron/{expression => }/expression_parse_test.v (99%) rename src/cron/{expression => }/expression_test.v (83%) rename src/cron/{expression => }/v.mod (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index be5f445..6b1e583 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Search in list of targets using API & CLI * Allow filtering targets by arch value +### Changed + +* Rewrote cron expression logic in C + ## [0.5.0](https://git.rustybever.be/vieter-v/vieter/src/tag/0.5.0) ### Added diff --git a/src/build/queue.v b/src/build/queue.v index e87024b..122180e 100644 --- a/src/build/queue.v +++ b/src/build/queue.v @@ -1,7 +1,7 @@ module build import models { BuildConfig, Target } -import cron.expression { CronExpression, parse_expression } +import cron import time import datatypes { MinHeap } import util @@ -13,7 +13,7 @@ pub mut: // Next timestamp from which point this job is allowed to be executed timestamp time.Time // Required for calculating next timestamp after having pop'ed a job - ce &CronExpression = unsafe { nil } + ce &cron.Expression = unsafe { nil } // Actual build config sent to the agent config BuildConfig // Whether this is a one-time job @@ -30,7 +30,7 @@ fn (r1 BuildJob) < (r2 BuildJob) bool { // for each architecture. Agents receive jobs from this queue. pub struct BuildJobQueue { // Schedule to use for targets without explicitely defined cron expression - default_schedule &CronExpression + default_schedule &cron.Expression // Base image to use for targets without defined base image default_base_image string mut: @@ -44,7 +44,7 @@ mut: } // new_job_queue initializes a new job queue -pub fn new_job_queue(default_schedule &CronExpression, default_base_image string) BuildJobQueue { +pub fn new_job_queue(default_schedule &cron.Expression, default_base_image string) BuildJobQueue { return BuildJobQueue{ default_schedule: unsafe { default_schedule } default_base_image: default_base_image @@ -85,7 +85,7 @@ pub fn (mut q BuildJobQueue) insert(input InsertConfig) ! { if !input.now { ce := if input.target.schedule != '' { - parse_expression(input.target.schedule) or { + cron.parse_expression(input.target.schedule) or { return error("Error while parsing cron expression '$input.target.schedule' (id $input.target.id): $err.msg()") } } else { diff --git a/src/console/schedule/schedule.v b/src/console/schedule/schedule.v index 7ce0516..40b300f 100644 --- a/src/console/schedule/schedule.v +++ b/src/console/schedule/schedule.v @@ -1,7 +1,7 @@ module schedule import cli -import cron.expression { parse_expression } +import cron import time // cmd returns the cli submodule for previewing a cron schedule. @@ -19,7 +19,7 @@ pub fn cmd() cli.Command { }, ] execute: fn (cmd cli.Command) ! { - ce := parse_expression(cmd.args.join(' '))! + ce := cron.parse_expression(cmd.args.join(' '))! count := cmd.flags.get_int('count')! for t in ce.next_n(time.now(), count)! { diff --git a/src/console/targets/targets.v b/src/console/targets/targets.v index 6152a53..709c196 100644 --- a/src/console/targets/targets.v +++ b/src/console/targets/targets.v @@ -2,7 +2,7 @@ module targets import cli import conf as vconf -import cron.expression { parse_expression } +import cron import client { NewTarget } import console import models { TargetFilter } @@ -295,7 +295,7 @@ fn patch(conf Config, id string, params map[string]string) ! { // We check the cron expression first because it's useless to send an // invalid one to the server. if 'schedule' in params && params['schedule'] != '' { - parse_expression(params['schedule']) or { + cron.parse_expression(params['schedule']) or { return error('Invalid cron expression: $err.msg()') } } diff --git a/src/cron/expression/c/expression.c b/src/cron/c/expression.c similarity index 91% rename from src/cron/expression/c/expression.c rename to src/cron/c/expression.c index f15e359..ed0306f 100644 --- a/src/cron/expression/c/expression.c +++ b/src/cron/c/expression.c @@ -3,11 +3,11 @@ const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}; -struct cron_expression *ce_init() { - return malloc(sizeof(struct cron_expression)); +cron_expression *ce_init() { + return malloc(sizeof(cron_expression)); } -void ce_free(struct cron_expression *ce) { +void ce_free(cron_expression *ce) { free(ce->months); free(ce->days); free(ce->hours); @@ -15,7 +15,7 @@ void ce_free(struct cron_expression *ce) { free(ce); } -int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cron_simple_time *ref) { +int ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref) { // For all of these values, the rule is the following: if their value is // the length of their respective array in the CronExpression object, that // means we've looped back around. This means that the "bigger" value has @@ -103,7 +103,7 @@ int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cro return 0; } -int ce_next_from_now(struct cron_simple_time *out, struct cron_expression *ce) { +int ce_next_from_now(cron_simple_time *out, cron_expression *ce) { time_t t = time(NULL); struct tm gm; gmtime_r(&t, &gm); diff --git a/src/cron/c/expression.h b/src/cron/c/expression.h new file mode 100644 index 0000000..c9441f6 --- /dev/null +++ b/src/cron/c/expression.h @@ -0,0 +1,40 @@ +#include +#include +#include +#include + +typedef enum cron_parse_error { + cron_parse_ok = 0, + cron_parse_invalid_expression = 1, + cron_parse_invalid_number = 2, + cron_parse_out_of_range = 3 +} cron_parse_error; + +typedef struct cron_expression { + uint8_t *minutes; + uint8_t *hours; + uint8_t *days; + uint8_t *months; + uint8_t minute_count; + uint8_t hour_count; + uint8_t day_count; + uint8_t month_count; +} cron_expression; + +typedef struct cron_simple_time { + int year; + int month; + int day; + int hour; + int minute; +} cron_simple_time; + +cron_expression *ce_init(); + +void cron_ce_free(cron_expression *ce); + +int cron_ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref); + +int cron_ce_next_from_now(cron_simple_time *out, cron_expression *ce); + +enum cron_parse_error cron_ce_parse_expression(cron_expression *out, char *s); diff --git a/src/cron/expression/c/parse.c b/src/cron/c/parse.c similarity index 92% rename from src/cron/expression/c/parse.c rename to src/cron/c/parse.c index cd23458..a577c6d 100644 --- a/src/cron/expression/c/parse.c +++ b/src/cron/c/parse.c @@ -9,10 +9,10 @@ const uint8_t max[4] = {59, 23, 31, 12}; #define SAFE_ATOI(v,s,min,max) \ int _##v = atoi(s); \ if ((_##v) == 0 && strcmp((s), "0") != 0) { \ - return CPEParseInvalidNumber; \ + return cron_parse_invalid_number; \ } \ if (v < (min) || v > (max)) { \ - return CPEParseOutOfRange; \ + return cron_parse_out_of_range; \ } \ v = (uint8_t) (_##v); @@ -37,12 +37,12 @@ enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_ if (s[0] == '*') { // A '*' is only valid on its own if (s[1] != '\0') { - return CPEParseInvalidExpression; + return cron_parse_invalid_expression; } *out = ~0; - return CPEParseOk; + return cron_parse_ok; } size_t slash_index = 0; @@ -90,7 +90,7 @@ enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_ } } - return CPEParseOk; + return cron_parse_ok; } /* @@ -108,7 +108,7 @@ enum cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t next[0] = '\0'; res = ce_parse_range(out, s, min, max); - if (res != CPEParseOk) { + if (res != cron_parse_ok) { return res; } @@ -170,7 +170,7 @@ uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { /* * Parse a cron expression string into a cron_expression struct. */ -enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) { +enum cron_parse_error ce_parse_expression(cron_expression *out, char *s) { // The parsing functions modify the input string in-place s = strdup(s); char *orig_s = s; @@ -178,7 +178,7 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) uint8_t part_count = 0; char *next; - enum cron_parse_error res = CPEParseOk; + enum cron_parse_error res = cron_parse_ok; uint64_t bfs[4]; // Skip leading spaces @@ -190,7 +190,7 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) next[0] = '\0'; res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - if (res != CPEParseOk) { + if (res != cron_parse_ok) { goto end; } @@ -209,7 +209,7 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) if (part_count < 4 && s[0] != '\0') { res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - if (res != CPEParseOk) { + if (res != cron_parse_ok) { goto end; } @@ -218,7 +218,7 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) // At least two parts need to be provided if (part_count < 2) { - res = CPEParseInvalidExpression; + res = cron_parse_invalid_expression; goto end; } diff --git a/src/cron/expression/expression.c.v b/src/cron/expression.c.v similarity index 88% rename from src/cron/expression/expression.c.v rename to src/cron/expression.c.v index fc97176..08af25c 100644 --- a/src/cron/expression/expression.c.v +++ b/src/cron/expression.c.v @@ -1,4 +1,4 @@ -module expression +module cron #flag -I @VMODROOT/c #flag @VMODROOT/c/parse.o @@ -16,7 +16,7 @@ pub struct C.cron_expression { month_count u8 } -pub type CronExpression = C.cron_expression +pub type Expression = C.cron_expression struct C.cron_simple_time { year int @@ -26,6 +26,8 @@ struct C.cron_simple_time { minute int } +type SimpleTime = C.cron_simple_time + fn C.ce_init() &C.cron_expression fn C.ce_free(ce &C.cron_expression) diff --git a/src/cron/expression/expression.v b/src/cron/expression.v similarity index 69% rename from src/cron/expression/expression.v rename to src/cron/expression.v index a51f562..0429b93 100644 --- a/src/cron/expression/expression.v +++ b/src/cron/expression.v @@ -1,8 +1,8 @@ -module expression +module cron import time -pub fn parse_expression(exp string) !&CronExpression { +pub fn parse_expression(exp string) !&Expression { out := C.ce_init() res := C.ce_parse_expression(out, exp.str) @@ -13,12 +13,12 @@ pub fn parse_expression(exp string) !&CronExpression { return out } -pub fn (ce &CronExpression) free() { +pub fn (ce &Expression) free() { C.ce_free(ce) } -pub fn (ce &CronExpression) next(ref time.Time) !time.Time { - st := C.cron_simple_time{ +pub fn (ce &Expression) next(ref time.Time) !time.Time { + st := SimpleTime{ year: ref.year month: ref.month day: ref.day @@ -26,7 +26,7 @@ pub fn (ce &CronExpression) next(ref time.Time) !time.Time { minute: ref.minute } - out := C.cron_simple_time{} + out := SimpleTime{} res := C.ce_next(&out, ce, &st) if res != 0 { @@ -42,8 +42,8 @@ pub fn (ce &CronExpression) next(ref time.Time) !time.Time { }) } -pub fn (ce &CronExpression) next_from_now() !time.Time { - out := C.cron_simple_time{} +pub fn (ce &Expression) next_from_now() !time.Time { + out := SimpleTime{} res := C.ce_next_from_now(&out, ce) if res != 0 { @@ -61,7 +61,7 @@ pub fn (ce &CronExpression) next_from_now() !time.Time { // next_n returns the n next occurences of the expression, given a starting // time. -pub fn (ce &CronExpression) next_n(ref time.Time, n int) ![]time.Time { +pub fn (ce &Expression) next_n(ref time.Time, n int) ![]time.Time { mut times := []time.Time{cap: n} times << ce.next(ref)! diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h deleted file mode 100644 index f6d8826..0000000 --- a/src/cron/expression/c/expression.h +++ /dev/null @@ -1,43 +0,0 @@ -#include -#include -#include -#include - -enum cron_parse_error { - CPEParseOk = 0, - CPEParseInvalidExpression = 1, - CPEParseInvalidNumber = 2, - CPEParseOutOfRange = 3 -}; - -struct cron_expression { - uint8_t *minutes; - uint8_t *hours; - uint8_t *days; - uint8_t *months; - uint8_t minute_count; - uint8_t hour_count; - uint8_t day_count; - uint8_t month_count; -}; - -struct cron_simple_time { - int year; - int month; - int day; - int hour; - int minute; -}; - -struct cron_expression *ce_init(); - -void cron_ce_free(struct cron_expression *ce); - -/** - * Given a - */ -int cron_ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct ce_simple_time *ref); - -int cron_ce_next_from_now(struct simple_time *out, struct cron_expression *ce); - -enum cron_parse_error cron_ce_parse_expression(struct cron_expression *out, char *s); diff --git a/src/cron/expression/expression_parse_test.v b/src/cron/expression_parse_test.v similarity index 99% rename from src/cron/expression/expression_parse_test.v rename to src/cron/expression_parse_test.v index 92e8291..0b0b605 100644 --- a/src/cron/expression/expression_parse_test.v +++ b/src/cron/expression_parse_test.v @@ -1,4 +1,4 @@ -module expression +module cron // parse_range_error returns the returned error message. If the result is '', // that means the function didn't error. diff --git a/src/cron/expression/expression_test.v b/src/cron/expression_test.v similarity index 83% rename from src/cron/expression/expression_test.v rename to src/cron/expression_test.v index 448927a..e1a9849 100644 --- a/src/cron/expression/expression_test.v +++ b/src/cron/expression_test.v @@ -1,4 +1,4 @@ -module expression +module cron import time { parse } @@ -19,7 +19,7 @@ fn util_test_time(exp string, t1_str string, t2_str string) ! { fn test_next_simple() ! { // Very simple - /* util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! */ + // util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! // Overlap to next day mut exp := '0 3 ' @@ -28,9 +28,9 @@ fn test_next_simple() ! { util_test_time('0 3/4', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! - /* // Overlap to next month */ + //// Overlap to next month util_test_time('0 3', '2002-11-31 04:00:00', '2002-12-01 03:00:00')! - /* // Overlap to next year */ + //// Overlap to next year util_test_time('0 3', '2002-12-31 04:00:00', '2003-01-01 03:00:00')! } diff --git a/src/cron/expression/v.mod b/src/cron/v.mod similarity index 100% rename from src/cron/expression/v.mod rename to src/cron/v.mod diff --git a/src/server/log_removal.v b/src/server/log_removal.v index 8e1a8c2..98cba93 100644 --- a/src/server/log_removal.v +++ b/src/server/log_removal.v @@ -3,12 +3,12 @@ module server import time import models { BuildLog } import os -import cron.expression { CronExpression } +import cron const fallback_log_removal_frequency = 24 * time.hour // log_removal_daemon removes old build logs every `log_removal_frequency`. -fn (mut app App) log_removal_daemon(schedule CronExpression) { +fn (mut app App) log_removal_daemon(schedule cron.Expression) { mut start_time := time.Time{} for { diff --git a/src/server/server.v b/src/server/server.v index 5dd1a20..ae086f5 100644 --- a/src/server/server.v +++ b/src/server/server.v @@ -7,7 +7,7 @@ import repo import util import db import build { BuildJobQueue } -import cron.expression +import cron import metrics const ( @@ -43,11 +43,11 @@ pub fn server(conf Config) ! { util.exit_with_message(1, "'any' is not allowed as the value for default_arch.") } - global_ce := expression.parse_expression(conf.global_schedule) or { + global_ce := cron.parse_expression(conf.global_schedule) or { util.exit_with_message(1, 'Invalid global cron expression: $err.msg()') } - log_removal_ce := expression.parse_expression(conf.log_removal_schedule) or { + log_removal_ce := cron.parse_expression(conf.log_removal_schedule) or { util.exit_with_message(1, 'Invalid log removal cron expression: $err.msg()') } From 243002f2828fca5996a720e7785e07cf482a6a46 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 18:44:33 +0100 Subject: [PATCH 11/30] fix(cron): ensure valid day values; some other stuff --- Makefile | 2 +- src/cron/c/expression.c | 7 +- src/cron/c/expression.h | 8 ++- src/cron/c/parse.c | 130 +++++++++++++++++++++++++++---------- src/cron/expression.v | 11 ++-- src/cron/expression_test.v | 2 +- 6 files changed, 112 insertions(+), 48 deletions(-) diff --git a/Makefile b/Makefile index c71ff1f..3571d30 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ # =====CONFIG===== SRC_DIR := src -SOURCES != find '$(SRC_DIR)' -iname '*.v' +SOURCES != find '$(SRC_DIR)' -\( -iname '*.v' -or -iname '*.h' -or -iname '*.c' -\) V_PATH ?= v V := $(V_PATH) -showcc -gc boehm -W -d use_openssl -skip-unused diff --git a/src/cron/c/expression.c b/src/cron/c/expression.c index ed0306f..0051e49 100644 --- a/src/cron/c/expression.c +++ b/src/cron/c/expression.c @@ -84,11 +84,6 @@ int ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref) { while (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { month_index++; - - // TODO find out if this can happen - if (month_index == 2 * ce->month_count) { - return 1; - } } } @@ -108,7 +103,7 @@ int ce_next_from_now(cron_simple_time *out, cron_expression *ce) { struct tm gm; gmtime_r(&t, &gm); - struct cron_simple_time ref = { + cron_simple_time ref = { .year = gm.tm_year, // tm_mon goes from 0 to 11 .month = gm.tm_mon + 1, diff --git a/src/cron/c/expression.h b/src/cron/c/expression.h index c9441f6..1c86436 100644 --- a/src/cron/c/expression.h +++ b/src/cron/c/expression.h @@ -1,3 +1,6 @@ +#ifndef VIETER_CRON +#define VIETER_CRON + #include #include #include @@ -7,7 +10,8 @@ typedef enum cron_parse_error { cron_parse_ok = 0, cron_parse_invalid_expression = 1, cron_parse_invalid_number = 2, - cron_parse_out_of_range = 3 + cron_parse_out_of_range = 3, + cron_parse_too_many_parts = 4 } cron_parse_error; typedef struct cron_expression { @@ -38,3 +42,5 @@ int cron_ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *r int cron_ce_next_from_now(cron_simple_time *out, cron_expression *ce); enum cron_parse_error cron_ce_parse_expression(cron_expression *out, char *s); + +#endif diff --git a/src/cron/c/parse.c b/src/cron/c/parse.c index a577c6d..f54b818 100644 --- a/src/cron/c/parse.c +++ b/src/cron/c/parse.c @@ -1,21 +1,28 @@ #include "expression.h" +const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}; + // Allowed value ranges for the minute, hour, day and month field const uint8_t min[4] = {0, 0, 1, 1}; const uint8_t max[4] = {59, 23, 31, 12}; -// Convert a string a uint8_t value by parsing it using atoi and checking +const uint8_t min_parts = 2; +const uint8_t max_parts = 4; + +// Convert a string into a uint8_t value by parsing it using atoi and checking // whether it's contained within the given range #define SAFE_ATOI(v,s,min,max) \ int _##v = atoi(s); \ if ((_##v) == 0 && strcmp((s), "0") != 0) { \ return cron_parse_invalid_number; \ } \ - if (v < (min) || v > (max)) { \ + if (((_##v) < (min)) || ((_##v) > (max))) { \ return cron_parse_out_of_range; \ } \ v = (uint8_t) (_##v); +#define MAX(x, y) (((x) > (y)) ? (x) : (y)) + /** * Given a range expression, produce a bit field defining what numbers in the * min-max range the expression represents. Bit 0 (starting from the @@ -32,7 +39,7 @@ const uint8_t max[4] = {59, 23, 31, 12}; * - a/c * - a-b/c */ -enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { +cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { // The * expression means "every possible value" if (s[0] == '*') { // A '*' is only valid on its own @@ -98,7 +105,7 @@ enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_ * min-max range the part represents. A part consists of one or more range * expressions, separated by commas. */ -enum cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { +cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { *out = 0; char *next; @@ -175,28 +182,30 @@ enum cron_parse_error ce_parse_expression(cron_expression *out, char *s) { s = strdup(s); char *orig_s = s; - uint8_t part_count = 0; - - char *next; enum cron_parse_error res = cron_parse_ok; - uint64_t bfs[4]; + uint64_t bfs[max_parts]; + + // First we divide the input string into its parts, divided by spaces. + // Each part is delimited by a NULL byte. + uint8_t part_count = 0; + char *parts[max_parts]; + char *next; // Skip leading spaces - while (s[0] == ' ') { - s++; + size_t offset = 0; + + while (s[offset] == ' ') { + offset++; } - while (part_count < 4 && ((next = strchr(s, ' ')) != NULL)) { + s += offset; + + while (part_count < max_parts && ((next = strchr(s, ' ')) != NULL)) { next[0] = '\0'; - res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - - if (res != cron_parse_ok) { - goto end; - } - - size_t offset = 1; + parts[part_count] = s; // Skip multiple spaces + offset = 1; while (next[offset] == ' ') { offset++; } @@ -205,34 +214,87 @@ enum cron_parse_error ce_parse_expression(cron_expression *out, char *s) { part_count++; } - // Parse final trailing part - if (part_count < 4 && s[0] != '\0') { - res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); + // The loop exited because we already have 4 parts, yet there's still at + // least one more part that follows. + if (next != NULL) { + res = cron_parse_too_many_parts; + } else if (s[0] != '\0') { + // There's one more excessive trailing part + if (part_count == max_parts) { + res = cron_parse_too_many_parts; + goto end; + } + + parts[part_count] = s; + part_count++; + } + + + // We now parse the parts in reverse. This is because the month part + // determines the maximum value of the day part. + + uint64_t bit_field = 0; + + // Months + if (part_count >= 4) { + res = ce_parse_part(&bit_field, parts[3], min[3], max[3]); if (res != cron_parse_ok) { goto end; } - part_count++; + out->month_count = bf_to_nums(&out->months, bit_field, min[3], max[3]); + } + // If months aren't provided, they're replaced with a * + else { + out->month_count = bf_to_nums(&out->months, ~0, min[3], max[3]); } - // At least two parts need to be provided - if (part_count < 2) { - res = cron_parse_invalid_expression; + // Determine what the largest allowed day value is, given the months + uint8_t max_day_value = 0; + + for (uint8_t i = 0; i < out->month_count; i++) { + max_day_value = MAX(max_day_value, month_days[out->months[i] - 1]); + } + + // Days + if (part_count >= 3) { + bit_field = 0; + + res = ce_parse_part(&bit_field, parts[2], min[2], max_day_value); + + if (res != cron_parse_ok) { + goto end; + } + + out->day_count = bf_to_nums(&out->days, bit_field, min[2], max_day_value); + } + // If days aren't provided, they're replaced with a * + else { + out->day_count = bf_to_nums(&out->days, ~0, min[2], max_day_value); + } + + // Hours + bit_field = 0; + + res = ce_parse_part(&bit_field, parts[1], min[1], max[1]); + + if (res != cron_parse_ok) { goto end; } - // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts - while (part_count < 4) { - // Expression is augmented with '*' expressions - bfs[part_count] = ~0; - part_count++; + out->hour_count = bf_to_nums(&out->hours, bit_field, min[1], max[1]); + + // Minutes + bit_field = 0; + + res = ce_parse_part(&bit_field, parts[0], min[0], max[0]); + + if (res != cron_parse_ok) { + goto end; } - out->minute_count = bf_to_nums(&out->minutes, bfs[0], min[0], max[0]); - out->hour_count = bf_to_nums(&out->hours, bfs[1], min[1], max[1]); - out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); - out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); + out->minute_count = bf_to_nums(&out->minutes, bit_field, min[0], max[0]); end: // s is cloned diff --git a/src/cron/expression.v b/src/cron/expression.v index 0429b93..23e1354 100644 --- a/src/cron/expression.v +++ b/src/cron/expression.v @@ -2,21 +2,22 @@ module cron import time +[unsafe] +pub fn (ce &Expression) free() { + C.ce_free(ce) +} + pub fn parse_expression(exp string) !&Expression { out := C.ce_init() res := C.ce_parse_expression(out, exp.str) if res != 0 { - return error('yuhh') + return error(res.str()) } return out } -pub fn (ce &Expression) free() { - C.ce_free(ce) -} - pub fn (ce &Expression) next(ref time.Time) !time.Time { st := SimpleTime{ year: ref.year diff --git a/src/cron/expression_test.v b/src/cron/expression_test.v index e1a9849..d6ec002 100644 --- a/src/cron/expression_test.v +++ b/src/cron/expression_test.v @@ -22,7 +22,7 @@ fn test_next_simple() ! { // util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! // Overlap to next day - mut exp := '0 3 ' + mut exp := '0 3 ' util_test_time(exp, '2002-01-01 03:00:00', '2002-01-02 03:00:00')! util_test_time(exp, '2002-01-01 04:00:00', '2002-01-02 03:00:00')! From 8c0315dea679fe52daa23d429310a7db645fbde8 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 18:51:01 +0100 Subject: [PATCH 12/30] refactor(cron): make next function infallible --- src/build/queue.v | 14 +++++--------- src/console/schedule/schedule.v | 2 +- src/cron/c/expression.c | 8 +++----- src/cron/c/expression.h | 4 ++-- src/cron/expression.c.v | 4 ++-- src/cron/expression.v | 22 +++++++--------------- src/cron/expression_test.v | 2 +- src/server/log_removal.v | 11 +---------- 8 files changed, 22 insertions(+), 45 deletions(-) diff --git a/src/build/queue.v b/src/build/queue.v index 122180e..abd4ec6 100644 --- a/src/build/queue.v +++ b/src/build/queue.v @@ -92,7 +92,7 @@ pub fn (mut q BuildJobQueue) insert(input InsertConfig) ! { q.default_schedule } - job.timestamp = ce.next_from_now()! + job.timestamp = ce.next_from_now() job.ce = ce } else { job.timestamp = time.now() @@ -105,8 +105,8 @@ pub fn (mut q BuildJobQueue) insert(input InsertConfig) ! { // reschedule the given job by calculating the next timestamp and re-adding it // to its respective queue. This function is called by the pop functions // *after* having pop'ed the job. -fn (mut q BuildJobQueue) reschedule(job BuildJob, arch string) ! { - new_timestamp := job.ce.next_from_now()! +fn (mut q BuildJobQueue) reschedule(job BuildJob, arch string) { + new_timestamp := job.ce.next_from_now() new_job := BuildJob{ ...job @@ -168,10 +168,7 @@ pub fn (mut q BuildJobQueue) pop(arch string) ?BuildJob { job = q.queues[arch].pop()? if !job.single { - // TODO how do we handle this properly? Is it even possible for a - // cron expression to not return a next time if it's already been - // used before? - q.reschedule(job, arch) or {} + q.reschedule(job, arch) } return job @@ -198,8 +195,7 @@ pub fn (mut q BuildJobQueue) pop_n(arch string, n int) []BuildJob { job = q.queues[arch].pop() or { break } if !job.single { - // TODO idem - q.reschedule(job, arch) or {} + q.reschedule(job, arch) } out << job diff --git a/src/console/schedule/schedule.v b/src/console/schedule/schedule.v index 40b300f..ceabf24 100644 --- a/src/console/schedule/schedule.v +++ b/src/console/schedule/schedule.v @@ -22,7 +22,7 @@ pub fn cmd() cli.Command { ce := cron.parse_expression(cmd.args.join(' '))! count := cmd.flags.get_int('count')! - for t in ce.next_n(time.now(), count)! { + for t in ce.next_n(time.now(), count) { println(t) } } diff --git a/src/cron/c/expression.c b/src/cron/c/expression.c index 0051e49..59d1bf0 100644 --- a/src/cron/c/expression.c +++ b/src/cron/c/expression.c @@ -15,7 +15,7 @@ void ce_free(cron_expression *ce) { free(ce); } -int ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref) { +void ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref) { // For all of these values, the rule is the following: if their value is // the length of their respective array in the CronExpression object, that // means we've looped back around. This means that the "bigger" value has @@ -94,11 +94,9 @@ int ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref) { } else { out->year = ref->year; } - - return 0; } -int ce_next_from_now(cron_simple_time *out, cron_expression *ce) { +void ce_next_from_now(cron_simple_time *out, cron_expression *ce) { time_t t = time(NULL); struct tm gm; gmtime_r(&t, &gm); @@ -112,5 +110,5 @@ int ce_next_from_now(cron_simple_time *out, cron_expression *ce) { .minute = gm.tm_min }; - return ce_next(out, ce, &ref); + ce_next(out, ce, &ref); } diff --git a/src/cron/c/expression.h b/src/cron/c/expression.h index 1c86436..c599e4e 100644 --- a/src/cron/c/expression.h +++ b/src/cron/c/expression.h @@ -37,9 +37,9 @@ cron_expression *ce_init(); void cron_ce_free(cron_expression *ce); -int cron_ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref); +void cron_ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref); -int cron_ce_next_from_now(cron_simple_time *out, cron_expression *ce); +void cron_ce_next_from_now(cron_simple_time *out, cron_expression *ce); enum cron_parse_error cron_ce_parse_expression(cron_expression *out, char *s); diff --git a/src/cron/expression.c.v b/src/cron/expression.c.v index 08af25c..7551e6f 100644 --- a/src/cron/expression.c.v +++ b/src/cron/expression.c.v @@ -32,8 +32,8 @@ fn C.ce_init() &C.cron_expression fn C.ce_free(ce &C.cron_expression) -fn C.ce_next(out &C.cron_simple_time, ce &C.cron_expression, ref &C.cron_simple_time) int +fn C.ce_next(out &C.cron_simple_time, ce &C.cron_expression, ref &C.cron_simple_time) -fn C.ce_next_from_now(out &C.cron_simple_time, ce &C.cron_expression) int +fn C.ce_next_from_now(out &C.cron_simple_time, ce &C.cron_expression) fn C.ce_parse_expression(out &C.cron_expression, s &char) int diff --git a/src/cron/expression.v b/src/cron/expression.v index 23e1354..c0cab8d 100644 --- a/src/cron/expression.v +++ b/src/cron/expression.v @@ -18,7 +18,7 @@ pub fn parse_expression(exp string) !&Expression { return out } -pub fn (ce &Expression) next(ref time.Time) !time.Time { +pub fn (ce &Expression) next(ref time.Time) time.Time { st := SimpleTime{ year: ref.year month: ref.month @@ -28,11 +28,7 @@ pub fn (ce &Expression) next(ref time.Time) !time.Time { } out := SimpleTime{} - res := C.ce_next(&out, ce, &st) - - if res != 0 { - return error('yuhh') - } + C.ce_next(&out, ce, &st) return time.new_time(time.Time{ year: out.year @@ -43,13 +39,9 @@ pub fn (ce &Expression) next(ref time.Time) !time.Time { }) } -pub fn (ce &Expression) next_from_now() !time.Time { +pub fn (ce &Expression) next_from_now() time.Time { out := SimpleTime{} - res := C.ce_next_from_now(&out, ce) - - if res != 0 { - return error('yuhh') - } + C.ce_next_from_now(&out, ce) return time.new_time(time.Time{ year: out.year @@ -62,13 +54,13 @@ pub fn (ce &Expression) next_from_now() !time.Time { // next_n returns the n next occurences of the expression, given a starting // time. -pub fn (ce &Expression) next_n(ref time.Time, n int) ![]time.Time { +pub fn (ce &Expression) next_n(ref time.Time, n int) []time.Time { mut times := []time.Time{cap: n} - times << ce.next(ref)! + times << ce.next(ref) for i in 1 .. n { - times << ce.next(times[i - 1])! + times << ce.next(times[i - 1]) } return times diff --git a/src/cron/expression_test.v b/src/cron/expression_test.v index d6ec002..7d1516d 100644 --- a/src/cron/expression_test.v +++ b/src/cron/expression_test.v @@ -8,7 +8,7 @@ fn util_test_time(exp string, t1_str string, t2_str string) ! { t1 := parse(t1_str)! t2 := parse(t2_str)! - t3 := ce.next(t1)! + t3 := ce.next(t1) assert t2.year == t3.year assert t2.month == t3.month diff --git a/src/server/log_removal.v b/src/server/log_removal.v index 98cba93..7f1cfb5 100644 --- a/src/server/log_removal.v +++ b/src/server/log_removal.v @@ -9,11 +9,7 @@ const fallback_log_removal_frequency = 24 * time.hour // log_removal_daemon removes old build logs every `log_removal_frequency`. fn (mut app App) log_removal_daemon(schedule cron.Expression) { - mut start_time := time.Time{} - for { - start_time = time.now() - mut too_old_timestamp := time.now().add_days(-app.conf.max_log_age) app.linfo('Cleaning logs before $too_old_timestamp') @@ -51,12 +47,7 @@ fn (mut app App) log_removal_daemon(schedule cron.Expression) { app.linfo('Cleaned $counter logs ($failed failed)') // Sleep until the next cycle - next_time := schedule.next_from_now() or { - app.lerror("Log removal daemon couldn't calculate next time: $err.msg(); fallback to $server.fallback_log_removal_frequency") - - start_time.add(server.fallback_log_removal_frequency) - } - + next_time := schedule.next_from_now() time.sleep(next_time - time.now()) } } From da5a77e489a6ca953571f058c3021177ef6d8aeb Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 19:08:35 +0100 Subject: [PATCH 13/30] feat(cron): proper parse error handling --- src/cron/expression.c.v | 20 +++++++++++++++++++- src/cron/expression.v | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/cron/expression.c.v b/src/cron/expression.c.v index 7551e6f..0698432 100644 --- a/src/cron/expression.c.v +++ b/src/cron/expression.c.v @@ -28,6 +28,24 @@ struct C.cron_simple_time { type SimpleTime = C.cron_simple_time +enum ParseError as u8 { + ok = 0 + invalid_expression = 1 + invalid_number = 2 + out_of_range = 3 + too_many_parts = 4 +} + +fn (e ParseError) str() string { + return match e { + .ok { '' } + .invalid_expression { 'Invalid expression' } + .invalid_number { 'Invalid number' } + .out_of_range { 'Out of range' } + .too_many_parts { 'Too many parts' } + } +} + fn C.ce_init() &C.cron_expression fn C.ce_free(ce &C.cron_expression) @@ -36,4 +54,4 @@ fn C.ce_next(out &C.cron_simple_time, ce &C.cron_expression, ref &C.cron_simple_ fn C.ce_next_from_now(out &C.cron_simple_time, ce &C.cron_expression) -fn C.ce_parse_expression(out &C.cron_expression, s &char) int +fn C.ce_parse_expression(out &C.cron_expression, s &char) ParseError diff --git a/src/cron/expression.v b/src/cron/expression.v index c0cab8d..4a0d04c 100644 --- a/src/cron/expression.v +++ b/src/cron/expression.v @@ -11,7 +11,7 @@ pub fn parse_expression(exp string) !&Expression { out := C.ce_init() res := C.ce_parse_expression(out, exp.str) - if res != 0 { + if res != .ok { return error(res.str()) } From 2f4acf59e3bcfdf81369e254e4473d43813173e3 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 20:46:03 +0100 Subject: [PATCH 14/30] fix(cron): fix some bugs --- src/cron/c/expression.c | 3 ++- src/cron/c/parse.c | 23 +++++++---------------- src/cron/expression_test.v | 8 ++++++++ 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/cron/c/expression.c b/src/cron/c/expression.c index 59d1bf0..4e1ca3c 100644 --- a/src/cron/c/expression.c +++ b/src/cron/c/expression.c @@ -102,7 +102,8 @@ void ce_next_from_now(cron_simple_time *out, cron_expression *ce) { gmtime_r(&t, &gm); cron_simple_time ref = { - .year = gm.tm_year, + // tm_year contains years since 1900 + .year = 1900 + gm.tm_year, // tm_mon goes from 0 to 11 .month = gm.tm_mon + 1, .day = gm.tm_mday, diff --git a/src/cron/c/parse.c b/src/cron/c/parse.c index f54b818..7a66200 100644 --- a/src/cron/c/parse.c +++ b/src/cron/c/parse.c @@ -40,18 +40,6 @@ const uint8_t max_parts = 4; * - a-b/c */ cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { - // The * expression means "every possible value" - if (s[0] == '*') { - // A '*' is only valid on its own - if (s[1] != '\0') { - return cron_parse_invalid_expression; - } - - *out = ~0; - - return cron_parse_ok; - } - size_t slash_index = 0; size_t dash_index = 0; size_t i = 0; @@ -74,14 +62,17 @@ cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max } // Parse the three possible numbers in the pattern - uint8_t start = 0; + uint8_t start = min; uint8_t end = max; uint8_t interval = 1; - SAFE_ATOI(start, s, min, max); + // * simply sets start as min and end as max + if (!(s[0] == '*' && strlen(s) == 1)) { + SAFE_ATOI(start, s, min, max); - if (dash_index > 0) { - SAFE_ATOI(end, &s[dash_index + 1], min, max); + if (dash_index > 0) { + SAFE_ATOI(end, &s[dash_index + 1], min, max); + } } if (slash_index > 0) { diff --git a/src/cron/expression_test.v b/src/cron/expression_test.v index 7d1516d..c016b72 100644 --- a/src/cron/expression_test.v +++ b/src/cron/expression_test.v @@ -34,3 +34,11 @@ fn test_next_simple() ! { //// Overlap to next year util_test_time('0 3', '2002-12-31 04:00:00', '2003-01-01 03:00:00')! } + +fn test_leading_star() { + mut x := false + + parse_expression('*5 8') or { x = true } + + assert x +} From a135068b3144e1e8bbb9513e367db9db08c9691a Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 20:48:14 +0100 Subject: [PATCH 15/30] chore: please the formatter --- src/cron/expression.c.v | 1 + src/cron/expression.v | 6 ++++++ src/cron/expression_test.v | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/cron/expression.c.v b/src/cron/expression.c.v index 0698432..cd4b8b0 100644 --- a/src/cron/expression.c.v +++ b/src/cron/expression.c.v @@ -36,6 +36,7 @@ enum ParseError as u8 { too_many_parts = 4 } +// str returns the string representation of a ParseError. fn (e ParseError) str() string { return match e { .ok { '' } diff --git a/src/cron/expression.v b/src/cron/expression.v index 4a0d04c..c463d06 100644 --- a/src/cron/expression.v +++ b/src/cron/expression.v @@ -2,11 +2,13 @@ module cron import time +// free the memory associated with the Expression. [unsafe] pub fn (ce &Expression) free() { C.ce_free(ce) } +// parse_expression parses a string into an Expression. pub fn parse_expression(exp string) !&Expression { out := C.ce_init() res := C.ce_parse_expression(out, exp.str) @@ -18,6 +20,8 @@ pub fn parse_expression(exp string) !&Expression { return out } +// next calculates the next occurence of the cron schedule, given a reference +// point. pub fn (ce &Expression) next(ref time.Time) time.Time { st := SimpleTime{ year: ref.year @@ -39,6 +43,8 @@ pub fn (ce &Expression) next(ref time.Time) time.Time { }) } +// next_from_now calculates the next occurence of the cron schedule with the +// current time as reference. pub fn (ce &Expression) next_from_now() time.Time { out := SimpleTime{} C.ce_next_from_now(&out, ce) diff --git a/src/cron/expression_test.v b/src/cron/expression_test.v index c016b72..9863ef5 100644 --- a/src/cron/expression_test.v +++ b/src/cron/expression_test.v @@ -37,7 +37,7 @@ fn test_next_simple() ! { fn test_leading_star() { mut x := false - + parse_expression('*5 8') or { x = true } assert x From b47d78c41caf77fe20b8e2eee1f0767a03762bfe Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 20:54:39 +0100 Subject: [PATCH 16/30] chore: remove outdated tests --- src/cron/expression_parse_test.v | 89 -------------------------------- src/cron/expression_test.v | 6 ++- 2 files changed, 4 insertions(+), 91 deletions(-) delete mode 100644 src/cron/expression_parse_test.v diff --git a/src/cron/expression_parse_test.v b/src/cron/expression_parse_test.v deleted file mode 100644 index 0b0b605..0000000 --- a/src/cron/expression_parse_test.v +++ /dev/null @@ -1,89 +0,0 @@ -module cron - -// parse_range_error returns the returned error message. If the result is '', -// that means the function didn't error. -fn parse_range_error(s string, min int, max int) string { - parse_range(s, min, max) or { return err.msg } - - return '' -} - -// =====parse_range===== -fn test_range_star_range() ! { - bf := parse_range('*', 0, 5)! - - assert bf_to_ints(bf, 0) == [0, 1, 2, 3, 4, 5] -} - -fn test_range_number() ! { - bf := parse_range('4', 0, 5)! - - assert bf_to_ints(bf, 0) == [4] -} - -fn test_range_number_too_large() ! { - assert parse_range_error('10', 0, 6) == 'Out of range.' -} - -fn test_range_number_too_small() ! { - assert parse_range_error('0', 2, 6) == 'Out of range.' -} - -fn test_range_number_invalid() ! { - assert parse_range_error('x', 0, 6) == 'Invalid number.' -} - -fn test_range_step_star_1() ! { - bf := parse_range('*/4', 0, 20)! - - assert bf_to_ints(bf, 0) == [0, 4, 8, 12, 16, 20] -} - -fn test_range_step_star_2() ! { - bf := parse_range('*/3', 1, 8)! - - assert bf_to_ints(bf, 1) == [1, 4, 7] -} - -fn test_range_step_star_too_large() ! { - assert parse_range_error('*/21', 0, 20) == 'Step size too large.' -} - -fn test_range_step_zero() ! { - assert parse_range_error('*/0', 0, 20) == 'Step size zero not allowed.' -} - -fn test_range_step_number() ! { - bf := parse_range('5/4', 2, 22)! - - assert bf_to_ints(bf, 2) == [5, 9, 13, 17, 21] -} - -fn test_range_step_number_too_large() ! { - assert parse_range_error('10/4', 0, 5) == 'Out of range.' -} - -fn test_range_step_number_too_small() ! { - assert parse_range_error('2/4', 5, 10) == 'Out of range.' -} - -fn test_range_dash() ! { - bf := parse_range('4-8', 0, 9)! - - assert bf_to_ints(bf, 0) == [4, 5, 6, 7, 8] -} - -fn test_range_dash_step() ! { - bf := parse_range('4-8/2', 0, 9)! - - assert bf_to_ints(bf, 0) == [4, 6, 8] -} - -// =====parse_part===== -fn test_part_single() ! { - assert parse_part('*', 0, 5)! == [0, 1, 2, 3, 4, 5] -} - -fn test_part_multiple() ! { - assert parse_part('*/2,2/3', 1, 8)! == [1, 2, 3, 5, 7, 8] -} diff --git a/src/cron/expression_test.v b/src/cron/expression_test.v index 9863ef5..4023012 100644 --- a/src/cron/expression_test.v +++ b/src/cron/expression_test.v @@ -26,7 +26,7 @@ fn test_next_simple() ! { util_test_time(exp, '2002-01-01 03:00:00', '2002-01-02 03:00:00')! util_test_time(exp, '2002-01-01 04:00:00', '2002-01-02 03:00:00')! - util_test_time('0 3/4', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! + util_test_time('0 3-7/4,7-19', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! //// Overlap to next month util_test_time('0 3', '2002-11-31 04:00:00', '2002-12-01 03:00:00')! @@ -37,8 +37,10 @@ fn test_next_simple() ! { fn test_leading_star() { mut x := false - parse_expression('*5 8') or { x = true } + assert x + x = false + parse_expression('x 8') or { x = true } assert x } From d0b5314619736c093b95d5a131463e21d1ac4603 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Thu, 12 Jan 2023 12:26:12 +0100 Subject: [PATCH 17/30] feat(cron): mostly written C expression parser --- .gitignore | 2 +- src/cron/expression/c/expression.h | 22 ++++ src/cron/expression/c/parse.c | 179 +++++++++++++++++++++++++++++ 3 files changed, 202 insertions(+), 1 deletion(-) create mode 100644 src/cron/expression/c/expression.h create mode 100644 src/cron/expression/c/parse.c diff --git a/.gitignore b/.gitignore index aaec9ef..daeb3d3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ -*.c +vieter.c /data/ # Build artifacts diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h new file mode 100644 index 0000000..5b8e4f9 --- /dev/null +++ b/src/cron/expression/c/expression.h @@ -0,0 +1,22 @@ +#include +#include +#include +#include + +typedef struct cron_expression { + uint8_t *minutes; + uint8_t *hours; + uint8_t *days; + uint8_t *months; + uint8_t minute_count; + uint8_t hour_count; + uint8_t day_count; + uint8_t month_count; +} CronExpression; + +/** + * Given a + */ +int ce_next(struct tm *out, struct tm *ref); + +int ce_parse(CronExpression *out, char *s); diff --git a/src/cron/expression/c/parse.c b/src/cron/expression/c/parse.c new file mode 100644 index 0000000..17d416f --- /dev/null +++ b/src/cron/expression/c/parse.c @@ -0,0 +1,179 @@ +#include "expression.h" + +const uint8_t min[4] = {0, 0, 1, 1}; +const uint8_t max[4] = {59, 23, 31, 12}; + +typedef enum parse_error { + ParseOk = 0, + ParseInvalidExpression = 1, + ParseInvalidNumber = 2, + ParseOutOfRange = 3 +} ParseError; + +#define SAFE_ATOI(v,s,min,max) \ + int _##v = atoi(s); \ + if ((_##v) == 0 && strcmp((s), "0") != 0) { \ + return ParseInvalidNumber; \ + } \ + if (v < (min) || v > (max)) { \ + return ParseOutOfRange; \ + } \ + v = (uint8_t) (_##v); + +/** + * Given a range expression, produce a bit field defining what numbers in the + * min-max range the expression represents. The first bit (starting from the + * right) corresponds to min, the max - min + 1'th bit to max. All trailing bits + * after this should be ignored. The given bitfield is modified in-place, so + * multiple calls of this function can be performed on the same value to create + * the effect of ORing their values: + * + * A range expression has one of the following forms: + * + * - * + * - a + * - a-b + * - a/c + * - a-b/c + */ +ParseError ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { + // The * expression means "every possible value" + if (s[0] == '*') { + // A '*' is only valid on its own + if (s[1] != '\0') { + return ParseInvalidExpression; + } + + *out = ~0; + + return ParseOk; + } + + size_t slash_index = 0; + size_t dash_index = 0; + size_t i = 0; + + // We first iterate over the string to determine whether it contains a slash + // and/or a dash. We know the dash can only be valid if it appears before + // the slash. + while (s[i] != '\0' && slash_index == 0) { + if (s[i] == '/') { + slash_index = i; + + s[i] = '\0'; + } else if (s[i] == '-') { + dash_index = i; + + s[i] = '\0'; + } + + i++; + } + + // Parse the three possible numbers in the pattern + uint8_t start = 0; + uint8_t end = 0; + uint8_t interval = 1; + + SAFE_ATOI(start, s, min, max); + + if (dash_index > 0) { + SAFE_ATOI(end, &s[dash_index + 1], min, max); + } + + if (slash_index > 0) { + SAFE_ATOI(interval, &s[slash_index + 1], 1, max - min); + } + + // Single number doesn't need to loop + if (end == 0 && slash_index == 0) { + *out |= 1 << (start - min); + } else { + for (;start <= end; start += interval) { + *out |= 1 << (start - min); + start += interval; + } + } + + return ParseOk; +} + +ParseError ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { + *out = 0; + + char *next; + ParseError res; + + while ((next = strchr(s, ',')) != NULL) { + next[0] = '\0'; + res = ce_parse_range(out, s, min, max); + + if (res != ParseOk) { + return res; + } + + s = next + 1; + } + + // Make sure to parse the final range as well + res = ce_parse_range(out, s, min, max); + + if (res != ParseOk) { + return res; + } + + return ParseOk; +} + +ParseError ce_parse_expression(uint64_t *out, char *s) { + uint8_t part_count = 0; + + char *next; + ParseError res; + + // Skip leading spaces + while (s[0] == ' ') { + s++; + } + + while (part_count < 4 && (next = strchr(s, ' ')) != NULL) { + next[0] = '\0'; + res = ce_parse_part(&out[part_count], s, min[part_count], max[part_count]); + + if (res != ParseOk) { + return res; + } + + s = next + 1; + size_t offset = 1; + + // Skip multiple spaces + while (next[offset] == ' ') { + offset++; + } + s = next + offset; + + part_count++; + } + + // Parse final trailing part + if (part_count < 4 && s[0] != '\0') { + // Make sure to parse the final range as well + res = ce_parse_part(&out[part_count], s, min[part_count], max[part_count]); + + if (res != ParseOk) { + return res; + } + + part_count++; + } + + // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts + while (part_count < 4) { + // Expression is augmented with '*' expressions + out[part_count] = ~0; + part_count++; + } + + return ParseOk; +} From aba1ff4de79b1510814e189d032c7b888cf2dc7a Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Thu, 12 Jan 2023 21:08:58 +0100 Subject: [PATCH 18/30] feat(cron): rest of parser in C --- src/cron/expression/c/expression.h | 9 ++++- src/cron/expression/c/parse.c | 55 ++++++++++++++++++++++++------ src/cron/expression/expression.c.v | 18 ++++++++++ src/cron/expression/v.mod | 0 4 files changed, 70 insertions(+), 12 deletions(-) create mode 100644 src/cron/expression/expression.c.v create mode 100644 src/cron/expression/v.mod diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h index 5b8e4f9..799a9a3 100644 --- a/src/cron/expression/c/expression.h +++ b/src/cron/expression/c/expression.h @@ -3,6 +3,13 @@ #include #include +typedef enum parse_error { + ParseOk = 0, + ParseInvalidExpression = 1, + ParseInvalidNumber = 2, + ParseOutOfRange = 3 +} ParseError; + typedef struct cron_expression { uint8_t *minutes; uint8_t *hours; @@ -19,4 +26,4 @@ typedef struct cron_expression { */ int ce_next(struct tm *out, struct tm *ref); -int ce_parse(CronExpression *out, char *s); +ParseError ce_parse_expression(CronExpression *out, char *s); diff --git a/src/cron/expression/c/parse.c b/src/cron/expression/c/parse.c index 17d416f..cb97373 100644 --- a/src/cron/expression/c/parse.c +++ b/src/cron/expression/c/parse.c @@ -3,13 +3,6 @@ const uint8_t min[4] = {0, 0, 1, 1}; const uint8_t max[4] = {59, 23, 31, 12}; -typedef enum parse_error { - ParseOk = 0, - ParseInvalidExpression = 1, - ParseInvalidNumber = 2, - ParseOutOfRange = 3 -} ParseError; - #define SAFE_ATOI(v,s,min,max) \ int _##v = atoi(s); \ if ((_##v) == 0 && strcmp((s), "0") != 0) { \ @@ -125,11 +118,41 @@ ParseError ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { return ParseOk; } -ParseError ce_parse_expression(uint64_t *out, char *s) { +uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { + uint8_t capacity = 8; + uint8_t size = 0; + + uint8_t *buf = malloc(capacity * sizeof(uint8_t)); + + for (uint8_t i = 0; i <= max - min; i++) { + if ((1 << i) & bf) { + // Resize buffer if needed + if (size == capacity) { + capacity *= 2; + buf = realloc(buf, capacity * sizeof(uint8_t)); + } + + buf[size] = min + i; + size++; + } + } + + // Resize buffer once more to remove any trailing unused bytes + if (size < capacity) { + buf = realloc(buf, size * sizeof(uint8_t)); + } + + *out = buf; + + return size; +} + +ParseError ce_parse_expression(CronExpression *out, char *s) { uint8_t part_count = 0; char *next; ParseError res; + uint64_t bfs[4]; // Skip leading spaces while (s[0] == ' ') { @@ -138,7 +161,7 @@ ParseError ce_parse_expression(uint64_t *out, char *s) { while (part_count < 4 && (next = strchr(s, ' ')) != NULL) { next[0] = '\0'; - res = ce_parse_part(&out[part_count], s, min[part_count], max[part_count]); + res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); if (res != ParseOk) { return res; @@ -159,7 +182,7 @@ ParseError ce_parse_expression(uint64_t *out, char *s) { // Parse final trailing part if (part_count < 4 && s[0] != '\0') { // Make sure to parse the final range as well - res = ce_parse_part(&out[part_count], s, min[part_count], max[part_count]); + res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); if (res != ParseOk) { return res; @@ -168,12 +191,22 @@ ParseError ce_parse_expression(uint64_t *out, char *s) { part_count++; } + // At least two parts need to be provided + if (part_count < 2) { + return ParseInvalidExpression; + } + // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts while (part_count < 4) { // Expression is augmented with '*' expressions - out[part_count] = ~0; + bfs[part_count] = ~0; part_count++; } + out->minute_count = bf_to_nums(&out->minutes, bfs[0], min[0], max[0]); + out->hour_count = bf_to_nums(&out->hours, bfs[1], min[1], max[1]); + out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); + out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); + return ParseOk; } diff --git a/src/cron/expression/expression.c.v b/src/cron/expression/expression.c.v new file mode 100644 index 0000000..ec39fd6 --- /dev/null +++ b/src/cron/expression/expression.c.v @@ -0,0 +1,18 @@ +module expression + +#flag -I @VMODROOT/c +#flag @VMODROOT/c/parse.o +#include "expression.h" + +pub struct C.CronExpression { + minutes &u8 + hours &u8 + days &u8 + months &u8 + minute_count u8 + hour_count u8 + day_count u8 + month_count u8 +} + +/* pub type CronExpression = C.CronExpression */ diff --git a/src/cron/expression/v.mod b/src/cron/expression/v.mod new file mode 100644 index 0000000..e69de29 From 2d50889e8efc8e44dde63b3251e7dcb69bf7aff2 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Thu, 12 Jan 2023 21:31:32 +0100 Subject: [PATCH 19/30] feat(cron): next function in C --- src/cron/expression/c/expression.c | 90 ++++++++++++++++++++++++++++++ src/cron/expression/c/expression.h | 10 +++- 2 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 src/cron/expression/c/expression.c diff --git a/src/cron/expression/c/expression.c b/src/cron/expression/c/expression.c new file mode 100644 index 0000000..3f65b6a --- /dev/null +++ b/src/cron/expression/c/expression.c @@ -0,0 +1,90 @@ +#include "expression.h" + +const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}; + +int ce_next(SimpleTime *out, CronExpression *ce, SimpleTime *ref) { + // For all of these values, the rule is the following: if their value is + // the length of their respective array in the CronExpression object, that + // means we've looped back around. This means that the "bigger" value has + // to be incremented by one. For example, if the minutes have looped + // around, that means that the hour has to be incremented as well. + uint8_t month_index = 0; + uint8_t day_index = 0; + uint8_t hour_index = 0; + uint8_t minute_index = 0; + + // This chain is the same logic multiple times, namely that if a "bigger" + // value loops around, then the smaller value will always reset as well. + // For example, if we're going to a new day, the hour & minute will always + // be their smallest value again. + while (month_index < ce->month_count && ref->month > ce->months[month_index]) { + month_index++; + } + + if (month_index < ce->month_count && ref->month == ce->months[month_index]) { + while (day_index < ce->day_count && ref->day > ce->days[day_index]) { + day_index++; + } + + if (day_index < ce->days_count && ref->day == ce->days[day_index]) { + while (hour_index < ce->hour_count && ref->hour > ce->hours[hour_index]) { + hour_index++; + } + + if (hour_index < ce->hours_count && ref->hour == ce->hours[hour_index]) { + // Minute is the only value where we explicitely make sure we + // can't match sref's value exactly. This is to ensure we only + // return values in the future. + while (minute_index < ce->minute_count && ref->minute > ce->minutes[minute_index]) { + minute_index++; + } + } + } + } + + // Here, we increment the "bigger" values by one if the smaller ones loop + // around. The order is important, as it allows a sort-of waterfall effect + // to occur which updates all values if required. + if (minute_index == ce->minute_count && hour_index < ce->hour_count) { + hour_index++; + } + + if (hour_index == ce->hour_count && day_index < ce->day_count) { + day_index++; + } + + if (day_index == ce->day_count && month_index < ce->month_count) { + month_index++; + } + + out->minute = ce->minutes[minute_index % ce->minute_count]; + out->hour = ce->hours[hour_index % ce->hour_count]; + out->day = ce->days[day_index % ce->day_count]; + + // Sometimes, we end up with a day that does not exist within the selected + // month, e.g. day 30 in February. When this occurs, we reset day back to + // the smallest value & loop over to the next month that does have this + // day. + if (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { + out->day = ce->days[0]; + month_index++; + + while (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { + month_index++; + + if (month_index == 2 * ce->month_count) { + return 1; + } + } + } + + out->month = ce->months[month_index * ce->month_count]; + + if (month_index >= ce->month_count) { + out->year = ref->year + 1; + } else { + out->year = ref->year; + } + + return 0; +} diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h index 799a9a3..7abb189 100644 --- a/src/cron/expression/c/expression.h +++ b/src/cron/expression/c/expression.h @@ -21,9 +21,17 @@ typedef struct cron_expression { uint8_t month_count; } CronExpression; +typedef struct simple_time { + int year; + int month; + int day; + int hour; + int minute; +} SimpleTime; + /** * Given a */ -int ce_next(struct tm *out, struct tm *ref); +int ce_next(SimpleTime *out, CronExpression *ce, SimpleTime *ref); ParseError ce_parse_expression(CronExpression *out, char *s); From 84e7e14a199e40774fe1621101626f09b7ab441e Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Fri, 13 Jan 2023 19:03:58 +0100 Subject: [PATCH 20/30] chore: remove old cron daemon code --- src/cron/cli.v | 32 ----- src/cron/cron.v | 33 ----- src/cron/daemon/build.v | 115 ---------------- src/cron/daemon/daemon.v | 274 --------------------------------------- src/cron/daemon/log.v | 36 ----- src/main.v | 2 - 6 files changed, 492 deletions(-) delete mode 100644 src/cron/cli.v delete mode 100644 src/cron/cron.v delete mode 100644 src/cron/daemon/build.v delete mode 100644 src/cron/daemon/daemon.v delete mode 100644 src/cron/daemon/log.v diff --git a/src/cron/cli.v b/src/cron/cli.v deleted file mode 100644 index 16a3537..0000000 --- a/src/cron/cli.v +++ /dev/null @@ -1,32 +0,0 @@ -module cron - -import cli -import conf as vconf - -struct Config { -pub: - log_level string = 'WARN' - api_key string - address string - data_dir string - base_image string = 'archlinux:base-devel' - max_concurrent_builds int = 1 - api_update_frequency int = 15 - image_rebuild_frequency int = 1440 - // Replicates the behavior of the original cron system - global_schedule string = '0 3' -} - -// cmd returns the cli module that handles the cron daemon. -pub fn cmd() cli.Command { - return cli.Command{ - name: 'cron' - description: 'Start the cron service that periodically runs builds.' - execute: fn (cmd cli.Command) ! { - config_file := cmd.flags.get_string('config-file')! - conf := vconf.load(prefix: 'VIETER_', default_path: config_file)! - - cron(conf)! - } - } -} diff --git a/src/cron/cron.v b/src/cron/cron.v deleted file mode 100644 index f1d6b7b..0000000 --- a/src/cron/cron.v +++ /dev/null @@ -1,33 +0,0 @@ -module cron - -import log -import cron.daemon -import cron.expression -import os - -const log_file_name = 'vieter.cron.log' - -// cron starts a cron daemon & starts periodically scheduling builds. -pub fn cron(conf Config) ! { - // Configure logger - log_level := log.level_from_tag(conf.log_level) or { - return error('Invalid log level. The allowed values are FATAL, ERROR, WARN, INFO & DEBUG.') - } - - mut logger := log.Log{ - level: log_level - } - - log_file := os.join_path_single(conf.data_dir, cron.log_file_name) - logger.set_full_logpath(log_file) - logger.log_to_console_too() - - ce := expression.parse_expression(conf.global_schedule) or { - return error('Error while parsing global cron expression: $err.msg()') - } - - mut d := daemon.init_daemon(logger, conf.address, conf.api_key, conf.base_image, ce, - conf.max_concurrent_builds, conf.api_update_frequency, conf.image_rebuild_frequency)! - - d.run() -} diff --git a/src/cron/daemon/build.v b/src/cron/daemon/build.v deleted file mode 100644 index 42edc92..0000000 --- a/src/cron/daemon/build.v +++ /dev/null @@ -1,115 +0,0 @@ -module daemon - -import time -import sync.stdatomic -import build -import os - -const ( - build_empty = 0 - build_running = 1 - build_done = 2 -) - -// clean_finished_builds removes finished builds from the build slots & returns -// them. -fn (mut d Daemon) clean_finished_builds() []ScheduledBuild { - mut out := []ScheduledBuild{} - - for i in 0 .. d.atomics.len { - if stdatomic.load_u64(&d.atomics[i]) == daemon.build_done { - stdatomic.store_u64(&d.atomics[i], daemon.build_empty) - out << d.builds[i] - } - } - - return out -} - -// update_builds starts as many builds as possible. -fn (mut d Daemon) start_new_builds() { - now := time.now() - - for d.queue.len() > 0 { - elem := d.queue.peek() or { - d.lerror("queue.peek() unexpectedly returned an error. This shouldn't happen.") - - break - } - - if elem.timestamp < now { - sb := d.queue.pop() or { - d.lerror("queue.pop() unexpectedly returned an error. This shouldn't happen.") - - break - } - - // If this build couldn't be scheduled, no more will be possible. - if !d.start_build(sb) { - d.queue.insert(sb) - break - } - } else { - break - } - } -} - -// start_build starts a build for the given ScheduledBuild object. -fn (mut d Daemon) start_build(sb ScheduledBuild) bool { - for i in 0 .. d.atomics.len { - if stdatomic.load_u64(&d.atomics[i]) == daemon.build_empty { - stdatomic.store_u64(&d.atomics[i], daemon.build_running) - d.builds[i] = sb - - go d.run_build(i, sb) - - return true - } - } - - return false -} - -// run_build actually starts the build process for a given target. -fn (mut d Daemon) run_build(build_index int, sb ScheduledBuild) { - d.linfo('started build: $sb.target.url -> $sb.target.repo') - - // 0 means success, 1 means failure - mut status := 0 - - res := build.build_target(d.client.address, d.client.api_key, d.builder_images.last(), - &sb.target, false) or { - d.ldebug('build_target error: $err.msg()') - status = 1 - - build.BuildResult{} - } - - if status == 0 { - d.linfo('finished build: $sb.target.url -> $sb.target.repo; uploading logs...') - - build_arch := os.uname().machine - d.client.add_build_log(sb.target.id, res.start_time, res.end_time, build_arch, - res.exit_code, res.logs) or { - d.lerror('Failed to upload logs for build: $sb.target.url -> $sb.target.repo') - } - } else { - d.linfo('an error occured during build: $sb.target.url -> $sb.target.repo') - } - - stdatomic.store_u64(&d.atomics[build_index], daemon.build_done) -} - -// current_build_count returns how many builds are currently running. -fn (mut d Daemon) current_build_count() int { - mut res := 0 - - for i in 0 .. d.atomics.len { - if stdatomic.load_u64(&d.atomics[i]) == daemon.build_running { - res += 1 - } - } - - return res -} diff --git a/src/cron/daemon/daemon.v b/src/cron/daemon/daemon.v deleted file mode 100644 index b94dab8..0000000 --- a/src/cron/daemon/daemon.v +++ /dev/null @@ -1,274 +0,0 @@ -module daemon - -import time -import log -import datatypes { MinHeap } -import cron.expression { CronExpression, parse_expression } -import math -import build -import docker -import os -import client -import models { Target } - -const ( - // How many seconds to wait before retrying to update API if failed - api_update_retry_timeout = 5 - // How many seconds to wait before retrying to rebuild image if failed - rebuild_base_image_retry_timout = 30 -) - -struct ScheduledBuild { -pub: - target Target - timestamp time.Time -} - -// Overloaded operator for comparing ScheduledBuild objects -fn (r1 ScheduledBuild) < (r2 ScheduledBuild) bool { - return r1.timestamp < r2.timestamp -} - -pub struct Daemon { -mut: - client client.Client - base_image string - builder_images []string - global_schedule CronExpression - api_update_frequency int - image_rebuild_frequency int - // Targets currently loaded from API. - targets []Target - // At what point to update the list of targets. - api_update_timestamp time.Time - image_build_timestamp time.Time - queue MinHeap - // Which builds are currently running - builds []ScheduledBuild - // Atomic variables used to detect when a build has finished; length is the - // same as builds - atomics []u64 - logger shared log.Log -} - -// init_daemon initializes a new Daemon object. It renews the targets & -// populates the build queue for the first time. -pub fn init_daemon(logger log.Log, address string, api_key string, base_image string, global_schedule CronExpression, max_concurrent_builds int, api_update_frequency int, image_rebuild_frequency int) !Daemon { - mut d := Daemon{ - client: client.new(address, api_key) - base_image: base_image - global_schedule: global_schedule - api_update_frequency: api_update_frequency - image_rebuild_frequency: image_rebuild_frequency - atomics: []u64{len: max_concurrent_builds} - builds: []ScheduledBuild{len: max_concurrent_builds} - logger: logger - } - - // Initialize the targets & queue - d.renew_targets() - d.renew_queue() - if !d.rebuild_base_image() { - return error('The base image failed to build. The Vieter cron daemon cannot run without an initial builder image.') - } - - return d -} - -// run starts the actual daemon process. It runs builds when possible & -// periodically refreshes the list of targets to ensure we stay in sync. -pub fn (mut d Daemon) run() { - for { - finished_builds := d.clean_finished_builds() - - // Update the API's contents if needed & renew the queue - if time.now() >= d.api_update_timestamp { - d.renew_targets() - d.renew_queue() - } - // The finished builds should only be rescheduled if the API contents - // haven't been renewed. - else { - for sb in finished_builds { - d.schedule_build(sb.target) - } - } - - // TODO remove old builder images. - // This issue is less trivial than it sounds, because a build could - // still be running when the image has to be rebuilt. That would - // prevent the image from being removed. Therefore, we will need to - // keep track of a list or something & remove an image once we have - // made sure it isn't being used anymore. - if time.now() >= d.image_build_timestamp { - d.rebuild_base_image() - // In theory, executing this function here allows an old builder - // image to exist for at most image_rebuild_frequency minutes. - d.clean_old_base_images() - } - - // Schedules new builds when possible - d.start_new_builds() - - // If there are builds currently running, the daemon should refresh - // every second to clean up any finished builds & start new ones. - mut delay := time.Duration(1 * time.second) - - // Sleep either until we have to refresh the targets or when the next - // build has to start, with a minimum of 1 second. - if d.current_build_count() == 0 { - now := time.now() - delay = d.api_update_timestamp - now - - if d.queue.len() > 0 { - elem := d.queue.peek() or { - d.lerror("queue.peek() unexpectedly returned an error. This shouldn't happen.") - - // This is just a fallback option. In theory, queue.peek() - // should *never* return an error or none, because we check - // its len beforehand. - time.sleep(1) - continue - } - - time_until_next_job := elem.timestamp - now - - delay = math.min(delay, time_until_next_job) - } - } - - // We sleep for at least one second. This is to prevent the program - // from looping agressively when a cronjob can be scheduled, but - // there's no spots free for it to be started. - delay = math.max(delay, 1 * time.second) - - d.ldebug('Sleeping for ${delay}...') - - time.sleep(delay) - } -} - -// schedule_build adds the next occurence of the given targets build to the -// queue. -fn (mut d Daemon) schedule_build(target Target) { - ce := if target.schedule != '' { - parse_expression(target.schedule) or { - // TODO This shouldn't return an error if the expression is empty. - d.lerror("Error while parsing cron expression '$target.schedule' (id $target.id): $err.msg()") - - d.global_schedule - } - } else { - d.global_schedule - } - - // A target that can't be scheduled will just be skipped for now - timestamp := ce.next_from_now() or { - d.lerror("Couldn't calculate next timestamp from '$target.schedule'; skipping") - return - } - - d.queue.insert(ScheduledBuild{ - target: target - timestamp: timestamp - }) -} - -// renew_targets requests the newest list of targets from the server & replaces -// the old one. -fn (mut d Daemon) renew_targets() { - d.linfo('Renewing targets...') - - mut new_targets := d.client.get_all_targets() or { - d.lerror('Failed to renew targets. Retrying in ${daemon.api_update_retry_timeout}s...') - d.api_update_timestamp = time.now().add_seconds(daemon.api_update_retry_timeout) - - return - } - - // Filter out any targets that shouldn't run on this architecture - cur_arch := os.uname().machine - new_targets = new_targets.filter(it.arch.any(it.value == cur_arch)) - - d.targets = new_targets - - d.api_update_timestamp = time.now().add_seconds(60 * d.api_update_frequency) -} - -// renew_queue replaces the old queue with a new one that reflects the newest -// values in targets. -fn (mut d Daemon) renew_queue() { - d.linfo('Renewing queue...') - mut new_queue := MinHeap{} - - // Move any jobs that should have already started from the old queue onto - // the new one - now := time.now() - - // For some reason, using - // ```v - // for d.queue.len() > 0 && d.queue.peek() !.timestamp < now { - //``` - // here causes the function to prematurely just exit, without any errors or anything, very weird - // https://github.com/vlang/v/issues/14042 - for d.queue.len() > 0 { - elem := d.queue.pop() or { - d.lerror("queue.pop() returned an error. This shouldn't happen.") - continue - } - - if elem.timestamp < now { - new_queue.insert(elem) - } else { - break - } - } - - d.queue = new_queue - - // For each target in targets, parse their cron expression (or use the - // default one if not present) & add them to the queue - for target in d.targets { - d.schedule_build(target) - } -} - -// rebuild_base_image recreates the builder image. -fn (mut d Daemon) rebuild_base_image() bool { - d.linfo('Rebuilding builder image....') - - d.builder_images << build.create_build_image(d.base_image) or { - d.lerror('Failed to rebuild base image. Retrying in ${daemon.rebuild_base_image_retry_timout}s...') - d.image_build_timestamp = time.now().add_seconds(daemon.rebuild_base_image_retry_timout) - - return false - } - - d.image_build_timestamp = time.now().add_seconds(60 * d.image_rebuild_frequency) - - return true -} - -// clean_old_base_images tries to remove any old but still present builder -// images. -fn (mut d Daemon) clean_old_base_images() { - mut i := 0 - - mut dd := docker.new_conn() or { - d.lerror('Failed to connect to Docker socket.') - return - } - - defer { - dd.close() or {} - } - - for i < d.builder_images.len - 1 { - // For each builder image, we try to remove it by calling the Docker - // API. If the function returns an error or false, that means the image - // wasn't deleted. Therefore, we move the index over. If the function - // returns true, the array's length has decreased by one so we don't - // move the index. - dd.image_remove(d.builder_images[i]) or { i += 1 } - } -} diff --git a/src/cron/daemon/log.v b/src/cron/daemon/log.v deleted file mode 100644 index 4f978fc..0000000 --- a/src/cron/daemon/log.v +++ /dev/null @@ -1,36 +0,0 @@ -module daemon - -// lfatal create a log message with the fatal level -pub fn (mut d Daemon) lfatal(msg string) { - lock d.logger { - d.logger.fatal(msg) - } -} - -// lerror create a log message with the error level -pub fn (mut d Daemon) lerror(msg string) { - lock d.logger { - d.logger.error(msg) - } -} - -// lwarn create a log message with the warn level -pub fn (mut d Daemon) lwarn(msg string) { - lock d.logger { - d.logger.warn(msg) - } -} - -// linfo create a log message with the info level -pub fn (mut d Daemon) linfo(msg string) { - lock d.logger { - d.logger.info(msg) - } -} - -// ldebug create a log message with the debug level -pub fn (mut d Daemon) ldebug(msg string) { - lock d.logger { - d.logger.debug(msg) - } -} diff --git a/src/main.v b/src/main.v index 1c8b816..ce9ec81 100644 --- a/src/main.v +++ b/src/main.v @@ -9,7 +9,6 @@ import console.schedule import console.man import console.aur import console.repos -import cron import agent fn main() { @@ -43,7 +42,6 @@ fn main() { commands: [ server.cmd(), targets.cmd(), - cron.cmd(), logs.cmd(), schedule.cmd(), man.cmd(), From fec8118ff5ab96ce7b20ba6c1c40aa18ce480bcd Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Thu, 12 Jan 2023 21:52:51 +0100 Subject: [PATCH 21/30] feat(cron): first step of replacing cron with C implementation --- Makefile | 2 +- src/build/queue.v | 8 +- src/cron/expression/c/expression.c | 36 +++++- src/cron/expression/c/expression.h | 30 +++-- src/cron/expression/c/parse.c | 39 ++++--- src/cron/expression/expression.c.v | 37 +++++-- src/cron/expression/expression.v | 146 +++++++------------------ src/cron/expression/expression_parse.v | 146 ------------------------- src/cron/expression/expression_test.v | 18 +-- 9 files changed, 157 insertions(+), 305 deletions(-) delete mode 100644 src/cron/expression/expression_parse.v diff --git a/Makefile b/Makefile index 4bd1edc..c71ff1f 100644 --- a/Makefile +++ b/Makefile @@ -81,7 +81,7 @@ fmt: .PHONY: test test: - $(V) test $(SRC_DIR) + $(V) -g test $(SRC_DIR) .PHONY: clean clean: diff --git a/src/build/queue.v b/src/build/queue.v index e74529c..e87024b 100644 --- a/src/build/queue.v +++ b/src/build/queue.v @@ -13,7 +13,7 @@ pub mut: // Next timestamp from which point this job is allowed to be executed timestamp time.Time // Required for calculating next timestamp after having pop'ed a job - ce CronExpression + ce &CronExpression = unsafe { nil } // Actual build config sent to the agent config BuildConfig // Whether this is a one-time job @@ -30,7 +30,7 @@ fn (r1 BuildJob) < (r2 BuildJob) bool { // for each architecture. Agents receive jobs from this queue. pub struct BuildJobQueue { // Schedule to use for targets without explicitely defined cron expression - default_schedule CronExpression + default_schedule &CronExpression // Base image to use for targets without defined base image default_base_image string mut: @@ -44,9 +44,9 @@ mut: } // new_job_queue initializes a new job queue -pub fn new_job_queue(default_schedule CronExpression, default_base_image string) BuildJobQueue { +pub fn new_job_queue(default_schedule &CronExpression, default_base_image string) BuildJobQueue { return BuildJobQueue{ - default_schedule: default_schedule + default_schedule: unsafe { default_schedule } default_base_image: default_base_image invalidated: map[int]time.Time{} } diff --git a/src/cron/expression/c/expression.c b/src/cron/expression/c/expression.c index 3f65b6a..c990b4f 100644 --- a/src/cron/expression/c/expression.c +++ b/src/cron/expression/c/expression.c @@ -1,8 +1,21 @@ #include "expression.h" +#include const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}; -int ce_next(SimpleTime *out, CronExpression *ce, SimpleTime *ref) { +struct cron_expression *ce_init() { + return malloc(sizeof(struct cron_expression)); +} + +void ce_free(struct cron_expression *ce) { + free(ce->months); + free(ce->days); + free(ce->hours); + free(ce->minutes); + free(ce); +} + +int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cron_simple_time *ref) { // For all of these values, the rule is the following: if their value is // the length of their respective array in the CronExpression object, that // means we've looped back around. This means that the "bigger" value has @@ -26,12 +39,12 @@ int ce_next(SimpleTime *out, CronExpression *ce, SimpleTime *ref) { day_index++; } - if (day_index < ce->days_count && ref->day == ce->days[day_index]) { + if (day_index < ce->day_count && ref->day == ce->days[day_index]) { while (hour_index < ce->hour_count && ref->hour > ce->hours[hour_index]) { hour_index++; } - if (hour_index < ce->hours_count && ref->hour == ce->hours[hour_index]) { + if (hour_index < ce->hour_count && ref->hour == ce->hours[hour_index]) { // Minute is the only value where we explicitely make sure we // can't match sref's value exactly. This is to ensure we only // return values in the future. @@ -88,3 +101,20 @@ int ce_next(SimpleTime *out, CronExpression *ce, SimpleTime *ref) { return 0; } + +int ce_next_from_now(struct cron_simple_time *out, struct cron_expression *ce) { + time_t t = time(NULL); + struct tm gm; + gmtime_r(&t, &gm); + + struct cron_simple_time ref = { + .year = gm.tm_year, + // tm_mon goes from 0 to 11 + .month = gm.tm_mon + 1, + .day = gm.tm_mday, + .hour = gm.tm_hour, + .minute = gm.tm_min + }; + + return ce_next(out, ce, &ref); +} diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h index 7abb189..91db5d0 100644 --- a/src/cron/expression/c/expression.h +++ b/src/cron/expression/c/expression.h @@ -3,14 +3,14 @@ #include #include -typedef enum parse_error { - ParseOk = 0, - ParseInvalidExpression = 1, - ParseInvalidNumber = 2, - ParseOutOfRange = 3 -} ParseError; +enum cron_parse_error { + CPEParseOk = 0, + CPEParseInvalidExpression = 1, + CPEParseInvalidNumber = 2, + CPEParseOutOfRange = 3 +}; -typedef struct cron_expression { +struct cron_expression { uint8_t *minutes; uint8_t *hours; uint8_t *days; @@ -19,19 +19,25 @@ typedef struct cron_expression { uint8_t hour_count; uint8_t day_count; uint8_t month_count; -} CronExpression; +}; -typedef struct simple_time { +struct cron_simple_time { int year; int month; int day; int hour; int minute; -} SimpleTime; +}; + +struct cron_expression *ce_init(); + +void cron_ce_free(struct cron_expression *ce); /** * Given a */ -int ce_next(SimpleTime *out, CronExpression *ce, SimpleTime *ref); +int cron_ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct ce_simple_time *ref); -ParseError ce_parse_expression(CronExpression *out, char *s); +int cron_ce_next_from_now(struct simple_time *out, struct cron_expression *ce); + +enum cron_parse_error cron_ce_parse_expression(struct cron_expression *out, char *s); diff --git a/src/cron/expression/c/parse.c b/src/cron/expression/c/parse.c index cb97373..b49b5dd 100644 --- a/src/cron/expression/c/parse.c +++ b/src/cron/expression/c/parse.c @@ -6,10 +6,10 @@ const uint8_t max[4] = {59, 23, 31, 12}; #define SAFE_ATOI(v,s,min,max) \ int _##v = atoi(s); \ if ((_##v) == 0 && strcmp((s), "0") != 0) { \ - return ParseInvalidNumber; \ + return CPEParseInvalidNumber; \ } \ if (v < (min) || v > (max)) { \ - return ParseOutOfRange; \ + return CPEParseOutOfRange; \ } \ v = (uint8_t) (_##v); @@ -29,17 +29,17 @@ const uint8_t max[4] = {59, 23, 31, 12}; * - a/c * - a-b/c */ -ParseError ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { +enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { // The * expression means "every possible value" if (s[0] == '*') { // A '*' is only valid on its own if (s[1] != '\0') { - return ParseInvalidExpression; + return CPEParseInvalidExpression; } *out = ~0; - return ParseOk; + return CPEParseOk; } size_t slash_index = 0; @@ -88,20 +88,20 @@ ParseError ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { } } - return ParseOk; + return CPEParseOk; } -ParseError ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { +enum cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { *out = 0; char *next; - ParseError res; + enum cron_parse_error res; while ((next = strchr(s, ',')) != NULL) { next[0] = '\0'; res = ce_parse_range(out, s, min, max); - if (res != ParseOk) { + if (res != CPEParseOk) { return res; } @@ -111,11 +111,11 @@ ParseError ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { // Make sure to parse the final range as well res = ce_parse_range(out, s, min, max); - if (res != ParseOk) { + if (res != CPEParseOk) { return res; } - return ParseOk; + return CPEParseOk; } uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { @@ -147,11 +147,14 @@ uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { return size; } -ParseError ce_parse_expression(CronExpression *out, char *s) { +enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) { + // The parsing functions modify the input string in-place + s = strdup(s); + uint8_t part_count = 0; char *next; - ParseError res; + enum cron_parse_error res; uint64_t bfs[4]; // Skip leading spaces @@ -159,11 +162,11 @@ ParseError ce_parse_expression(CronExpression *out, char *s) { s++; } - while (part_count < 4 && (next = strchr(s, ' ')) != NULL) { + while (part_count < 4 && ((next = strchr(s, ' ')) != NULL)) { next[0] = '\0'; res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - if (res != ParseOk) { + if (res != CPEParseOk) { return res; } @@ -184,7 +187,7 @@ ParseError ce_parse_expression(CronExpression *out, char *s) { // Make sure to parse the final range as well res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - if (res != ParseOk) { + if (res != CPEParseOk) { return res; } @@ -193,7 +196,7 @@ ParseError ce_parse_expression(CronExpression *out, char *s) { // At least two parts need to be provided if (part_count < 2) { - return ParseInvalidExpression; + return CPEParseInvalidExpression; } // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts @@ -208,5 +211,5 @@ ParseError ce_parse_expression(CronExpression *out, char *s) { out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); - return ParseOk; + return CPEParseOk; } diff --git a/src/cron/expression/expression.c.v b/src/cron/expression/expression.c.v index ec39fd6..fc97176 100644 --- a/src/cron/expression/expression.c.v +++ b/src/cron/expression/expression.c.v @@ -2,17 +2,36 @@ module expression #flag -I @VMODROOT/c #flag @VMODROOT/c/parse.o +#flag @VMODROOT/c/expression.o #include "expression.h" -pub struct C.CronExpression { - minutes &u8 - hours &u8 - days &u8 - months &u8 +pub struct C.cron_expression { + minutes &u8 + hours &u8 + days &u8 + months &u8 minute_count u8 - hour_count u8 - day_count u8 - month_count u8 + hour_count u8 + day_count u8 + month_count u8 } -/* pub type CronExpression = C.CronExpression */ +pub type CronExpression = C.cron_expression + +struct C.cron_simple_time { + year int + month int + day int + hour int + minute int +} + +fn C.ce_init() &C.cron_expression + +fn C.ce_free(ce &C.cron_expression) + +fn C.ce_next(out &C.cron_simple_time, ce &C.cron_expression, ref &C.cron_simple_time) int + +fn C.ce_next_from_now(out &C.cron_simple_time, ce &C.cron_expression) int + +fn C.ce_parse_expression(out &C.cron_expression, s &char) int diff --git a/src/cron/expression/expression.v b/src/cron/expression/expression.v index c3ff8c5..a51f562 100644 --- a/src/cron/expression/expression.v +++ b/src/cron/expression/expression.v @@ -2,123 +2,61 @@ module expression import time -pub struct CronExpression { - minutes []int - hours []int - days []int - months []int +pub fn parse_expression(exp string) !&CronExpression { + out := C.ce_init() + res := C.ce_parse_expression(out, exp.str) + + if res != 0 { + return error('yuhh') + } + + return out +} + +pub fn (ce &CronExpression) free() { + C.ce_free(ce) } -// next calculates the earliest time this cron expression is valid. It will -// always pick a moment in the future, even if ref matches completely up to the -// minute. This function conciously does not take gap years into account. pub fn (ce &CronExpression) next(ref time.Time) !time.Time { - // If the given ref matches the next cron occurence up to the minute, it - // will return that value. Because we always want to return a value in the - // future, we artifically shift the ref 60 seconds to make sure we always - // match in the future. A shift of 60 seconds is enough because the cron - // expression does not allow for accuracy smaller than one minute. - sref := ref - - // For all of these values, the rule is the following: if their value is - // the length of their respective array in the CronExpression object, that - // means we've looped back around. This means that the "bigger" value has - // to be incremented by one. For example, if the minutes have looped - // around, that means that the hour has to be incremented as well. - mut minute_index := 0 - mut hour_index := 0 - mut day_index := 0 - mut month_index := 0 - - // This chain is the same logic multiple times, namely that if a "bigger" - // value loops around, then the smaller value will always reset as well. - // For example, if we're going to a new day, the hour & minute will always - // be their smallest value again. - for month_index < ce.months.len && sref.month > ce.months[month_index] { - month_index++ + st := C.cron_simple_time{ + year: ref.year + month: ref.month + day: ref.day + hour: ref.hour + minute: ref.minute } - if month_index < ce.months.len && sref.month == ce.months[month_index] { - for day_index < ce.days.len && sref.day > ce.days[day_index] { - day_index++ - } + out := C.cron_simple_time{} + res := C.ce_next(&out, ce, &st) - if day_index < ce.days.len && ce.days[day_index] == sref.day { - for hour_index < ce.hours.len && sref.hour > ce.hours[hour_index] { - hour_index++ - } - - if hour_index < ce.hours.len && ce.hours[hour_index] == sref.hour { - // Minute is the only value where we explicitely make sure we - // can't match sref's value exactly. This is to ensure we only - // return values in the future. - for minute_index < ce.minutes.len && sref.minute >= ce.minutes[minute_index] { - minute_index++ - } - } - } - } - - // Here, we increment the "bigger" values by one if the smaller ones loop - // around. The order is important, as it allows a sort-of waterfall effect - // to occur which updates all values if required. - if minute_index == ce.minutes.len && hour_index < ce.hours.len { - hour_index += 1 - } - - if hour_index == ce.hours.len && day_index < ce.days.len { - day_index += 1 - } - - if day_index == ce.days.len && month_index < ce.months.len { - month_index += 1 - } - - mut minute := ce.minutes[minute_index % ce.minutes.len] - mut hour := ce.hours[hour_index % ce.hours.len] - mut day := ce.days[day_index % ce.days.len] - - // Sometimes, we end up with a day that does not exist within the selected - // month, e.g. day 30 in February. When this occurs, we reset day back to - // the smallest value & loop over to the next month that does have this - // day. - if day > time.month_days[ce.months[month_index % ce.months.len] - 1] { - day = ce.days[0] - month_index += 1 - - for day > time.month_days[ce.months[month_index & ce.months.len] - 1] { - month_index += 1 - - // If for whatever reason the day value ends up being something - // that can't be scheduled in any month, we have to make sure we - // don't create an infinite loop. - if month_index == 2 * ce.months.len { - return error('No schedulable moment.') - } - } - } - - month := ce.months[month_index % ce.months.len] - mut year := sref.year - - // If the month loops over, we need to increment the year. - if month_index >= ce.months.len { - year++ + if res != 0 { + return error('yuhh') } return time.new_time(time.Time{ - year: year - month: month - day: day - minute: minute - hour: hour + year: out.year + month: out.month + day: out.day + hour: out.hour + minute: out.minute }) } -// next_from_now returns the result of ce.next(ref) where ref is the result of -// time.now(). pub fn (ce &CronExpression) next_from_now() !time.Time { - return ce.next(time.now()) + out := C.cron_simple_time{} + res := C.ce_next_from_now(&out, ce) + + if res != 0 { + return error('yuhh') + } + + return time.new_time(time.Time{ + year: out.year + month: out.month + day: out.day + hour: out.hour + minute: out.minute + }) } // next_n returns the n next occurences of the expression, given a starting diff --git a/src/cron/expression/expression_parse.v b/src/cron/expression/expression_parse.v deleted file mode 100644 index 4aaec5b..0000000 --- a/src/cron/expression/expression_parse.v +++ /dev/null @@ -1,146 +0,0 @@ -module expression - -import bitfield - -// parse_range parses a given string into a range of sorted integers. Its -// result is a BitField with set bits for all numbers in the result. -fn parse_range(s string, min int, max int) !bitfield.BitField { - mut start := min - mut end := max - mut interval := 1 - mut bf := bitfield.new(max - min + 1) - - exps := s.split('/') - - if exps.len > 2 { - return error('Invalid expression.') - } - - if exps[0] != '*' { - dash_parts := exps[0].split('-') - - if dash_parts.len > 2 { - return error('Invalid expression.') - } - - start = dash_parts[0].int() - - // The builtin parsing functions return zero if the string can't be - // parsed into a number, so we have to explicitely check whether they - // actually entered zero or if it's an invalid number. - if start == 0 && dash_parts[0] != '0' { - return error('Invalid number.') - } - - // Check whether the start value is out of range - if start < min || start > max { - return error('Out of range.') - } - - if dash_parts.len == 2 { - end = dash_parts[1].int() - - if end == 0 && dash_parts[1] != '0' { - return error('Invalid number.') - } - - if end < start || end > max { - return error('Out of range.') - } - } - } - - if exps.len > 1 { - interval = exps[1].int() - - // interval being zero is always invalid, but we want to check why - // it's invalid for better error messages. - if interval == 0 { - if exps[1] != '0' { - return error('Invalid number.') - } else { - return error('Step size zero not allowed.') - } - } - - if interval > max - min { - return error('Step size too large.') - } - } - // Here, s solely consists of a number, so that's the only value we - // should return. - else if exps[0] != '*' && !exps[0].contains('-') { - bf.set_bit(start - min) - return bf - } - - for start <= end { - bf.set_bit(start - min) - start += interval - } - - return bf -} - -// bf_to_ints takes a BitField and converts it into the expected list of actual -// integers. -fn bf_to_ints(bf bitfield.BitField, min int) []int { - mut out := []int{} - - for i in 0 .. bf.get_size() { - if bf.get_bit(i) == 1 { - out << min + i - } - } - - return out -} - -// parse_part parses a given part of a cron expression & returns the -// corresponding array of ints. -fn parse_part(s string, min int, max int) ![]int { - mut bf := bitfield.new(max - min + 1) - - for range in s.split(',') { - bf2 := parse_range(range, min, max)! - bf = bitfield.bf_or(bf, bf2) - } - - return bf_to_ints(bf, min) -} - -// parse_expression parses an entire cron expression string into a -// CronExpression object, if possible. -pub fn parse_expression(exp string) !CronExpression { - // The filter allows for multiple spaces between parts - mut parts := exp.split(' ').filter(it != '') - - if parts.len < 2 || parts.len > 4 { - return error('Expression must contain between 2 and 4 space-separated parts.') - } - - // For ease of use, we allow the user to only specify as many parts as they - // need. - for parts.len < 4 { - parts << '*' - } - - mut part_results := [][]int{} - - mins := [0, 0, 1, 1] - maxs := [59, 23, 31, 12] - - // This for loop allows us to more clearly propagate the error to the user. - for i, min in mins { - part_results << parse_part(parts[i], min, maxs[i]) or { - return error('An error occurred with part $i: $err.msg()') - } - } - - return CronExpression{ - minutes: part_results[0] - hours: part_results[1] - days: part_results[2] - months: part_results[3] - } -} diff --git a/src/cron/expression/expression_test.v b/src/cron/expression/expression_test.v index 82bf959..2b21b4b 100644 --- a/src/cron/expression/expression_test.v +++ b/src/cron/expression/expression_test.v @@ -4,6 +4,7 @@ import time { parse } fn util_test_time(exp string, t1_str string, t2_str string) ! { ce := parse_expression(exp)! + dump(ce) t1 := parse(t1_str)! t2 := parse(t2_str)! @@ -18,17 +19,18 @@ fn util_test_time(exp string, t1_str string, t2_str string) ! { fn test_next_simple() ! { // Very simple - util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! + /* util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! */ // Overlap to next day - util_test_time('0 3', '2002-01-01 03:00:00', '2002-01-02 03:00:00')! - util_test_time('0 3', '2002-01-01 04:00:00', '2002-01-02 03:00:00')! + mut exp := '0 3' + util_test_time(exp, '2002-01-01 03:00:00', '2002-01-02 03:00:00')! + util_test_time(exp, '2002-01-01 04:00:00', '2002-01-02 03:00:00')! - util_test_time('0 3/4', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! + /* util_test_time('0 3/4', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! */ - // Overlap to next month - util_test_time('0 3', '2002-11-31 04:00:00', '2002-12-01 03:00:00')! + /* // Overlap to next month */ + /* util_test_time('0 3', '2002-11-31 04:00:00', '2002-12-01 03:00:00')! */ - // Overlap to next year - util_test_time('0 3', '2002-12-31 04:00:00', '2003-01-01 03:00:00')! + /* // Overlap to next year */ + /* util_test_time('0 3', '2002-12-31 04:00:00', '2003-01-01 03:00:00')! */ } From c2e6d168e515e8069ea7e31866d8c8c6a54efca6 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 14:18:41 +0100 Subject: [PATCH 22/30] feat(cron): pass original expression tests --- src/cron/expression/c/expression.c | 4 ++-- src/cron/expression/c/parse.c | 31 +++++++++++++++--------------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/cron/expression/c/expression.c b/src/cron/expression/c/expression.c index c990b4f..f9dc534 100644 --- a/src/cron/expression/c/expression.c +++ b/src/cron/expression/c/expression.c @@ -48,7 +48,7 @@ int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cro // Minute is the only value where we explicitely make sure we // can't match sref's value exactly. This is to ensure we only // return values in the future. - while (minute_index < ce->minute_count && ref->minute > ce->minutes[minute_index]) { + while (minute_index < ce->minute_count && ref->minute >= ce->minutes[minute_index]) { minute_index++; } } @@ -91,7 +91,7 @@ int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cro } } - out->month = ce->months[month_index * ce->month_count]; + out->month = ce->months[month_index % ce->month_count]; if (month_index >= ce->month_count) { out->year = ref->year + 1; diff --git a/src/cron/expression/c/parse.c b/src/cron/expression/c/parse.c index b49b5dd..e664dd8 100644 --- a/src/cron/expression/c/parse.c +++ b/src/cron/expression/c/parse.c @@ -80,10 +80,10 @@ enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_ // Single number doesn't need to loop if (end == 0 && slash_index == 0) { - *out |= 1 << (start - min); + *out |= ((uint64_t) 1) << (start - min); } else { for (;start <= end; start += interval) { - *out |= 1 << (start - min); + *out |= ((uint64_t) 1) << (start - min); start += interval; } } @@ -109,13 +109,7 @@ enum cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t } // Make sure to parse the final range as well - res = ce_parse_range(out, s, min, max); - - if (res != CPEParseOk) { - return res; - } - - return CPEParseOk; + return ce_parse_range(out, s, min, max); } uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { @@ -125,7 +119,7 @@ uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { uint8_t *buf = malloc(capacity * sizeof(uint8_t)); for (uint8_t i = 0; i <= max - min; i++) { - if ((1 << i) & bf) { + if (((uint64_t) 1 << i) & bf) { // Resize buffer if needed if (size == capacity) { capacity *= 2; @@ -150,11 +144,12 @@ uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) { // The parsing functions modify the input string in-place s = strdup(s); + char *orig_s = s; uint8_t part_count = 0; char *next; - enum cron_parse_error res; + enum cron_parse_error res = CPEParseOk; uint64_t bfs[4]; // Skip leading spaces @@ -167,10 +162,9 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); if (res != CPEParseOk) { - return res; + goto end; } - s = next + 1; size_t offset = 1; // Skip multiple spaces @@ -188,7 +182,7 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); if (res != CPEParseOk) { - return res; + goto end; } part_count++; @@ -196,7 +190,8 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) // At least two parts need to be provided if (part_count < 2) { - return CPEParseInvalidExpression; + res = CPEParseInvalidExpression; + goto end; } // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts @@ -211,5 +206,9 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); - return CPEParseOk; +end: + // s is cloned + free(orig_s); + + return res; } From dce00bfab6bb20046fa2f8613a5c1ebbb30f1ce0 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 15:03:11 +0100 Subject: [PATCH 23/30] feat(cron): some bug fixes & formatting --- .editorconfig | 3 +- src/cron/expression/c/expression.c | 141 ++++++------ src/cron/expression/c/expression.h | 34 +-- src/cron/expression/c/parse.c | 318 ++++++++++++++------------ src/cron/expression/expression_test.v | 8 +- 5 files changed, 266 insertions(+), 238 deletions(-) diff --git a/.editorconfig b/.editorconfig index e23a3c7..e9c1e63 100644 --- a/.editorconfig +++ b/.editorconfig @@ -5,6 +5,5 @@ root = true end_of_line = lf insert_final_newline = true -[*.v] -# vfmt wants it :( +[*.{v,c,h}] indent_style = tab diff --git a/src/cron/expression/c/expression.c b/src/cron/expression/c/expression.c index f9dc534..f15e359 100644 --- a/src/cron/expression/c/expression.c +++ b/src/cron/expression/c/expression.c @@ -4,15 +4,15 @@ const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}; struct cron_expression *ce_init() { - return malloc(sizeof(struct cron_expression)); + return malloc(sizeof(struct cron_expression)); } void ce_free(struct cron_expression *ce) { - free(ce->months); - free(ce->days); - free(ce->hours); - free(ce->minutes); - free(ce); + free(ce->months); + free(ce->days); + free(ce->hours); + free(ce->minutes); + free(ce); } int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cron_simple_time *ref) { @@ -21,100 +21,101 @@ int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cro // means we've looped back around. This means that the "bigger" value has // to be incremented by one. For example, if the minutes have looped // around, that means that the hour has to be incremented as well. - uint8_t month_index = 0; - uint8_t day_index = 0; - uint8_t hour_index = 0; - uint8_t minute_index = 0; + uint8_t month_index = 0; + uint8_t day_index = 0; + uint8_t hour_index = 0; + uint8_t minute_index = 0; // This chain is the same logic multiple times, namely that if a "bigger" // value loops around, then the smaller value will always reset as well. // For example, if we're going to a new day, the hour & minute will always // be their smallest value again. - while (month_index < ce->month_count && ref->month > ce->months[month_index]) { - month_index++; - } + while (month_index < ce->month_count && ref->month > ce->months[month_index]) { + month_index++; + } - if (month_index < ce->month_count && ref->month == ce->months[month_index]) { - while (day_index < ce->day_count && ref->day > ce->days[day_index]) { - day_index++; - } + if (month_index < ce->month_count && ref->month == ce->months[month_index]) { + while (day_index < ce->day_count && ref->day > ce->days[day_index]) { + day_index++; + } - if (day_index < ce->day_count && ref->day == ce->days[day_index]) { - while (hour_index < ce->hour_count && ref->hour > ce->hours[hour_index]) { - hour_index++; - } + if (day_index < ce->day_count && ref->day == ce->days[day_index]) { + while (hour_index < ce->hour_count && ref->hour > ce->hours[hour_index]) { + hour_index++; + } - if (hour_index < ce->hour_count && ref->hour == ce->hours[hour_index]) { + if (hour_index < ce->hour_count && ref->hour == ce->hours[hour_index]) { // Minute is the only value where we explicitely make sure we // can't match sref's value exactly. This is to ensure we only // return values in the future. - while (minute_index < ce->minute_count && ref->minute >= ce->minutes[minute_index]) { - minute_index++; - } - } - } - } + while (minute_index < ce->minute_count && ref->minute >= ce->minutes[minute_index]) { + minute_index++; + } + } + } + } // Here, we increment the "bigger" values by one if the smaller ones loop // around. The order is important, as it allows a sort-of waterfall effect // to occur which updates all values if required. - if (minute_index == ce->minute_count && hour_index < ce->hour_count) { - hour_index++; - } + if (minute_index == ce->minute_count && hour_index < ce->hour_count) { + hour_index++; + } - if (hour_index == ce->hour_count && day_index < ce->day_count) { - day_index++; - } + if (hour_index == ce->hour_count && day_index < ce->day_count) { + day_index++; + } - if (day_index == ce->day_count && month_index < ce->month_count) { - month_index++; - } + if (day_index == ce->day_count && month_index < ce->month_count) { + month_index++; + } - out->minute = ce->minutes[minute_index % ce->minute_count]; - out->hour = ce->hours[hour_index % ce->hour_count]; - out->day = ce->days[day_index % ce->day_count]; + out->minute = ce->minutes[minute_index % ce->minute_count]; + out->hour = ce->hours[hour_index % ce->hour_count]; + out->day = ce->days[day_index % ce->day_count]; // Sometimes, we end up with a day that does not exist within the selected // month, e.g. day 30 in February. When this occurs, we reset day back to // the smallest value & loop over to the next month that does have this // day. - if (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { - out->day = ce->days[0]; - month_index++; + if (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { + out->day = ce->days[0]; + month_index++; - while (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { - month_index++; - - if (month_index == 2 * ce->month_count) { - return 1; - } - } - } + while (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { + month_index++; - out->month = ce->months[month_index % ce->month_count]; + // TODO find out if this can happen + if (month_index == 2 * ce->month_count) { + return 1; + } + } + } - if (month_index >= ce->month_count) { - out->year = ref->year + 1; - } else { - out->year = ref->year; - } + out->month = ce->months[month_index % ce->month_count]; - return 0; + if (month_index >= ce->month_count) { + out->year = ref->year + 1; + } else { + out->year = ref->year; + } + + return 0; } int ce_next_from_now(struct cron_simple_time *out, struct cron_expression *ce) { - time_t t = time(NULL); - struct tm gm; - gmtime_r(&t, &gm); + time_t t = time(NULL); + struct tm gm; + gmtime_r(&t, &gm); - struct cron_simple_time ref = { - .year = gm.tm_year, - // tm_mon goes from 0 to 11 - .month = gm.tm_mon + 1, - .day = gm.tm_mday, - .hour = gm.tm_hour, - .minute = gm.tm_min - }; + struct cron_simple_time ref = { + .year = gm.tm_year, + // tm_mon goes from 0 to 11 + .month = gm.tm_mon + 1, + .day = gm.tm_mday, + .hour = gm.tm_hour, + .minute = gm.tm_min + }; - return ce_next(out, ce, &ref); + return ce_next(out, ce, &ref); } diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h index 91db5d0..f6d8826 100644 --- a/src/cron/expression/c/expression.h +++ b/src/cron/expression/c/expression.h @@ -4,29 +4,29 @@ #include enum cron_parse_error { - CPEParseOk = 0, - CPEParseInvalidExpression = 1, - CPEParseInvalidNumber = 2, - CPEParseOutOfRange = 3 + CPEParseOk = 0, + CPEParseInvalidExpression = 1, + CPEParseInvalidNumber = 2, + CPEParseOutOfRange = 3 }; struct cron_expression { - uint8_t *minutes; - uint8_t *hours; - uint8_t *days; - uint8_t *months; - uint8_t minute_count; - uint8_t hour_count; - uint8_t day_count; - uint8_t month_count; + uint8_t *minutes; + uint8_t *hours; + uint8_t *days; + uint8_t *months; + uint8_t minute_count; + uint8_t hour_count; + uint8_t day_count; + uint8_t month_count; }; struct cron_simple_time { - int year; - int month; - int day; - int hour; - int minute; + int year; + int month; + int day; + int hour; + int minute; }; struct cron_expression *ce_init(); diff --git a/src/cron/expression/c/parse.c b/src/cron/expression/c/parse.c index e664dd8..cd23458 100644 --- a/src/cron/expression/c/parse.c +++ b/src/cron/expression/c/parse.c @@ -1,25 +1,28 @@ #include "expression.h" +// Allowed value ranges for the minute, hour, day and month field const uint8_t min[4] = {0, 0, 1, 1}; const uint8_t max[4] = {59, 23, 31, 12}; +// Convert a string a uint8_t value by parsing it using atoi and checking +// whether it's contained within the given range #define SAFE_ATOI(v,s,min,max) \ - int _##v = atoi(s); \ - if ((_##v) == 0 && strcmp((s), "0") != 0) { \ - return CPEParseInvalidNumber; \ - } \ - if (v < (min) || v > (max)) { \ - return CPEParseOutOfRange; \ - } \ - v = (uint8_t) (_##v); + int _##v = atoi(s); \ + if ((_##v) == 0 && strcmp((s), "0") != 0) { \ + return CPEParseInvalidNumber; \ + } \ + if (v < (min) || v > (max)) { \ + return CPEParseOutOfRange; \ + } \ + v = (uint8_t) (_##v); /** * Given a range expression, produce a bit field defining what numbers in the - * min-max range the expression represents. The first bit (starting from the - * right) corresponds to min, the max - min + 1'th bit to max. All trailing bits + * min-max range the expression represents. Bit 0 (starting from the + * right) corresponds to min, the bit max - min to max. All trailing bits * after this should be ignored. The given bitfield is modified in-place, so * multiple calls of this function can be performed on the same value to create - * the effect of ORing their values: + * the effect of ORing their values. * * A range expression has one of the following forms: * @@ -30,185 +33,210 @@ const uint8_t max[4] = {59, 23, 31, 12}; * - a-b/c */ enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { - // The * expression means "every possible value" - if (s[0] == '*') { - // A '*' is only valid on its own - if (s[1] != '\0') { - return CPEParseInvalidExpression; - } + // The * expression means "every possible value" + if (s[0] == '*') { + // A '*' is only valid on its own + if (s[1] != '\0') { + return CPEParseInvalidExpression; + } - *out = ~0; + *out = ~0; - return CPEParseOk; - } + return CPEParseOk; + } - size_t slash_index = 0; - size_t dash_index = 0; - size_t i = 0; + size_t slash_index = 0; + size_t dash_index = 0; + size_t i = 0; - // We first iterate over the string to determine whether it contains a slash - // and/or a dash. We know the dash can only be valid if it appears before - // the slash. - while (s[i] != '\0' && slash_index == 0) { - if (s[i] == '/') { - slash_index = i; + // We first iterate over the string to determine whether it contains a slash + // and/or a dash. We know the dash can only be valid if it appears before + // the slash. + while (s[i] != '\0' && slash_index == 0) { + if (s[i] == '/') { + slash_index = i; - s[i] = '\0'; - } else if (s[i] == '-') { - dash_index = i; + s[i] = '\0'; + } else if (s[i] == '-') { + dash_index = i; - s[i] = '\0'; - } + s[i] = '\0'; + } - i++; - } + i++; + } - // Parse the three possible numbers in the pattern - uint8_t start = 0; - uint8_t end = 0; - uint8_t interval = 1; + // Parse the three possible numbers in the pattern + uint8_t start = 0; + uint8_t end = max; + uint8_t interval = 1; - SAFE_ATOI(start, s, min, max); + SAFE_ATOI(start, s, min, max); - if (dash_index > 0) { - SAFE_ATOI(end, &s[dash_index + 1], min, max); - } + if (dash_index > 0) { + SAFE_ATOI(end, &s[dash_index + 1], min, max); + } - if (slash_index > 0) { - SAFE_ATOI(interval, &s[slash_index + 1], 1, max - min); - } + if (slash_index > 0) { + SAFE_ATOI(interval, &s[slash_index + 1], 1, max - min); + } - // Single number doesn't need to loop - if (end == 0 && slash_index == 0) { - *out |= ((uint64_t) 1) << (start - min); - } else { - for (;start <= end; start += interval) { - *out |= ((uint64_t) 1) << (start - min); - start += interval; - } - } + if (dash_index == 0 && slash_index == 0) { + *out |= ((uint64_t) 1) << (start - min); + } else { + while (start <= end) { + *out |= ((uint64_t) 1) << (start - min); + start += interval; + } + } - return CPEParseOk; + return CPEParseOk; } +/* + * Given an expression part, produce a bitfield defining what numbers in the + * min-max range the part represents. A part consists of one or more range + * expressions, separated by commas. + */ enum cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { - *out = 0; + *out = 0; - char *next; - enum cron_parse_error res; - - while ((next = strchr(s, ',')) != NULL) { - next[0] = '\0'; - res = ce_parse_range(out, s, min, max); + char *next; + enum cron_parse_error res; - if (res != CPEParseOk) { - return res; - } + while ((next = strchr(s, ',')) != NULL) { + next[0] = '\0'; + res = ce_parse_range(out, s, min, max); - s = next + 1; - } + if (res != CPEParseOk) { + return res; + } - // Make sure to parse the final range as well - return ce_parse_range(out, s, min, max); + s = next + 1; + } + + // Make sure to parse the final range as well + return ce_parse_range(out, s, min, max); } +/* + * Return how many bits are set in the bitfield, better known as popcount. I + * added my own implementation (taken from my algorithms course) as I don't want + * to be dependent on GCC-specific extensions. + */ +uint8_t uint64_t_popcount(uint64_t n) { + uint8_t c = 0; + + while (n != 0) { + // This sets the least significant bit to zero (very cool) + n &= n - 1; + c++; + } + + return c; +} + +/* + * Convert a bitfield into an array containing the numbers in the min-max range + * it represents. + */ uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { - uint8_t capacity = 8; - uint8_t size = 0; + // Each bit field only has `max - min + 1` meaningful bits. All other bits + // should be ignored, and can be any value. By shifting the bit field back and + // forth, we set these excessive bits to zero, ensuring popcount returns the + // correct value. + uint8_t excess_bits = 64 - (max - min + 1); + bf = (bf << excess_bits) >> excess_bits; + uint8_t size = uint64_t_popcount(bf); + uint8_t *buf = malloc(size * sizeof(uint8_t)); - uint8_t *buf = malloc(capacity * sizeof(uint8_t)); + uint8_t i = 0, j = 0; - for (uint8_t i = 0; i <= max - min; i++) { - if (((uint64_t) 1 << i) & bf) { - // Resize buffer if needed - if (size == capacity) { - capacity *= 2; - buf = realloc(buf, capacity * sizeof(uint8_t)); - } + while (j < size && i <= max - min) { + if (((uint64_t)1 << i) & bf) { + // Resize buffer if needed + buf[j] = min + i; + j++; + } - buf[size] = min + i; - size++; - } - } + i++; + } - // Resize buffer once more to remove any trailing unused bytes - if (size < capacity) { - buf = realloc(buf, size * sizeof(uint8_t)); - } + *out = buf; - *out = buf; - - return size; + return size; } +/* + * Parse a cron expression string into a cron_expression struct. + */ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) { - // The parsing functions modify the input string in-place - s = strdup(s); - char *orig_s = s; + // The parsing functions modify the input string in-place + s = strdup(s); + char *orig_s = s; - uint8_t part_count = 0; + uint8_t part_count = 0; - char *next; - enum cron_parse_error res = CPEParseOk; - uint64_t bfs[4]; + char *next; + enum cron_parse_error res = CPEParseOk; + uint64_t bfs[4]; - // Skip leading spaces - while (s[0] == ' ') { - s++; - } - - while (part_count < 4 && ((next = strchr(s, ' ')) != NULL)) { - next[0] = '\0'; - res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); + // Skip leading spaces + while (s[0] == ' ') { + s++; + } - if (res != CPEParseOk) { - goto end; - } + while (part_count < 4 && ((next = strchr(s, ' ')) != NULL)) { + next[0] = '\0'; + res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - size_t offset = 1; + if (res != CPEParseOk) { + goto end; + } - // Skip multiple spaces - while (next[offset] == ' ') { - offset++; - } - s = next + offset; + size_t offset = 1; - part_count++; - } + // Skip multiple spaces + while (next[offset] == ' ') { + offset++; + } + s = next + offset; - // Parse final trailing part - if (part_count < 4 && s[0] != '\0') { - // Make sure to parse the final range as well - res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); + part_count++; + } - if (res != CPEParseOk) { - goto end; - } + // Parse final trailing part + if (part_count < 4 && s[0] != '\0') { + res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - part_count++; - } + if (res != CPEParseOk) { + goto end; + } - // At least two parts need to be provided - if (part_count < 2) { - res = CPEParseInvalidExpression; - goto end; - } + part_count++; + } - // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts - while (part_count < 4) { - // Expression is augmented with '*' expressions - bfs[part_count] = ~0; - part_count++; - } + // At least two parts need to be provided + if (part_count < 2) { + res = CPEParseInvalidExpression; + goto end; + } - out->minute_count = bf_to_nums(&out->minutes, bfs[0], min[0], max[0]); - out->hour_count = bf_to_nums(&out->hours, bfs[1], min[1], max[1]); - out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); - out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); + // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts + while (part_count < 4) { + // Expression is augmented with '*' expressions + bfs[part_count] = ~0; + part_count++; + } + + out->minute_count = bf_to_nums(&out->minutes, bfs[0], min[0], max[0]); + out->hour_count = bf_to_nums(&out->hours, bfs[1], min[1], max[1]); + out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); + out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); end: - // s is cloned - free(orig_s); + // s is cloned + free(orig_s); - return res; + return res; } diff --git a/src/cron/expression/expression_test.v b/src/cron/expression/expression_test.v index 2b21b4b..448927a 100644 --- a/src/cron/expression/expression_test.v +++ b/src/cron/expression/expression_test.v @@ -22,15 +22,15 @@ fn test_next_simple() ! { /* util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! */ // Overlap to next day - mut exp := '0 3' + mut exp := '0 3 ' util_test_time(exp, '2002-01-01 03:00:00', '2002-01-02 03:00:00')! util_test_time(exp, '2002-01-01 04:00:00', '2002-01-02 03:00:00')! - /* util_test_time('0 3/4', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! */ + util_test_time('0 3/4', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! /* // Overlap to next month */ - /* util_test_time('0 3', '2002-11-31 04:00:00', '2002-12-01 03:00:00')! */ + util_test_time('0 3', '2002-11-31 04:00:00', '2002-12-01 03:00:00')! /* // Overlap to next year */ - /* util_test_time('0 3', '2002-12-31 04:00:00', '2003-01-01 03:00:00')! */ + util_test_time('0 3', '2002-12-31 04:00:00', '2003-01-01 03:00:00')! } From 3f1aea13e2aa4b4f6a7d30bafc5014655677a1af Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 16:52:30 +0100 Subject: [PATCH 24/30] refactor: make cron.expression into cron module --- CHANGELOG.md | 4 ++ src/build/queue.v | 10 ++--- src/console/schedule/schedule.v | 4 +- src/console/targets/targets.v | 4 +- src/cron/{expression => }/c/expression.c | 10 ++--- src/cron/c/expression.h | 40 +++++++++++++++++ src/cron/{expression => }/c/parse.c | 22 +++++----- src/cron/{expression => }/expression.c.v | 6 ++- src/cron/{expression => }/expression.v | 18 ++++---- src/cron/expression/c/expression.h | 43 ------------------- .../{expression => }/expression_parse_test.v | 2 +- src/cron/{expression => }/expression_test.v | 8 ++-- src/cron/{expression => }/v.mod | 0 src/server/log_removal.v | 4 +- src/server/server.v | 6 +-- 15 files changed, 92 insertions(+), 89 deletions(-) rename src/cron/{expression => }/c/expression.c (91%) create mode 100644 src/cron/c/expression.h rename src/cron/{expression => }/c/parse.c (92%) rename src/cron/{expression => }/expression.c.v (88%) rename src/cron/{expression => }/expression.v (69%) delete mode 100644 src/cron/expression/c/expression.h rename src/cron/{expression => }/expression_parse_test.v (99%) rename src/cron/{expression => }/expression_test.v (83%) rename src/cron/{expression => }/v.mod (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index be5f445..6b1e583 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Search in list of targets using API & CLI * Allow filtering targets by arch value +### Changed + +* Rewrote cron expression logic in C + ## [0.5.0](https://git.rustybever.be/vieter-v/vieter/src/tag/0.5.0) ### Added diff --git a/src/build/queue.v b/src/build/queue.v index e87024b..122180e 100644 --- a/src/build/queue.v +++ b/src/build/queue.v @@ -1,7 +1,7 @@ module build import models { BuildConfig, Target } -import cron.expression { CronExpression, parse_expression } +import cron import time import datatypes { MinHeap } import util @@ -13,7 +13,7 @@ pub mut: // Next timestamp from which point this job is allowed to be executed timestamp time.Time // Required for calculating next timestamp after having pop'ed a job - ce &CronExpression = unsafe { nil } + ce &cron.Expression = unsafe { nil } // Actual build config sent to the agent config BuildConfig // Whether this is a one-time job @@ -30,7 +30,7 @@ fn (r1 BuildJob) < (r2 BuildJob) bool { // for each architecture. Agents receive jobs from this queue. pub struct BuildJobQueue { // Schedule to use for targets without explicitely defined cron expression - default_schedule &CronExpression + default_schedule &cron.Expression // Base image to use for targets without defined base image default_base_image string mut: @@ -44,7 +44,7 @@ mut: } // new_job_queue initializes a new job queue -pub fn new_job_queue(default_schedule &CronExpression, default_base_image string) BuildJobQueue { +pub fn new_job_queue(default_schedule &cron.Expression, default_base_image string) BuildJobQueue { return BuildJobQueue{ default_schedule: unsafe { default_schedule } default_base_image: default_base_image @@ -85,7 +85,7 @@ pub fn (mut q BuildJobQueue) insert(input InsertConfig) ! { if !input.now { ce := if input.target.schedule != '' { - parse_expression(input.target.schedule) or { + cron.parse_expression(input.target.schedule) or { return error("Error while parsing cron expression '$input.target.schedule' (id $input.target.id): $err.msg()") } } else { diff --git a/src/console/schedule/schedule.v b/src/console/schedule/schedule.v index 7ce0516..40b300f 100644 --- a/src/console/schedule/schedule.v +++ b/src/console/schedule/schedule.v @@ -1,7 +1,7 @@ module schedule import cli -import cron.expression { parse_expression } +import cron import time // cmd returns the cli submodule for previewing a cron schedule. @@ -19,7 +19,7 @@ pub fn cmd() cli.Command { }, ] execute: fn (cmd cli.Command) ! { - ce := parse_expression(cmd.args.join(' '))! + ce := cron.parse_expression(cmd.args.join(' '))! count := cmd.flags.get_int('count')! for t in ce.next_n(time.now(), count)! { diff --git a/src/console/targets/targets.v b/src/console/targets/targets.v index 6152a53..709c196 100644 --- a/src/console/targets/targets.v +++ b/src/console/targets/targets.v @@ -2,7 +2,7 @@ module targets import cli import conf as vconf -import cron.expression { parse_expression } +import cron import client { NewTarget } import console import models { TargetFilter } @@ -295,7 +295,7 @@ fn patch(conf Config, id string, params map[string]string) ! { // We check the cron expression first because it's useless to send an // invalid one to the server. if 'schedule' in params && params['schedule'] != '' { - parse_expression(params['schedule']) or { + cron.parse_expression(params['schedule']) or { return error('Invalid cron expression: $err.msg()') } } diff --git a/src/cron/expression/c/expression.c b/src/cron/c/expression.c similarity index 91% rename from src/cron/expression/c/expression.c rename to src/cron/c/expression.c index f15e359..ed0306f 100644 --- a/src/cron/expression/c/expression.c +++ b/src/cron/c/expression.c @@ -3,11 +3,11 @@ const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}; -struct cron_expression *ce_init() { - return malloc(sizeof(struct cron_expression)); +cron_expression *ce_init() { + return malloc(sizeof(cron_expression)); } -void ce_free(struct cron_expression *ce) { +void ce_free(cron_expression *ce) { free(ce->months); free(ce->days); free(ce->hours); @@ -15,7 +15,7 @@ void ce_free(struct cron_expression *ce) { free(ce); } -int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cron_simple_time *ref) { +int ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref) { // For all of these values, the rule is the following: if their value is // the length of their respective array in the CronExpression object, that // means we've looped back around. This means that the "bigger" value has @@ -103,7 +103,7 @@ int ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct cro return 0; } -int ce_next_from_now(struct cron_simple_time *out, struct cron_expression *ce) { +int ce_next_from_now(cron_simple_time *out, cron_expression *ce) { time_t t = time(NULL); struct tm gm; gmtime_r(&t, &gm); diff --git a/src/cron/c/expression.h b/src/cron/c/expression.h new file mode 100644 index 0000000..c9441f6 --- /dev/null +++ b/src/cron/c/expression.h @@ -0,0 +1,40 @@ +#include +#include +#include +#include + +typedef enum cron_parse_error { + cron_parse_ok = 0, + cron_parse_invalid_expression = 1, + cron_parse_invalid_number = 2, + cron_parse_out_of_range = 3 +} cron_parse_error; + +typedef struct cron_expression { + uint8_t *minutes; + uint8_t *hours; + uint8_t *days; + uint8_t *months; + uint8_t minute_count; + uint8_t hour_count; + uint8_t day_count; + uint8_t month_count; +} cron_expression; + +typedef struct cron_simple_time { + int year; + int month; + int day; + int hour; + int minute; +} cron_simple_time; + +cron_expression *ce_init(); + +void cron_ce_free(cron_expression *ce); + +int cron_ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref); + +int cron_ce_next_from_now(cron_simple_time *out, cron_expression *ce); + +enum cron_parse_error cron_ce_parse_expression(cron_expression *out, char *s); diff --git a/src/cron/expression/c/parse.c b/src/cron/c/parse.c similarity index 92% rename from src/cron/expression/c/parse.c rename to src/cron/c/parse.c index cd23458..a577c6d 100644 --- a/src/cron/expression/c/parse.c +++ b/src/cron/c/parse.c @@ -9,10 +9,10 @@ const uint8_t max[4] = {59, 23, 31, 12}; #define SAFE_ATOI(v,s,min,max) \ int _##v = atoi(s); \ if ((_##v) == 0 && strcmp((s), "0") != 0) { \ - return CPEParseInvalidNumber; \ + return cron_parse_invalid_number; \ } \ if (v < (min) || v > (max)) { \ - return CPEParseOutOfRange; \ + return cron_parse_out_of_range; \ } \ v = (uint8_t) (_##v); @@ -37,12 +37,12 @@ enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_ if (s[0] == '*') { // A '*' is only valid on its own if (s[1] != '\0') { - return CPEParseInvalidExpression; + return cron_parse_invalid_expression; } *out = ~0; - return CPEParseOk; + return cron_parse_ok; } size_t slash_index = 0; @@ -90,7 +90,7 @@ enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_ } } - return CPEParseOk; + return cron_parse_ok; } /* @@ -108,7 +108,7 @@ enum cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t next[0] = '\0'; res = ce_parse_range(out, s, min, max); - if (res != CPEParseOk) { + if (res != cron_parse_ok) { return res; } @@ -170,7 +170,7 @@ uint8_t bf_to_nums(uint8_t **out, uint64_t bf, uint8_t min, uint8_t max) { /* * Parse a cron expression string into a cron_expression struct. */ -enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) { +enum cron_parse_error ce_parse_expression(cron_expression *out, char *s) { // The parsing functions modify the input string in-place s = strdup(s); char *orig_s = s; @@ -178,7 +178,7 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) uint8_t part_count = 0; char *next; - enum cron_parse_error res = CPEParseOk; + enum cron_parse_error res = cron_parse_ok; uint64_t bfs[4]; // Skip leading spaces @@ -190,7 +190,7 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) next[0] = '\0'; res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - if (res != CPEParseOk) { + if (res != cron_parse_ok) { goto end; } @@ -209,7 +209,7 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) if (part_count < 4 && s[0] != '\0') { res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - if (res != CPEParseOk) { + if (res != cron_parse_ok) { goto end; } @@ -218,7 +218,7 @@ enum cron_parse_error ce_parse_expression(struct cron_expression *out, char *s) // At least two parts need to be provided if (part_count < 2) { - res = CPEParseInvalidExpression; + res = cron_parse_invalid_expression; goto end; } diff --git a/src/cron/expression/expression.c.v b/src/cron/expression.c.v similarity index 88% rename from src/cron/expression/expression.c.v rename to src/cron/expression.c.v index fc97176..08af25c 100644 --- a/src/cron/expression/expression.c.v +++ b/src/cron/expression.c.v @@ -1,4 +1,4 @@ -module expression +module cron #flag -I @VMODROOT/c #flag @VMODROOT/c/parse.o @@ -16,7 +16,7 @@ pub struct C.cron_expression { month_count u8 } -pub type CronExpression = C.cron_expression +pub type Expression = C.cron_expression struct C.cron_simple_time { year int @@ -26,6 +26,8 @@ struct C.cron_simple_time { minute int } +type SimpleTime = C.cron_simple_time + fn C.ce_init() &C.cron_expression fn C.ce_free(ce &C.cron_expression) diff --git a/src/cron/expression/expression.v b/src/cron/expression.v similarity index 69% rename from src/cron/expression/expression.v rename to src/cron/expression.v index a51f562..0429b93 100644 --- a/src/cron/expression/expression.v +++ b/src/cron/expression.v @@ -1,8 +1,8 @@ -module expression +module cron import time -pub fn parse_expression(exp string) !&CronExpression { +pub fn parse_expression(exp string) !&Expression { out := C.ce_init() res := C.ce_parse_expression(out, exp.str) @@ -13,12 +13,12 @@ pub fn parse_expression(exp string) !&CronExpression { return out } -pub fn (ce &CronExpression) free() { +pub fn (ce &Expression) free() { C.ce_free(ce) } -pub fn (ce &CronExpression) next(ref time.Time) !time.Time { - st := C.cron_simple_time{ +pub fn (ce &Expression) next(ref time.Time) !time.Time { + st := SimpleTime{ year: ref.year month: ref.month day: ref.day @@ -26,7 +26,7 @@ pub fn (ce &CronExpression) next(ref time.Time) !time.Time { minute: ref.minute } - out := C.cron_simple_time{} + out := SimpleTime{} res := C.ce_next(&out, ce, &st) if res != 0 { @@ -42,8 +42,8 @@ pub fn (ce &CronExpression) next(ref time.Time) !time.Time { }) } -pub fn (ce &CronExpression) next_from_now() !time.Time { - out := C.cron_simple_time{} +pub fn (ce &Expression) next_from_now() !time.Time { + out := SimpleTime{} res := C.ce_next_from_now(&out, ce) if res != 0 { @@ -61,7 +61,7 @@ pub fn (ce &CronExpression) next_from_now() !time.Time { // next_n returns the n next occurences of the expression, given a starting // time. -pub fn (ce &CronExpression) next_n(ref time.Time, n int) ![]time.Time { +pub fn (ce &Expression) next_n(ref time.Time, n int) ![]time.Time { mut times := []time.Time{cap: n} times << ce.next(ref)! diff --git a/src/cron/expression/c/expression.h b/src/cron/expression/c/expression.h deleted file mode 100644 index f6d8826..0000000 --- a/src/cron/expression/c/expression.h +++ /dev/null @@ -1,43 +0,0 @@ -#include -#include -#include -#include - -enum cron_parse_error { - CPEParseOk = 0, - CPEParseInvalidExpression = 1, - CPEParseInvalidNumber = 2, - CPEParseOutOfRange = 3 -}; - -struct cron_expression { - uint8_t *minutes; - uint8_t *hours; - uint8_t *days; - uint8_t *months; - uint8_t minute_count; - uint8_t hour_count; - uint8_t day_count; - uint8_t month_count; -}; - -struct cron_simple_time { - int year; - int month; - int day; - int hour; - int minute; -}; - -struct cron_expression *ce_init(); - -void cron_ce_free(struct cron_expression *ce); - -/** - * Given a - */ -int cron_ce_next(struct cron_simple_time *out, struct cron_expression *ce, struct ce_simple_time *ref); - -int cron_ce_next_from_now(struct simple_time *out, struct cron_expression *ce); - -enum cron_parse_error cron_ce_parse_expression(struct cron_expression *out, char *s); diff --git a/src/cron/expression/expression_parse_test.v b/src/cron/expression_parse_test.v similarity index 99% rename from src/cron/expression/expression_parse_test.v rename to src/cron/expression_parse_test.v index 92e8291..0b0b605 100644 --- a/src/cron/expression/expression_parse_test.v +++ b/src/cron/expression_parse_test.v @@ -1,4 +1,4 @@ -module expression +module cron // parse_range_error returns the returned error message. If the result is '', // that means the function didn't error. diff --git a/src/cron/expression/expression_test.v b/src/cron/expression_test.v similarity index 83% rename from src/cron/expression/expression_test.v rename to src/cron/expression_test.v index 448927a..e1a9849 100644 --- a/src/cron/expression/expression_test.v +++ b/src/cron/expression_test.v @@ -1,4 +1,4 @@ -module expression +module cron import time { parse } @@ -19,7 +19,7 @@ fn util_test_time(exp string, t1_str string, t2_str string) ! { fn test_next_simple() ! { // Very simple - /* util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! */ + // util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! // Overlap to next day mut exp := '0 3 ' @@ -28,9 +28,9 @@ fn test_next_simple() ! { util_test_time('0 3/4', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! - /* // Overlap to next month */ + //// Overlap to next month util_test_time('0 3', '2002-11-31 04:00:00', '2002-12-01 03:00:00')! - /* // Overlap to next year */ + //// Overlap to next year util_test_time('0 3', '2002-12-31 04:00:00', '2003-01-01 03:00:00')! } diff --git a/src/cron/expression/v.mod b/src/cron/v.mod similarity index 100% rename from src/cron/expression/v.mod rename to src/cron/v.mod diff --git a/src/server/log_removal.v b/src/server/log_removal.v index 8e1a8c2..98cba93 100644 --- a/src/server/log_removal.v +++ b/src/server/log_removal.v @@ -3,12 +3,12 @@ module server import time import models { BuildLog } import os -import cron.expression { CronExpression } +import cron const fallback_log_removal_frequency = 24 * time.hour // log_removal_daemon removes old build logs every `log_removal_frequency`. -fn (mut app App) log_removal_daemon(schedule CronExpression) { +fn (mut app App) log_removal_daemon(schedule cron.Expression) { mut start_time := time.Time{} for { diff --git a/src/server/server.v b/src/server/server.v index 5dd1a20..ae086f5 100644 --- a/src/server/server.v +++ b/src/server/server.v @@ -7,7 +7,7 @@ import repo import util import db import build { BuildJobQueue } -import cron.expression +import cron import metrics const ( @@ -43,11 +43,11 @@ pub fn server(conf Config) ! { util.exit_with_message(1, "'any' is not allowed as the value for default_arch.") } - global_ce := expression.parse_expression(conf.global_schedule) or { + global_ce := cron.parse_expression(conf.global_schedule) or { util.exit_with_message(1, 'Invalid global cron expression: $err.msg()') } - log_removal_ce := expression.parse_expression(conf.log_removal_schedule) or { + log_removal_ce := cron.parse_expression(conf.log_removal_schedule) or { util.exit_with_message(1, 'Invalid log removal cron expression: $err.msg()') } From 801a2cd495c558917127961b163e93b5209a562d Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 18:44:33 +0100 Subject: [PATCH 25/30] fix(cron): ensure valid day values; some other stuff --- Makefile | 2 +- src/cron/c/expression.c | 7 +- src/cron/c/expression.h | 8 ++- src/cron/c/parse.c | 130 +++++++++++++++++++++++++++---------- src/cron/expression.v | 11 ++-- src/cron/expression_test.v | 2 +- 6 files changed, 112 insertions(+), 48 deletions(-) diff --git a/Makefile b/Makefile index c71ff1f..3571d30 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ # =====CONFIG===== SRC_DIR := src -SOURCES != find '$(SRC_DIR)' -iname '*.v' +SOURCES != find '$(SRC_DIR)' -\( -iname '*.v' -or -iname '*.h' -or -iname '*.c' -\) V_PATH ?= v V := $(V_PATH) -showcc -gc boehm -W -d use_openssl -skip-unused diff --git a/src/cron/c/expression.c b/src/cron/c/expression.c index ed0306f..0051e49 100644 --- a/src/cron/c/expression.c +++ b/src/cron/c/expression.c @@ -84,11 +84,6 @@ int ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref) { while (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { month_index++; - - // TODO find out if this can happen - if (month_index == 2 * ce->month_count) { - return 1; - } } } @@ -108,7 +103,7 @@ int ce_next_from_now(cron_simple_time *out, cron_expression *ce) { struct tm gm; gmtime_r(&t, &gm); - struct cron_simple_time ref = { + cron_simple_time ref = { .year = gm.tm_year, // tm_mon goes from 0 to 11 .month = gm.tm_mon + 1, diff --git a/src/cron/c/expression.h b/src/cron/c/expression.h index c9441f6..1c86436 100644 --- a/src/cron/c/expression.h +++ b/src/cron/c/expression.h @@ -1,3 +1,6 @@ +#ifndef VIETER_CRON +#define VIETER_CRON + #include #include #include @@ -7,7 +10,8 @@ typedef enum cron_parse_error { cron_parse_ok = 0, cron_parse_invalid_expression = 1, cron_parse_invalid_number = 2, - cron_parse_out_of_range = 3 + cron_parse_out_of_range = 3, + cron_parse_too_many_parts = 4 } cron_parse_error; typedef struct cron_expression { @@ -38,3 +42,5 @@ int cron_ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *r int cron_ce_next_from_now(cron_simple_time *out, cron_expression *ce); enum cron_parse_error cron_ce_parse_expression(cron_expression *out, char *s); + +#endif diff --git a/src/cron/c/parse.c b/src/cron/c/parse.c index a577c6d..f54b818 100644 --- a/src/cron/c/parse.c +++ b/src/cron/c/parse.c @@ -1,21 +1,28 @@ #include "expression.h" +const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}; + // Allowed value ranges for the minute, hour, day and month field const uint8_t min[4] = {0, 0, 1, 1}; const uint8_t max[4] = {59, 23, 31, 12}; -// Convert a string a uint8_t value by parsing it using atoi and checking +const uint8_t min_parts = 2; +const uint8_t max_parts = 4; + +// Convert a string into a uint8_t value by parsing it using atoi and checking // whether it's contained within the given range #define SAFE_ATOI(v,s,min,max) \ int _##v = atoi(s); \ if ((_##v) == 0 && strcmp((s), "0") != 0) { \ return cron_parse_invalid_number; \ } \ - if (v < (min) || v > (max)) { \ + if (((_##v) < (min)) || ((_##v) > (max))) { \ return cron_parse_out_of_range; \ } \ v = (uint8_t) (_##v); +#define MAX(x, y) (((x) > (y)) ? (x) : (y)) + /** * Given a range expression, produce a bit field defining what numbers in the * min-max range the expression represents. Bit 0 (starting from the @@ -32,7 +39,7 @@ const uint8_t max[4] = {59, 23, 31, 12}; * - a/c * - a-b/c */ -enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { +cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { // The * expression means "every possible value" if (s[0] == '*') { // A '*' is only valid on its own @@ -98,7 +105,7 @@ enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_ * min-max range the part represents. A part consists of one or more range * expressions, separated by commas. */ -enum cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { +cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { *out = 0; char *next; @@ -175,28 +182,30 @@ enum cron_parse_error ce_parse_expression(cron_expression *out, char *s) { s = strdup(s); char *orig_s = s; - uint8_t part_count = 0; - - char *next; enum cron_parse_error res = cron_parse_ok; - uint64_t bfs[4]; + uint64_t bfs[max_parts]; + + // First we divide the input string into its parts, divided by spaces. + // Each part is delimited by a NULL byte. + uint8_t part_count = 0; + char *parts[max_parts]; + char *next; // Skip leading spaces - while (s[0] == ' ') { - s++; + size_t offset = 0; + + while (s[offset] == ' ') { + offset++; } - while (part_count < 4 && ((next = strchr(s, ' ')) != NULL)) { + s += offset; + + while (part_count < max_parts && ((next = strchr(s, ' ')) != NULL)) { next[0] = '\0'; - res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - - if (res != cron_parse_ok) { - goto end; - } - - size_t offset = 1; + parts[part_count] = s; // Skip multiple spaces + offset = 1; while (next[offset] == ' ') { offset++; } @@ -205,34 +214,87 @@ enum cron_parse_error ce_parse_expression(cron_expression *out, char *s) { part_count++; } - // Parse final trailing part - if (part_count < 4 && s[0] != '\0') { - res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); + // The loop exited because we already have 4 parts, yet there's still at + // least one more part that follows. + if (next != NULL) { + res = cron_parse_too_many_parts; + } else if (s[0] != '\0') { + // There's one more excessive trailing part + if (part_count == max_parts) { + res = cron_parse_too_many_parts; + goto end; + } + + parts[part_count] = s; + part_count++; + } + + + // We now parse the parts in reverse. This is because the month part + // determines the maximum value of the day part. + + uint64_t bit_field = 0; + + // Months + if (part_count >= 4) { + res = ce_parse_part(&bit_field, parts[3], min[3], max[3]); if (res != cron_parse_ok) { goto end; } - part_count++; + out->month_count = bf_to_nums(&out->months, bit_field, min[3], max[3]); + } + // If months aren't provided, they're replaced with a * + else { + out->month_count = bf_to_nums(&out->months, ~0, min[3], max[3]); } - // At least two parts need to be provided - if (part_count < 2) { - res = cron_parse_invalid_expression; + // Determine what the largest allowed day value is, given the months + uint8_t max_day_value = 0; + + for (uint8_t i = 0; i < out->month_count; i++) { + max_day_value = MAX(max_day_value, month_days[out->months[i] - 1]); + } + + // Days + if (part_count >= 3) { + bit_field = 0; + + res = ce_parse_part(&bit_field, parts[2], min[2], max_day_value); + + if (res != cron_parse_ok) { + goto end; + } + + out->day_count = bf_to_nums(&out->days, bit_field, min[2], max_day_value); + } + // If days aren't provided, they're replaced with a * + else { + out->day_count = bf_to_nums(&out->days, ~0, min[2], max_day_value); + } + + // Hours + bit_field = 0; + + res = ce_parse_part(&bit_field, parts[1], min[1], max[1]); + + if (res != cron_parse_ok) { goto end; } - // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts - while (part_count < 4) { - // Expression is augmented with '*' expressions - bfs[part_count] = ~0; - part_count++; + out->hour_count = bf_to_nums(&out->hours, bit_field, min[1], max[1]); + + // Minutes + bit_field = 0; + + res = ce_parse_part(&bit_field, parts[0], min[0], max[0]); + + if (res != cron_parse_ok) { + goto end; } - out->minute_count = bf_to_nums(&out->minutes, bfs[0], min[0], max[0]); - out->hour_count = bf_to_nums(&out->hours, bfs[1], min[1], max[1]); - out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); - out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); + out->minute_count = bf_to_nums(&out->minutes, bit_field, min[0], max[0]); end: // s is cloned diff --git a/src/cron/expression.v b/src/cron/expression.v index 0429b93..23e1354 100644 --- a/src/cron/expression.v +++ b/src/cron/expression.v @@ -2,21 +2,22 @@ module cron import time +[unsafe] +pub fn (ce &Expression) free() { + C.ce_free(ce) +} + pub fn parse_expression(exp string) !&Expression { out := C.ce_init() res := C.ce_parse_expression(out, exp.str) if res != 0 { - return error('yuhh') + return error(res.str()) } return out } -pub fn (ce &Expression) free() { - C.ce_free(ce) -} - pub fn (ce &Expression) next(ref time.Time) !time.Time { st := SimpleTime{ year: ref.year diff --git a/src/cron/expression_test.v b/src/cron/expression_test.v index e1a9849..d6ec002 100644 --- a/src/cron/expression_test.v +++ b/src/cron/expression_test.v @@ -22,7 +22,7 @@ fn test_next_simple() ! { // util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! // Overlap to next day - mut exp := '0 3 ' + mut exp := '0 3 ' util_test_time(exp, '2002-01-01 03:00:00', '2002-01-02 03:00:00')! util_test_time(exp, '2002-01-01 04:00:00', '2002-01-02 03:00:00')! From 86e519a185c7e12c0282d24223980c105d511435 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 18:51:01 +0100 Subject: [PATCH 26/30] refactor(cron): make next function infallible --- src/build/queue.v | 14 +++++--------- src/console/schedule/schedule.v | 2 +- src/cron/c/expression.c | 8 +++----- src/cron/c/expression.h | 4 ++-- src/cron/expression.c.v | 4 ++-- src/cron/expression.v | 22 +++++++--------------- src/cron/expression_test.v | 2 +- src/server/log_removal.v | 11 +---------- 8 files changed, 22 insertions(+), 45 deletions(-) diff --git a/src/build/queue.v b/src/build/queue.v index 122180e..abd4ec6 100644 --- a/src/build/queue.v +++ b/src/build/queue.v @@ -92,7 +92,7 @@ pub fn (mut q BuildJobQueue) insert(input InsertConfig) ! { q.default_schedule } - job.timestamp = ce.next_from_now()! + job.timestamp = ce.next_from_now() job.ce = ce } else { job.timestamp = time.now() @@ -105,8 +105,8 @@ pub fn (mut q BuildJobQueue) insert(input InsertConfig) ! { // reschedule the given job by calculating the next timestamp and re-adding it // to its respective queue. This function is called by the pop functions // *after* having pop'ed the job. -fn (mut q BuildJobQueue) reschedule(job BuildJob, arch string) ! { - new_timestamp := job.ce.next_from_now()! +fn (mut q BuildJobQueue) reschedule(job BuildJob, arch string) { + new_timestamp := job.ce.next_from_now() new_job := BuildJob{ ...job @@ -168,10 +168,7 @@ pub fn (mut q BuildJobQueue) pop(arch string) ?BuildJob { job = q.queues[arch].pop()? if !job.single { - // TODO how do we handle this properly? Is it even possible for a - // cron expression to not return a next time if it's already been - // used before? - q.reschedule(job, arch) or {} + q.reschedule(job, arch) } return job @@ -198,8 +195,7 @@ pub fn (mut q BuildJobQueue) pop_n(arch string, n int) []BuildJob { job = q.queues[arch].pop() or { break } if !job.single { - // TODO idem - q.reschedule(job, arch) or {} + q.reschedule(job, arch) } out << job diff --git a/src/console/schedule/schedule.v b/src/console/schedule/schedule.v index 40b300f..ceabf24 100644 --- a/src/console/schedule/schedule.v +++ b/src/console/schedule/schedule.v @@ -22,7 +22,7 @@ pub fn cmd() cli.Command { ce := cron.parse_expression(cmd.args.join(' '))! count := cmd.flags.get_int('count')! - for t in ce.next_n(time.now(), count)! { + for t in ce.next_n(time.now(), count) { println(t) } } diff --git a/src/cron/c/expression.c b/src/cron/c/expression.c index 0051e49..59d1bf0 100644 --- a/src/cron/c/expression.c +++ b/src/cron/c/expression.c @@ -15,7 +15,7 @@ void ce_free(cron_expression *ce) { free(ce); } -int ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref) { +void ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref) { // For all of these values, the rule is the following: if their value is // the length of their respective array in the CronExpression object, that // means we've looped back around. This means that the "bigger" value has @@ -94,11 +94,9 @@ int ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref) { } else { out->year = ref->year; } - - return 0; } -int ce_next_from_now(cron_simple_time *out, cron_expression *ce) { +void ce_next_from_now(cron_simple_time *out, cron_expression *ce) { time_t t = time(NULL); struct tm gm; gmtime_r(&t, &gm); @@ -112,5 +110,5 @@ int ce_next_from_now(cron_simple_time *out, cron_expression *ce) { .minute = gm.tm_min }; - return ce_next(out, ce, &ref); + ce_next(out, ce, &ref); } diff --git a/src/cron/c/expression.h b/src/cron/c/expression.h index 1c86436..c599e4e 100644 --- a/src/cron/c/expression.h +++ b/src/cron/c/expression.h @@ -37,9 +37,9 @@ cron_expression *ce_init(); void cron_ce_free(cron_expression *ce); -int cron_ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref); +void cron_ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref); -int cron_ce_next_from_now(cron_simple_time *out, cron_expression *ce); +void cron_ce_next_from_now(cron_simple_time *out, cron_expression *ce); enum cron_parse_error cron_ce_parse_expression(cron_expression *out, char *s); diff --git a/src/cron/expression.c.v b/src/cron/expression.c.v index 08af25c..7551e6f 100644 --- a/src/cron/expression.c.v +++ b/src/cron/expression.c.v @@ -32,8 +32,8 @@ fn C.ce_init() &C.cron_expression fn C.ce_free(ce &C.cron_expression) -fn C.ce_next(out &C.cron_simple_time, ce &C.cron_expression, ref &C.cron_simple_time) int +fn C.ce_next(out &C.cron_simple_time, ce &C.cron_expression, ref &C.cron_simple_time) -fn C.ce_next_from_now(out &C.cron_simple_time, ce &C.cron_expression) int +fn C.ce_next_from_now(out &C.cron_simple_time, ce &C.cron_expression) fn C.ce_parse_expression(out &C.cron_expression, s &char) int diff --git a/src/cron/expression.v b/src/cron/expression.v index 23e1354..c0cab8d 100644 --- a/src/cron/expression.v +++ b/src/cron/expression.v @@ -18,7 +18,7 @@ pub fn parse_expression(exp string) !&Expression { return out } -pub fn (ce &Expression) next(ref time.Time) !time.Time { +pub fn (ce &Expression) next(ref time.Time) time.Time { st := SimpleTime{ year: ref.year month: ref.month @@ -28,11 +28,7 @@ pub fn (ce &Expression) next(ref time.Time) !time.Time { } out := SimpleTime{} - res := C.ce_next(&out, ce, &st) - - if res != 0 { - return error('yuhh') - } + C.ce_next(&out, ce, &st) return time.new_time(time.Time{ year: out.year @@ -43,13 +39,9 @@ pub fn (ce &Expression) next(ref time.Time) !time.Time { }) } -pub fn (ce &Expression) next_from_now() !time.Time { +pub fn (ce &Expression) next_from_now() time.Time { out := SimpleTime{} - res := C.ce_next_from_now(&out, ce) - - if res != 0 { - return error('yuhh') - } + C.ce_next_from_now(&out, ce) return time.new_time(time.Time{ year: out.year @@ -62,13 +54,13 @@ pub fn (ce &Expression) next_from_now() !time.Time { // next_n returns the n next occurences of the expression, given a starting // time. -pub fn (ce &Expression) next_n(ref time.Time, n int) ![]time.Time { +pub fn (ce &Expression) next_n(ref time.Time, n int) []time.Time { mut times := []time.Time{cap: n} - times << ce.next(ref)! + times << ce.next(ref) for i in 1 .. n { - times << ce.next(times[i - 1])! + times << ce.next(times[i - 1]) } return times diff --git a/src/cron/expression_test.v b/src/cron/expression_test.v index d6ec002..7d1516d 100644 --- a/src/cron/expression_test.v +++ b/src/cron/expression_test.v @@ -8,7 +8,7 @@ fn util_test_time(exp string, t1_str string, t2_str string) ! { t1 := parse(t1_str)! t2 := parse(t2_str)! - t3 := ce.next(t1)! + t3 := ce.next(t1) assert t2.year == t3.year assert t2.month == t3.month diff --git a/src/server/log_removal.v b/src/server/log_removal.v index 98cba93..7f1cfb5 100644 --- a/src/server/log_removal.v +++ b/src/server/log_removal.v @@ -9,11 +9,7 @@ const fallback_log_removal_frequency = 24 * time.hour // log_removal_daemon removes old build logs every `log_removal_frequency`. fn (mut app App) log_removal_daemon(schedule cron.Expression) { - mut start_time := time.Time{} - for { - start_time = time.now() - mut too_old_timestamp := time.now().add_days(-app.conf.max_log_age) app.linfo('Cleaning logs before $too_old_timestamp') @@ -51,12 +47,7 @@ fn (mut app App) log_removal_daemon(schedule cron.Expression) { app.linfo('Cleaned $counter logs ($failed failed)') // Sleep until the next cycle - next_time := schedule.next_from_now() or { - app.lerror("Log removal daemon couldn't calculate next time: $err.msg(); fallback to $server.fallback_log_removal_frequency") - - start_time.add(server.fallback_log_removal_frequency) - } - + next_time := schedule.next_from_now() time.sleep(next_time - time.now()) } } From d6b7ce98c18c29e62ea7d137db24f06574d447c3 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 19:08:35 +0100 Subject: [PATCH 27/30] feat(cron): proper parse error handling --- src/cron/expression.c.v | 20 +++++++++++++++++++- src/cron/expression.v | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/cron/expression.c.v b/src/cron/expression.c.v index 7551e6f..0698432 100644 --- a/src/cron/expression.c.v +++ b/src/cron/expression.c.v @@ -28,6 +28,24 @@ struct C.cron_simple_time { type SimpleTime = C.cron_simple_time +enum ParseError as u8 { + ok = 0 + invalid_expression = 1 + invalid_number = 2 + out_of_range = 3 + too_many_parts = 4 +} + +fn (e ParseError) str() string { + return match e { + .ok { '' } + .invalid_expression { 'Invalid expression' } + .invalid_number { 'Invalid number' } + .out_of_range { 'Out of range' } + .too_many_parts { 'Too many parts' } + } +} + fn C.ce_init() &C.cron_expression fn C.ce_free(ce &C.cron_expression) @@ -36,4 +54,4 @@ fn C.ce_next(out &C.cron_simple_time, ce &C.cron_expression, ref &C.cron_simple_ fn C.ce_next_from_now(out &C.cron_simple_time, ce &C.cron_expression) -fn C.ce_parse_expression(out &C.cron_expression, s &char) int +fn C.ce_parse_expression(out &C.cron_expression, s &char) ParseError diff --git a/src/cron/expression.v b/src/cron/expression.v index c0cab8d..4a0d04c 100644 --- a/src/cron/expression.v +++ b/src/cron/expression.v @@ -11,7 +11,7 @@ pub fn parse_expression(exp string) !&Expression { out := C.ce_init() res := C.ce_parse_expression(out, exp.str) - if res != 0 { + if res != .ok { return error(res.str()) } From 2e6ac5cda625e64c1b9e23812e162fd9c5afdd75 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 20:46:03 +0100 Subject: [PATCH 28/30] fix(cron): fix some bugs --- src/cron/c/expression.c | 3 ++- src/cron/c/parse.c | 23 +++++++---------------- src/cron/expression_test.v | 8 ++++++++ 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/cron/c/expression.c b/src/cron/c/expression.c index 59d1bf0..4e1ca3c 100644 --- a/src/cron/c/expression.c +++ b/src/cron/c/expression.c @@ -102,7 +102,8 @@ void ce_next_from_now(cron_simple_time *out, cron_expression *ce) { gmtime_r(&t, &gm); cron_simple_time ref = { - .year = gm.tm_year, + // tm_year contains years since 1900 + .year = 1900 + gm.tm_year, // tm_mon goes from 0 to 11 .month = gm.tm_mon + 1, .day = gm.tm_mday, diff --git a/src/cron/c/parse.c b/src/cron/c/parse.c index f54b818..7a66200 100644 --- a/src/cron/c/parse.c +++ b/src/cron/c/parse.c @@ -40,18 +40,6 @@ const uint8_t max_parts = 4; * - a-b/c */ cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { - // The * expression means "every possible value" - if (s[0] == '*') { - // A '*' is only valid on its own - if (s[1] != '\0') { - return cron_parse_invalid_expression; - } - - *out = ~0; - - return cron_parse_ok; - } - size_t slash_index = 0; size_t dash_index = 0; size_t i = 0; @@ -74,14 +62,17 @@ cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max } // Parse the three possible numbers in the pattern - uint8_t start = 0; + uint8_t start = min; uint8_t end = max; uint8_t interval = 1; - SAFE_ATOI(start, s, min, max); + // * simply sets start as min and end as max + if (!(s[0] == '*' && strlen(s) == 1)) { + SAFE_ATOI(start, s, min, max); - if (dash_index > 0) { - SAFE_ATOI(end, &s[dash_index + 1], min, max); + if (dash_index > 0) { + SAFE_ATOI(end, &s[dash_index + 1], min, max); + } } if (slash_index > 0) { diff --git a/src/cron/expression_test.v b/src/cron/expression_test.v index 7d1516d..c016b72 100644 --- a/src/cron/expression_test.v +++ b/src/cron/expression_test.v @@ -34,3 +34,11 @@ fn test_next_simple() ! { //// Overlap to next year util_test_time('0 3', '2002-12-31 04:00:00', '2003-01-01 03:00:00')! } + +fn test_leading_star() { + mut x := false + + parse_expression('*5 8') or { x = true } + + assert x +} From 4fb6f629ac7d460597c7a0d97af397fb6e311a29 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 20:48:14 +0100 Subject: [PATCH 29/30] chore: please the formatter --- src/cron/expression.c.v | 1 + src/cron/expression.v | 6 ++++++ src/cron/expression_test.v | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/cron/expression.c.v b/src/cron/expression.c.v index 0698432..cd4b8b0 100644 --- a/src/cron/expression.c.v +++ b/src/cron/expression.c.v @@ -36,6 +36,7 @@ enum ParseError as u8 { too_many_parts = 4 } +// str returns the string representation of a ParseError. fn (e ParseError) str() string { return match e { .ok { '' } diff --git a/src/cron/expression.v b/src/cron/expression.v index 4a0d04c..c463d06 100644 --- a/src/cron/expression.v +++ b/src/cron/expression.v @@ -2,11 +2,13 @@ module cron import time +// free the memory associated with the Expression. [unsafe] pub fn (ce &Expression) free() { C.ce_free(ce) } +// parse_expression parses a string into an Expression. pub fn parse_expression(exp string) !&Expression { out := C.ce_init() res := C.ce_parse_expression(out, exp.str) @@ -18,6 +20,8 @@ pub fn parse_expression(exp string) !&Expression { return out } +// next calculates the next occurence of the cron schedule, given a reference +// point. pub fn (ce &Expression) next(ref time.Time) time.Time { st := SimpleTime{ year: ref.year @@ -39,6 +43,8 @@ pub fn (ce &Expression) next(ref time.Time) time.Time { }) } +// next_from_now calculates the next occurence of the cron schedule with the +// current time as reference. pub fn (ce &Expression) next_from_now() time.Time { out := SimpleTime{} C.ce_next_from_now(&out, ce) diff --git a/src/cron/expression_test.v b/src/cron/expression_test.v index c016b72..9863ef5 100644 --- a/src/cron/expression_test.v +++ b/src/cron/expression_test.v @@ -37,7 +37,7 @@ fn test_next_simple() ! { fn test_leading_star() { mut x := false - + parse_expression('*5 8') or { x = true } assert x From 433184877c5cfd8c2a87cd68057ca86b42ed118d Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 20:54:39 +0100 Subject: [PATCH 30/30] chore: remove outdated tests --- src/cron/expression_parse_test.v | 89 -------------------------------- src/cron/expression_test.v | 6 ++- 2 files changed, 4 insertions(+), 91 deletions(-) delete mode 100644 src/cron/expression_parse_test.v diff --git a/src/cron/expression_parse_test.v b/src/cron/expression_parse_test.v deleted file mode 100644 index 0b0b605..0000000 --- a/src/cron/expression_parse_test.v +++ /dev/null @@ -1,89 +0,0 @@ -module cron - -// parse_range_error returns the returned error message. If the result is '', -// that means the function didn't error. -fn parse_range_error(s string, min int, max int) string { - parse_range(s, min, max) or { return err.msg } - - return '' -} - -// =====parse_range===== -fn test_range_star_range() ! { - bf := parse_range('*', 0, 5)! - - assert bf_to_ints(bf, 0) == [0, 1, 2, 3, 4, 5] -} - -fn test_range_number() ! { - bf := parse_range('4', 0, 5)! - - assert bf_to_ints(bf, 0) == [4] -} - -fn test_range_number_too_large() ! { - assert parse_range_error('10', 0, 6) == 'Out of range.' -} - -fn test_range_number_too_small() ! { - assert parse_range_error('0', 2, 6) == 'Out of range.' -} - -fn test_range_number_invalid() ! { - assert parse_range_error('x', 0, 6) == 'Invalid number.' -} - -fn test_range_step_star_1() ! { - bf := parse_range('*/4', 0, 20)! - - assert bf_to_ints(bf, 0) == [0, 4, 8, 12, 16, 20] -} - -fn test_range_step_star_2() ! { - bf := parse_range('*/3', 1, 8)! - - assert bf_to_ints(bf, 1) == [1, 4, 7] -} - -fn test_range_step_star_too_large() ! { - assert parse_range_error('*/21', 0, 20) == 'Step size too large.' -} - -fn test_range_step_zero() ! { - assert parse_range_error('*/0', 0, 20) == 'Step size zero not allowed.' -} - -fn test_range_step_number() ! { - bf := parse_range('5/4', 2, 22)! - - assert bf_to_ints(bf, 2) == [5, 9, 13, 17, 21] -} - -fn test_range_step_number_too_large() ! { - assert parse_range_error('10/4', 0, 5) == 'Out of range.' -} - -fn test_range_step_number_too_small() ! { - assert parse_range_error('2/4', 5, 10) == 'Out of range.' -} - -fn test_range_dash() ! { - bf := parse_range('4-8', 0, 9)! - - assert bf_to_ints(bf, 0) == [4, 5, 6, 7, 8] -} - -fn test_range_dash_step() ! { - bf := parse_range('4-8/2', 0, 9)! - - assert bf_to_ints(bf, 0) == [4, 6, 8] -} - -// =====parse_part===== -fn test_part_single() ! { - assert parse_part('*', 0, 5)! == [0, 1, 2, 3, 4, 5] -} - -fn test_part_multiple() ! { - assert parse_part('*/2,2/3', 1, 8)! == [1, 2, 3, 5, 7, 8] -} diff --git a/src/cron/expression_test.v b/src/cron/expression_test.v index 9863ef5..4023012 100644 --- a/src/cron/expression_test.v +++ b/src/cron/expression_test.v @@ -26,7 +26,7 @@ fn test_next_simple() ! { util_test_time(exp, '2002-01-01 03:00:00', '2002-01-02 03:00:00')! util_test_time(exp, '2002-01-01 04:00:00', '2002-01-02 03:00:00')! - util_test_time('0 3/4', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! + util_test_time('0 3-7/4,7-19', '2002-01-01 04:00:00', '2002-01-01 07:00:00')! //// Overlap to next month util_test_time('0 3', '2002-11-31 04:00:00', '2002-12-01 03:00:00')! @@ -37,8 +37,10 @@ fn test_next_simple() ! { fn test_leading_star() { mut x := false - parse_expression('*5 8') or { x = true } + assert x + x = false + parse_expression('x 8') or { x = true } assert x }