diff --git a/bin/ch-image.py.in b/bin/ch-image.py.in index 2d6067cca..18b16bda7 100644 --- a/bin/ch-image.py.in +++ b/bin/ch-image.py.in @@ -309,6 +309,7 @@ def main(): # Dispatch. ch.profile_start() cli.func(cli) + ch.warnings_dump() ch.exit(0) @@ -318,5 +319,6 @@ if (__name__ == "__main__"): try: main() except ch.Fatal_Error as x: + ch.warnings_dump() ch.ERROR(*x.args, **x.kwargs) ch.exit(1) diff --git a/bin/ch-run.c b/bin/ch-run.c index ae4ea5031..c7e3df006 100644 --- a/bin/ch-run.c +++ b/bin/ch-run.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include "config.h" @@ -73,6 +74,7 @@ const struct argp_option options[] = { { "unset-env", -7, "GLOB", 0, "unset environment variable(s)" }, { "verbose", 'v', 0, 0, "be more verbose (can be repeated)" }, { "version", 'V', 0, 0, "print version and exit" }, + { "warnings", -16, "NUM", 0, "log NUM warnings and exit" }, { "write", 'w', 0, 0, "mount image read-write"}, { 0 } }; @@ -104,12 +106,14 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state); void parse_set_env(struct args *args, char *arg, int delim); void privs_verify_invoking(); char *storage_default(void); +extern void warnings_reprint(void); /** Global variables **/ const struct argp argp = { options, parse_opt, args_doc, usage }; extern char **environ; // see environ(7) +extern char *warnings; /** Main **/ @@ -121,8 +125,15 @@ int main(int argc, char *argv[]) int arg_next; char ** c_argv; + // initialize “warnings” buffer + warnings = mmap(NULL, WARNINGS_SIZE, PROT_READ | PROT_WRITE, + MAP_SHARED | MAP_ANONYMOUS, -1, 0); + T_ (warnings != MAP_FAILED); + privs_verify_invoking(); + Z_ (atexit(warnings_reprint)); + #ifdef ENABLE_SYSLOG syslog(LOG_USER|LOG_INFO, "uid=%u args=%d: %s", getuid(), argc, argv_to_string(argv)); @@ -448,6 +459,11 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state) args->seccomp_p = true; break; #endif + case -16: // --warnings + for (int i = 1; i <= parse_int(arg, false, "--warnings"); i++) + WARNING("this is warning %d!", i); + exit(0); + break; case -15: // --set-env0 parse_set_env(args, arg, '\0'); break; diff --git a/bin/ch_misc.c b/bin/ch_misc.c index e9908d3e8..92034613e 100644 --- a/bin/ch_misc.c +++ b/bin/ch_misc.c @@ -41,6 +41,18 @@ char *host_tmp = NULL; /* Username of invoking users. Set during command line processing. */ char *username = NULL; +/* List of warnings to be re-printed on exit. This is a buffer of shared memory + allocated by mmap(2), structured as a sequence of null-terminated character + strings. Warnings that do not fit in this buffer will be lost, though we + allocate enough memory that this is unlikely. See “string_append()” for + more details. */ +char *warnings; + +/* Current byte offset from start of “warnings” buffer. This gives the address + where the next appended string will start. This means that the null + terminator of the previous string is warnings_offset - 1. */ +size_t warnings_offset = 0; + /** Function prototypes (private) **/ @@ -127,6 +139,37 @@ char *argv_to_string(char **argv) return s; } +/* Iterate through buffer “buf” of size “s” consisting of null-terminated + strings and return the number of strings in it. Key assumptions: + + 1. The buffer has been initialized to zero, i.e. all bytes that have not + been explicitly set are null. + + 2. All strings have been appended to the buffer in full without + truncation, including their null terminator. + + 3. The buffer contains no empty strings. + + These assumptions are consistent with the construction of the “warnings” + shared memory buffer, which is the main justification for this function. Note + that under these assumptions, the final byte in the buffer is guaranteed to + be null. */ +int buf_strings_count(char *buf, size_t size) +{ + int count = 0; + + if (buf[0] != '\0') { + for (size_t i = 0; i < size; i++) + if (buf[i] == '\0') { // found string terminator + count++; + if (i < size - 1 && buf[i+1] == '\0') // two term. in a row; done + break; + } + } + + return count; +} + /* Return true if buffer buf of length size is all zeros, false otherwise. */ bool buf_zero_p(void *buf, size_t size) { @@ -450,18 +493,21 @@ noreturn void msg_fatal(const char *file, int line, int errno_, void msgv(enum log_level level, const char *file, int line, int errno_, const char *fmt, va_list ap) { + char *message, *ap_msg; + if (level > verbose) return; - fprintf(stderr, "%s[%d]: ", program_invocation_short_name, getpid()); + T_ (1 <= asprintf(&message, "%s[%d]: ", + program_invocation_short_name, getpid())); // Prefix for the more urgent levels. switch (level) { case LL_FATAL: - fprintf(stderr, "error: "); // "fatal" too morbid for users + message = cat(message, "error: "); // "fatal" too morbid for users break; case LL_WARNING: - fprintf(stderr, "warning: "); + message = cat(message, "warning: "); break; default: break; @@ -471,12 +517,19 @@ void msgv(enum log_level level, const char *file, int line, int errno_, if (fmt == NULL) fmt = "please report this bug"; - vfprintf(stderr, fmt, ap); - if (errno_) - fprintf(stderr, ": %s (%s:%d %d)\n", - strerror(errno_), file, line, errno_); - else - fprintf(stderr, " (%s:%d)\n", file, line); + T_ (1 <= vasprintf(&ap_msg, fmt, ap)); + if (errno_) { + T_ (1 <= asprintf(&message, "%s%s: %s (%s:%d %d)", message, ap_msg, + strerror(errno_), file, line, errno_)); + } else { + T_ (1 <= asprintf(&message, "%s%s (%s:%d)", message, ap_msg, file, line)); + } + + if (level == LL_WARNING) { + warnings_offset += string_append(warnings, message, WARNINGS_SIZE, + warnings_offset); + } + fprintf(stderr, "%s\n", message); if (fflush(stderr)) abort(); // can't print an error b/c already trying to do that } @@ -640,3 +693,38 @@ void version(void) { fprintf(stderr, "%s\n", VERSION); } + +/* Append null-terminated string “str” to the memory buffer “offset” bytes after + from the address pointed to by “addr”. Buffer length is “size” bytes. Return + the number of bytes written. If there isn’t enough room for the string, do + nothing and return zero. */ +size_t string_append(char *addr, char *str, size_t size, size_t offset) +{ + size_t written = strlen(str) + 1; + + if (size > (offset + written - 1)) // there is space + memcpy(addr + offset, str, written); + + return written; +} + +/* Reprint messages stored in “warnings” memory buffer. */ +void warnings_reprint(void) +{ + size_t offset = 0; + int warn_ct = buf_strings_count(warnings, WARNINGS_SIZE); + + if (warn_ct > 0) + fprintf(stderr, "%s[%d]: warning: reprinting first %d warning(s)\n", + program_invocation_short_name, getpid(), warn_ct); + + while ( warnings[offset] != 0 + || (offset < (WARNINGS_SIZE - 1) && warnings[offset+1] != 0)) { + fputs(warnings + offset, stderr); + fputc('\n', stderr); + offset += strlen(warnings + offset) + 1; + } + + if (fflush(stderr)) + abort(); // can't print an error b/c already trying to do that +} diff --git a/bin/ch_misc.h b/bin/ch_misc.h index 267658162..52a76ec6b 100644 --- a/bin/ch_misc.h +++ b/bin/ch_misc.h @@ -19,6 +19,10 @@ and hopefully others support the following extension. */ #define noreturn __attribute__ ((noreturn)) +/* Size of “warnings” buffer, in bytes. We want this to be big enough that we + don’t need to worry about running out of room. */ +#define WARNINGS_SIZE (4*1024) + /* Test some value, and if it's not what we expect, exit with a fatal error. These are macros so we have access to the file and line number. @@ -103,11 +107,14 @@ enum log_level { LL_FATAL = -2, // minimum number of -v to print the msg extern enum log_level verbose; extern char *host_tmp; extern char *username; +extern char *warnings; +extern size_t warnings_offset; /** Function prototypes **/ char *argv_to_string(char **argv); +int buf_strings_count(char *str, size_t s); bool buf_zero_p(void *buf, size_t size); char *cat(const char *a, const char *b); struct env_var *env_file_read(const char *path, int delim); @@ -130,3 +137,5 @@ char *realpath_(const char *path, bool fail_ok); void replace_char(char *str, char old, char new); void split(char **a, char **b, const char *str, char del); void version(void); +size_t string_append(char *addr, char *str, size_t size, size_t offset); +void warnings_reprint(void); diff --git a/lib/charliecloud.py b/lib/charliecloud.py index a4e9062be..2c5357663 100644 --- a/lib/charliecloud.py +++ b/lib/charliecloud.py @@ -119,6 +119,9 @@ class Download_Mode(enum.Enum): log_fp = sys.stderr # File object to print logs to. trace_fatal = False # Add abbreviated traceback to fatal error hint. +# Warnings to be re-printed when program exits +warnings = list() + # True if the download cache is enabled. dlcache_p = None @@ -439,7 +442,9 @@ def VERBOSE(msg, hint=None, **kwargs): if (verbose >= 1): log(msg, hint, None, "38;5;14m", "", **kwargs) # light cyan (1;36m, not bold) -def WARNING(msg, hint=None, **kwargs): +def WARNING(msg, hint=None, msg_save=True, **kwargs): + if (msg_save): + warnings.append(msg) log(msg, hint, None, "31m", "warning: ", **kwargs) # red def arch_host_get(): @@ -847,3 +852,9 @@ def version_check(argv, min_, required=True, regex=r"(\d+)\.(\d+)\.(\d+)"): return False VERBOSE("%s version OK: %d.%d.%d ≥ %d.%d.%d" % ((prog,) + v + min_)) return True + +def warnings_dump(): + if (len(warnings) > 0): + WARNING("reprinting %d warning(s)" % len(warnings), msg_save=False) + for msg in warnings: + WARNING(msg, msg_save=False) diff --git a/test/approved-trailing-whitespace b/test/approved-trailing-whitespace index 37fdefd56..7b1dee981 100644 --- a/test/approved-trailing-whitespace +++ b/test/approved-trailing-whitespace @@ -10,5 +10,5 @@ ./test/build/50_dockerfile.bats:96:RUN echo test4 \ ./test/build/50_dockerfile.bats:97:b \ ./test/build/50_dockerfile.bats:131: 4. RUN true -./test/build/50_dockerfile.bats:431:#ENV chse_1a value 1a -./test/build/50_dockerfile.bats:434:#ENV chse_1c=value\ 1c\ +./test/build/50_dockerfile.bats:433:#ENV chse_1a value 1a +./test/build/50_dockerfile.bats:436:#ENV chse_1c=value\ 1c\ diff --git a/test/build/50_dockerfile.bats b/test/build/50_dockerfile.bats index b1776f13a..58fab1800 100644 --- a/test/build/50_dockerfile.bats +++ b/test/build/50_dockerfile.bats @@ -160,6 +160,8 @@ test 7 b grown in 16 instructions: tmpimg build slow? consider enabling the new build cache hint: https://hpc.github.io/charliecloud/command-usage.html#build-cache +warning: reprinting 1 warning(s) +warning: not yet supported, ignored: issue #777: .dockerignore file EOF ) diff -u <(echo "$output_expected") <(echo "$output") @@ -289,7 +291,7 @@ ONBUILD foo EOF echo "$output" [[ $status -eq 0 ]] - [[ $(echo "$output" | grep -Ec 'not yet supported.+instruction') -eq 4 ]] + [[ $(echo "$output" | grep -Ec 'not yet supported.+instruction') -eq 8 ]] [[ $output = *'warning: not yet supported, ignored: issue #782: ADD instruction'* ]] [[ $output = *'warning: not yet supported, ignored: issue #780: CMD instruction'* ]] [[ $output = *'warning: not yet supported, ignored: issue #780: ENTRYPOINT instruction'* ]] @@ -361,7 +363,7 @@ EOF echo "$output" [[ $status -eq 0 ]] [[ $output = *'warning: not supported, ignored: parser directives'* ]] - [[ $(echo "$output" | grep -Fc 'parser directives') -eq 5 ]] + [[ $(echo "$output" | grep -Fc 'parser directives') -eq 10 ]] # COPY --from run ch-image build -t tmpimg -f - . <<'EOF' @@ -384,7 +386,7 @@ VOLUME foo EOF echo "$output" [[ $status -eq 0 ]] - [[ $(echo "$output" | grep -Fc 'not supported') -eq 6 ]] + [[ $(echo "$output" | grep -Fc 'not supported') -eq 12 ]] [[ $output = *'warning: not supported, ignored: EXPOSE instruction'* ]] [[ $output = *'warning: not supported, ignored: HEALTHCHECK instruction'* ]] [[ $output = *'warning: not supported, ignored: MAINTAINER instruction'* ]] diff --git a/test/run/ch-run_misc.bats b/test/run/ch-run_misc.bats index 7123f334d..cd7637adf 100644 --- a/test/run/ch-run_misc.bats +++ b/test/run/ch-run_misc.bats @@ -1054,3 +1054,28 @@ EOF echo "$text" echo "$text" | grep -F "$expected" } + +@test 'reprint warnings' { + run ch-run --warnings=0 + [[ $status -eq 0 ]] + [[ $(echo "$output" | grep -Fc 'this is warning 1!') -eq 0 ]] + [[ $(echo "$output" | grep -Fc 'this is warning 2!') -eq 0 ]] + [[ "$output" != *'reprinting first'* ]] + + run ch-run --warnings=1 + echo "$output" + [[ $status -eq 0 ]] + [[ $output == *'ch-run['*']: warning: reprinting first 1 warning(s)'* ]] + [[ $(echo "$output" | grep -Fc 'this is warning 1!') -eq 2 ]] + [[ $(echo "$output" | grep -Fc 'this is warning 2!') -eq 0 ]] + + # Warnings list is a statically sized memory buffer. Ensure it works as + # intended by printing more warnings than can be saved to this buffer. + run ch-run --warnings=100 + echo "$output" + [[ $status -eq 0 ]] + [[ $output == *'ch-run['*']: warning: reprinting first '*' warning(s)'* ]] + [[ $(echo "$output" | grep -Fc 'this is warning 1!') -eq 2 ]] + [[ $(echo "$output" | grep -Fc 'this is warning 100!') -eq 1 ]] + +}