[core] Print setpgid() files to the tracer, no stderr

Job control with pipelines is a inherently racy.

Zulip thread:

    https://oilshell.zulipchat.com/#narrow/stream/307442-nix/topic/starship.20bug.20repro

This is part of issue #1822.

core/dev.py

@@ -271,7 +271,7 @@ class Tracer(object):
     Other hooks:
 
     - Command completion starts other processes
-    - Oil command constructs: BareDecl, VarDecl, Mutation, Expr
+    - YSH command constructs: BareDecl, VarDecl, Mutation, Expr
     """
 
     def __init__(
@@ -490,9 +490,9 @@ def PopMessage(self, label, arg):
             buf.write('\n')
             self.f.write(buf.getvalue())
 
-    def PrintMessage(self, message):
+    def OtherMessage(self, message):
         # type: (str) -> None
-        """Used when receiving signals."""
+        """Can be used when receiving signals."""
         buf = self._RichTraceBegin('!')
         if not buf:
             return

core/executor.py

@@ -337,10 +337,10 @@ def RunSimpleCommand(self, cmd_val, cmd_st, run_flags):
                     # If job control is enabled, this should be true
                     assert pgid != process.INVALID_PGID
 
-                    change = process.SetPgid(pgid)
+                    change = process.SetPgid(pgid, self.tracer)
                     self.fg_pipeline = None  # clear to avoid confusion in subshells
                 else:
-                    change = process.SetPgid(process.OWN_LEADER)
+                    change = process.SetPgid(process.OWN_LEADER, self.tracer)
                 p.AddStateChange(change)
 
             status = p.RunProcess(self.waiter, trace.External(cmd_val.argv))
@@ -375,7 +375,7 @@ def RunBackgroundJob(self, node):
         if UP_node.tag() == command_e.Pipeline:
             node = cast(command.Pipeline, UP_node)
             pi = process.Pipeline(self.exec_opts.sigpipe_status_ok(),
-                                  self.job_control, self.job_list)
+                                  self.job_control, self.job_list, self.tracer)
             for child in node.children:
                 p = self._MakeProcess(child)
                 p.Init_ParentPipeline(pi)
@@ -395,7 +395,8 @@ def RunBackgroundJob(self, node):
 
             p = self._MakeProcess(node)
             if self.job_control.Enabled():
-                p.AddStateChange(process.SetPgid(process.OWN_LEADER))
+                p.AddStateChange(
+                    process.SetPgid(process.OWN_LEADER, self.tracer))
 
             p.SetBackground()
             pid = p.StartProcess(trace.Fork)
@@ -407,8 +408,7 @@ def RunPipeline(self, node, status_out):
         # type: (command.Pipeline, CommandStatus) -> None
 
         pi = process.Pipeline(self.exec_opts.sigpipe_status_ok(),
-                              self.job_control, self.job_list)
-        #self.job_list.AddPipeline(pi)
+                              self.job_control, self.job_list, self.tracer)
 
         # initialized with CommandStatus.CreateNull()
         pipe_locs = []  # type: List[loc_t]
@@ -442,7 +442,7 @@ def RunSubshell(self, node):
         # type: (command_t) -> int
         p = self._MakeProcess(node)
         if self.job_control.Enabled():
-            p.AddStateChange(process.SetPgid(process.OWN_LEADER))
+            p.AddStateChange(process.SetPgid(process.OWN_LEADER, self.tracer))
 
         return p.RunProcess(self.waiter, trace.ForkWait)
 
@@ -609,7 +609,7 @@ def RunProcessSub(self, cs_part):
         p.AddStateChange(redir)
 
         if self.job_control.Enabled():
-            p.AddStateChange(process.SetPgid(process.OWN_LEADER))
+            p.AddStateChange(process.SetPgid(process.OWN_LEADER, self.tracer))
 
         # Fork, letting the child inherit the pipe file descriptors.
         p.StartProcess(trace.ProcessSub)

core/process.py

@@ -624,25 +624,26 @@ def Apply(self):
 
 class SetPgid(ChildStateChange):
 
-    def __init__(self, pgid):
-        # type: (int) -> None
+    def __init__(self, pgid, tracer):
+        # type: (int, dev.Tracer) -> None
         self.pgid = pgid
+        self.tracer = tracer
 
     def Apply(self):
         # type: () -> None
         try:
             posix.setpgid(0, self.pgid)
         except (IOError, OSError) as e:
-            print_stderr(
-                'osh: child failed to set process group for PID %d to %d: %s' %
+            self.tracer.OtherMessage(
+                'osh: child %d failed to set its process group to %d: %s' %
                 (posix.getpid(), self.pgid, pyutil.strerror(e)))
 
     def ApplyFromParent(self, proc):
         # type: (Process) -> None
         try:
             posix.setpgid(proc.pid, self.pgid)
         except (IOError, OSError) as e:
-            print_stderr(
+            self.tracer.OtherMessage(
                 'osh: parent failed to set process group for PID %d to %d: %s'
                 % (proc.pid, self.pgid, pyutil.strerror(e)))
 
@@ -1186,11 +1187,13 @@ class Pipeline(Job):
     foo | bar | read v
     """
 
-    def __init__(self, sigpipe_status_ok, job_control, job_list):
-        # type: (bool, JobControl, JobList) -> None
+    def __init__(self, sigpipe_status_ok, job_control, job_list, tracer):
+        # type: (bool, JobControl, JobList, dev.Tracer) -> None
         Job.__init__(self)
         self.job_control = job_control
         self.job_list = job_list
+        self.tracer = tracer
+
         self.procs = []  # type: List[Process]
         self.pids = []  # type: List[int]  # pids in order
         self.pipe_status = []  # type: List[int]  # status in order
@@ -1282,7 +1285,7 @@ def StartPipeline(self, waiter):
 
         for i, proc in enumerate(self.procs):
             if self.pgid != INVALID_PGID:
-                proc.AddStateChange(SetPgid(self.pgid))
+                proc.AddStateChange(SetPgid(self.pgid, self.tracer))
 
             # Figure out the pid
             pid = proc.StartProcess(trace.PipelinePart)

core/util.py

@@ -89,6 +89,11 @@ def __init__(self):
 
 
 class DebugFile(_DebugFile):
+    """Trivial wrapper with writeln() method
+
+    Can we turn this into a plain mylib::File?  Right now that type only exists
+    in C++, but it should probably exist in Python too.
+    """
 
     def __init__(self, f):
         # type: (mylib.Writer) -> None

mycpp/mylib.py

@@ -64,6 +64,21 @@ def print_stderr(s):
     print(s, file=sys.stderr)
 
 
+class File:
+    """
+    TODO: This should define a read/write interface, and then LineReader() and
+    Writer() can possibly inherit it, with runtime assertions
+
+    Then we allow downcasting from File -> LineReader, like we currently do in
+    C++ in gc_mylib.h.
+
+    Inheritance can't express the structural Reader/Writer pattern of Go, which
+    would be better.  I suppose we could use File* everywhere, but having
+    fine-grained types is nicer.  And there will be very few casts.
+    """
+    pass
+
+
 class LineReader:
 
     def readline(self):