Diff options
| author | Bryce Kwon <bryce@brycekwon.com> | |
|---|---|---|
| committer | Bryce Kwon <bryce@brycekwon.com> | |
| commit | ||
| parent | ||
| tree | ||
| download | ||
Report a max-stats value that names no period
A value cgit does not know as a period turns the statistics page off,
so a typo such as "decade" read the same as leaving stats off on
purpose. The empty value stays quiet because a cached repolist writes
it for a repository that turned stats off.
| -rw-r--r-- | MANUAL.txt | 5 | |
| -rw-r--r-- | source/cgit.c | 29 | |
| -rwxr-xr-x | tests/t0205-config.sh | 27 |
3 files changed, 57 insertions, 4 deletions
@@ -456,8 +456,9 @@ max-stats:: Enable the statistics page, which holds the commits-per-author table, and set the coarsest period it offers. Valid values are "week", "month", "quarter" and "year", each of which also offers the finer periods before - it. If unspecified, statistics are disabled. Default value: none. See - also: "repo.max-stats". + it. If unspecified, statistics are disabled, and any other value but "0" + disables them with a warning. Default value: none. See also: + "repo.max-stats". mimetype-file:: Specifies the file to use for automatic mimetype lookup. If specified diff --git a/source/cgit.c b/source/cgit.c index 12ed19e..5a0f2e3 100644 --- a/source/cgit.c +++ b/source/cgit.c @@ -575,6 +575,31 @@ static void parse_date_setting(const char *format, struct date_mode *mode) } } +/* + * A max-stats value that names no period turns the statistics page off, the + * same as leaving it unset, so a typo is logged with where it was written. + * An empty value and "0" turn the page off on purpose, and a cached repolist + * writes the empty one, so those stay quiet. + */ +static int config_stats_period(const char *value) +{ + int period = cgit_find_stats_period(value, NULL); + int len; + + if (period || !*value || !strcmp(value, "0")) + return period; + // The first line alone, since a git config value can hold a newline + // that would forge a log line of its own. + len = (int)strcspn(value, "\n"); + if (config_origin.file) + fprintf(stderr, "[cgit] Turning statistics off for max-stats=%.*s in %s line %d, since it names no period\n", + len, value, config_origin.file, config_origin.line); + else + fprintf(stderr, "[cgit] Turning statistics off for max-stats=%.*s, since it names no period\n", + len, value); + return period; +} + static void apply_config(const char *name, const char *value) { const char *arg; @@ -666,7 +691,7 @@ static void apply_config(const char *name, const char *value) else if (!strcmp(name, "enable-cache-list")) ctx.cfg.enable_cache_list = config_int(name, value); else if (!strcmp(name, "max-stats")) - ctx.cfg.max_stats = cgit_find_stats_period(value, NULL); + ctx.cfg.max_stats = config_stats_period(value); else if (!strcmp(name, "cache-size")) ctx.cfg.cache_size = config_int(name, value); else if (!strcmp(name, "cache-root")) @@ -1367,7 +1392,7 @@ void cgit_repo_config(struct cgit_repo *repo, const char *name, const char *valu if (!strcmp(value, "topo")) repo->commit_sort = 2; } else if (!strcmp(name, "max-stats")) - repo->max_stats = cgit_find_stats_period(value, NULL); + repo->max_stats = config_stats_period(value); else if (!strcmp(name, "module-link")) repo->module_link = cgit_strdup_first_line(value); else if (skip_prefix(name, "module-link.", &path)) { diff --git a/tests/t0205-config.sh b/tests/t0205-config.sh index 0adb29f..88a271f 100755 --- a/tests/t0205-config.sh +++ b/tests/t0205-config.sh @@ -242,6 +242,33 @@ test_expect_success 'an unknown snapshot format is reported' ' test_must_be_empty err ' +# A max-stats value that names no period turns the statistics page off as if +# it were unset, so it is logged with the file and line it came from. The +# empty value and 0 that turn the page off on purpose stay quiet, and so does +# a period's one letter code. +test_expect_success 'a max-stats that names no period is reported' ' + cgitrc_repo statsrc stats max-stats=decade "repo.path=$PWD/repos/foo/.git" && + cgit_rc statsrc "url=stats/stats/" >tmp 2>err && + test_grep "^Status: 404" tmp && + test_grep -F "Turning statistics off for max-stats=decade in $PWD/statsrc line 3, since it names no period" err && + test_line_count = 1 err && + cgitrc_repo repostatsrc stats max-stats=year "repo.path=$PWD/repos/foo/.git" repo.max-stats=century && + cgit_rc repostatsrc "url=stats/stats/" >tmp 2>err && + test_grep "^Status: 404" tmp && + test_grep -F "Turning statistics off for max-stats=century in $PWD/repostatsrc line 6, since it names no period" err && + test_line_count = 1 err && + cgitrc_repo letterrc stats max-stats=y "repo.path=$PWD/repos/foo/.git" && + cgit_ok letterrc "url=stats/stats/" >tmp && + test_grep "<option value=.y.>year</option>" tmp && + for off in 0 "" + do + cgitrc_repo offrc stats max-stats=year "repo.path=$PWD/repos/foo/.git" "repo.max-stats=$off" && + cgit_rc offrc "url=stats/stats/" >tmp 2>err && + test_grep "^Status: 404" tmp && + test_must_be_empty err || return 1 + done +' + # The error page names the repository alone, so the log gets the path that # would not open, once for the request. test_expect_success 'a repository that will not open is logged with its path' ' |