reprint warnings before exit (ch-run, ch-image)

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)

bin/ch-run.c

@@ -9,6 +9,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <syslog.h>
+#include <sys/mman.h>
 #include <unistd.h>
 
 #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;

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
+}

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);

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)

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\ 

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'* ]]

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 ]]
+
+}