Merge pull request #5274 from grondo/issue#5272

do not suppress job shell and broker errors with `flux alloc`

Author: mergify[bot]

src/cmd/flux-job.c

@@ -1576,6 +1576,7 @@ struct attach_ctx {
     flux_watcher_t *sigtstp_w;
     flux_watcher_t *notify_timer;
     struct flux_pty_client *pty_client;
+    int pty_capture;
     struct timespec t_sigint;
     flux_watcher_t *stdin_w;
     zlist_t *stdin_rpcs;
@@ -1646,6 +1647,15 @@ static void handle_output_data (struct attach_ctx *ctx, json_t *context)
         log_msg_exit ("stream data read before header");
     if (iodecode (context, &stream, &rank, &data, &len, NULL) < 0)
         log_msg_exit ("malformed event context");
+    /*
+     * If this process is attached to a pty (ctx->pty_client != NULL)
+     *  and output corresponds to rank 0 and the interactive pty is being
+     *  captured, then this data is a duplicate, so do nothing.
+     */
+    if (ctx->pty_client != NULL
+        && streq (rank, "0")
+        && ctx->pty_capture)
+        goto out;
     if (streq (stream, "stdout"))
         fp = stdout;
     else
@@ -1654,8 +1664,14 @@ static void handle_output_data (struct attach_ctx *ctx, json_t *context)
         if (optparse_hasopt (ctx->p, "label-io"))
             fprintf (fp, "%s: ", rank);
         fwrite (data, len, 1, fp);
+        /*  If attached to a pty, terminal is in raw mode so a carriage
+         *  return will be necessary to return cursor to the start of line.
+         */
+        if (ctx->pty_client)
+            fputc ('\r', fp);
         fflush (fp);
     }
+out:
     free (data);
 }
 
@@ -1725,6 +1741,11 @@ static void handle_output_log (struct attach_ctx *ctx,
                 fprintf (stderr, ":%d", line);
         }
         fprintf (stderr, ": %s\n", msg);
+        /*  If attached to a pty, terminal is in raw mode so a carriage
+         *  return will be necessary to return cursor to the start of line.
+         */
+        if (ctx->pty_client)
+            fprintf (stderr, "\r");
     }
 }
 
@@ -2314,13 +2335,15 @@ void attach_exec_event_continuation (flux_future_t *f, void *arg)
     if (streq (name, "shell.init")) {
         const char *pty_service = NULL;
         if (json_unpack (context,
-                         "{s:i s:s s?s}",
+                         "{s:i s:s s?s s?i}",
                          "leader-rank",
                          &ctx->leader_rank,
                          "service",
                          &service,
                          "pty",
-                         &pty_service) < 0)
+                         &pty_service,
+                         "capture",
+                         &ctx->pty_capture) < 0)
             log_err_exit ("error decoding shell.init context");
         if (!(ctx->service = strdup (service)))
             log_err_exit ("strdup service from shell.init");
@@ -2333,15 +2356,14 @@ void attach_exec_event_continuation (flux_future_t *f, void *arg)
          *   to process normal stdio. (This may be because the job is
          *   already complete).
          */
+        attach_output_start (ctx);
         if (pty_service) {
             if (ctx->readonly)
                 log_msg_exit ("Cannot connect to pty in readonly mode");
             attach_pty (ctx, pty_service);
         }
-        else {
+        else
             attach_setup_stdin (ctx);
-            attach_output_start (ctx);
-        }
     } else if (streq (name, "shell.start")) {
         if (MPIR_being_debugged)
             setup_mpir_interface (ctx, context);

src/shell/pty.c

@@ -339,9 +339,24 @@ static int pty_init (flux_plugin_t *p,
                                               0,
                                               "{s:s}",
                                               "pty", "terminus.0") < 0) {
-                shell_log_errno ("flux_shell_service_register");
+                shell_log_errno ("flux_shell_add_event_context (pty)");
                 goto error;
             }
+            if (capture) {
+                /*
+                 * If also capturing the pty output for an interactive
+                 *  pty, note this in the shell.init event context. This
+                 *  will hint to the pty reader that the terminal output
+                 *  is duplicated for rank 0.
+                 */
+                if (flux_shell_add_event_context (shell,
+                                                  "shell.init",
+                                                  0,
+                                                  "{s:i}",
+                                                  "capture", 1) < 0) {
+                    shell_log_errno ("flux_shell_add_event_context (capture)");
+                }
+            }
             /*  Ensure that rank 0 pty waits for client to attach
              *   in pty.interactive mode, even if pty.capture is also
              *   specified.

t/t2712-python-cli-alloc.t

@@ -143,5 +143,13 @@ test_expect_success 'flux alloc: --dump=FILE works with mustache' '
 	flux job wait-event $jobid clean &&
         tar tvf testdump-${jobid}.tgz
 '
+test_expect_success 'flux alloc: does not suppress log messages' '
+	$runpty -o logmsgs.out flux alloc -n1 --cwd=/noexist pwd &&
+	grep -i "going to /tmp instead" logmsgs.out
+'
+test_expect_success 'flux alloc: no duplication of output with pty.capture' '
+	$runpty -o duplicates.out flux alloc -o pty.capture -n1 echo testing &&
+	test $(grep -c testing duplicates.out) -eq 1
+'
 
 test_done