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,15 @@ 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;
+extern const size_t warnings_size;
 
 
 /** Main **/
@@ -121,8 +126,14 @@ int main(int argc, char *argv[])
    int arg_next;
    char ** c_argv;
 
+   // initialze “warnings” buffer
+   warnings = mmap(NULL, warnings_size, PROT_READ | PROT_WRITE,
+                   MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+
    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,12 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
       args->seccomp_p = true;
       break;
 #endif
+   case -16: { // --warnings
+      int w_num = parse_int(arg, false, "--warnings");
+      for (int i = 0; i < w_num; i++) {
+         WARNING("this is a warning!");
+      }
+      exit(0); }
    case -15: // --set-env0
       parse_set_env(args, arg, '\0');
       break;

bin/ch_misc.c

@@ -41,6 +41,21 @@ 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
+   “warnings_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. */
+size_t warnings_offset = 0;
+
+/* 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. */
+const size_t warnings_size = 4096;
+
 
 /** Function prototypes (private) **/
 
@@ -450,18 +465,19 @@ 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 = "";
    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
+      T_ (1 <= asprintf(&message, "%s%s", message, "error: ")); // "fatal" too morbid for users
       break;
    case LL_WARNING:
-      fprintf(stderr, "warning: ");
+      T_ (1 <= asprintf(&message, "%s%s", message, "warning: "));
       break;
    default:
       break;
@@ -471,12 +487,22 @@ 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);
+   char *ap_msg = "";
+   T_ (1 <= vasprintf(&ap_msg, fmt, ap));
+   T_ (1 <= asprintf(&message, "%s%s", message, ap_msg));
+
+   if (errno_) {
+      T_ (1 <= asprintf(&message, "%s: %s (%s:%d %d)", message,
+                        strerror(errno_), file, line, errno_));
+   } else {
+      T_ (1 <= asprintf(&message, "%s (%s:%d)", message, file, line));
+   }
+
+   if (level == LL_WARNING) {
+      warnings_offset += warnings_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 +666,37 @@ void version(void)
 {
    fprintf(stderr, "%s\n", VERSION);
 }
+
+/* Append null-terminated string “str” to the memory buffer “offset” bytes away
+   from the address pointed to by “addr”. Buffer should be “size” bytes long.
+   Return the number of bytes written. If there isn’t enough room for the
+   string, do nothing and return zero. */
+size_t warnings_append(char *addr, char *str, size_t size, size_t offset)
+{
+   size_t written = 0;
+   if (size > (offset + strlen(str))) { // check if there’s space
+      // FIXME: better solution than writing byte-by-byte?
+      for (written = 0; written <= strlen(str); written++) {
+         addr[offset + written] = str[written];
+      }
+   }
+   return written;
+}
+
+/* Reprint messages stored in “warnings” memory buffer. */
+void warnings_reprint(void)
+{
+   size_t offset = 0;
+   while ((warnings[offset] != 0) ||
+            ((offset < (warnings_size - 1) &&
+            (warnings[offset+1] != 0)))) {
+        fputs(warnings + offset, stderr);
+        fputc('\n', stderr);
+        while ((offset < warnings_size - 2) && (warnings[offset] != 0)) {
+            offset++;
+        }
+        offset++;
+    }
+   if (fflush(stderr))
+         abort();  // can't print an error b/c already trying to do that
+}

bin/ch_misc.h

@@ -103,6 +103,9 @@ 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;
+extern const size_t warnings_size;
 
 
 /** Function prototypes **/
@@ -130,3 +133,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 warnings_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 exists
+warnings = []
+
 # 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,7 @@ 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():
+   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:432:#ENV chse_1a value 1a 
+./test/build/50_dockerfile.bats:435:#ENV chse_1c=value\ 1c\ 

test/build/50_dockerfile.bats

@@ -160,6 +160,7 @@ 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: not yet supported, ignored: issue #777: .dockerignore file
 EOF
 )
     diff -u <(echo "$output_expected") <(echo "$output")
@@ -289,7 +290,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 +362,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 +385,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,19 @@ 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 a warning!') -eq 0 ]]
+
+    run ch-run --warnings=1
+    [[ $status -eq 0 ]]
+    [[ $(echo "$output" | grep -Fc 'this is a warning!') -eq 2 ]]
+
+    # Warnings list is a statically sized memory buffer. Ensure it works as
+    # intended by printing more warnings than can be saved to this buffer and
+    # checking that the program doesn’t crash.
+    run ch-run --warnings=10000
+    [[ $status -eq 0 ]]
+}