comment on log level stuff

bin/ch-image.py.in

@@ -283,14 +283,18 @@ def main():
    # modify
    sp = ap.add_parser("modify", "foo")
    add_opts(sp, modify.main, deps_check=True, stog_init=True)
-   sp.add_argument("-c", metavar="CMD", action="append", default=[], nargs=1, help="foo")
-   sp.add_argument("-i", "--interactive", action="store_true", help="foo")
-   sp.add_argument("-t", "--test", action="store_true", help="foo")
-   sp.add_argument("-S", "--shell", metavar="shell", default="/bin/sh", help="foo")
+   sp.add_argument("-c", metavar="CMD", action="append", default=[], nargs=1,
+                   help="Run CMD as though specified by a RUN instruction. Can be repeated.")
+   sp.add_argument("-i", "--interactive", action="store_true",
+                   help="modify in interactive mode, even if stdin is not a TTY")
+   sp.add_argument("-S", "--shell", metavar="shell", default="/bin/sh",
+                   help="use SHELL instead of the default /bin/sh")
    sp.add_argument("image_ref", metavar="IMAGE_REF", help="image to modify")
    sp.add_argument("out_image", metavar="OUT_IMAGE", help="destination of modified image")
    sp.add_argument("script", metavar="SCRIPT", help="foo", nargs='?')
-   # Options “modify” shares with “build”.
+   # Options “modify” shares with “build”. Note that while we could abstract
+   # this out to avoid repeated lines, as we do for “common_opts”, we’ve decided
+   # that the tradeoff in code readability wouldn’t be worth it.
    sp.add_argument("-b", "--bind", metavar="SRC[:DST]",
                    action="append", default=[],
                    help="mount SRC at guest DST (default: same as SRC)")

bin/ch_misc.c

@@ -592,10 +592,16 @@ void msg_error(const char *file, int line, int errno_,
    va_list ap;
 
    va_start(ap, fmt);
+   /* We print errors at LL_FATAL because, according to our documentation,
+      errors are never suppressed. Perhaps we need to rename this log level (see
+      issue #1914). */
    msgv(LL_FATAL, file, line, errno_, fmt, ap);
    va_end(ap);
 }
 
+/* Note that msg_fatal doesn’t call msg_error like we do in the Python code
+   because the variable number of arguments make it easier to simply define
+   separate functions. */
 noreturn void msg_fatal(const char *file, int line, int errno_,
                        const char *fmt, ...)
 {

doc/ch-image.rst

@@ -2258,9 +2258,9 @@ Interactive mode
    iterating a container, it rarely saves time over editing a Dockerfile or
    shell script. Only use it if you really know what you are doing.
 
-If :code:`SCRIPT` is not provided and standard input *is* a TTY,
-:code:`ch-image modify` opens an interactive shell. That is, the following are
-roughly equivalent (assuming a terminal)::
+If :code:`SCRIPT` is not provided and standard input *is* a TTY, or if
+:code:`-i` is specified :code:`ch-image modify` opens an interactive shell. That
+is, the following are roughly equivalent (assuming a terminal)::
 
   $ ch-image modify foo bar
 

lib/modify.py

@@ -91,9 +91,6 @@ def main(cli_):
    ch.VERBOSE("modify shell: %s" % cli.shell)
    ch.VERBOSE("modify mode: %s" % mode.value)
 
-   if (cli.test):
-      exit(0)
-
    if (mode == Modify_Mode.INTERACTIVE):
       # Interactive case
 

lib/pull.py

@@ -106,7 +106,7 @@ def download(self):
                # currently, this error is only raised if we’ve downloaded the
                # skinny manifest.
                have_skinny = True
-               if (ch.arch in "amd64"):
+               if (ch.arch == "amd64"):
                   # We’re guessing that enough arch-unaware images are amd64 to
                   # barge ahead if requested architecture is amd64.
                   ch.arch = "yolo"