[core] Add Job::ProcessGroupId() API and unit test it

This is after the last change, which removed getpgid() calls.

We shouldn't access the Pipeline.pgid field directly.

builtin/process_osh.py

@@ -79,6 +79,9 @@ def Run(self, cmd_val):
             return 1
 
         pgid = job.ProcessGroupId()
+        assert pgid != process.INVALID_PGID, \
+            'Processes put in the background should have a PGID'
+
         # TODO: Print job ID rather than the PID
         log('Continue PID %d', pgid)
         # Put the job's process group back into the foreground. GiveTerminal() must

core/executor.py

@@ -333,9 +333,11 @@ def RunSimpleCommand(self, cmd_val, cmd_st, run_flags):
 
             if self.job_control.Enabled():
                 if self.fg_pipeline is not None:
+                    pgid = self.fg_pipeline.ProcessGroupId()
                     # If job control is enabled, this should be true
-                    assert self.fg_pipeline.pgid != process.INVALID_PGID
-                    change = process.SetPgid(self.fg_pipeline.pgid)
+                    assert pgid != process.INVALID_PGID
+
+                    change = process.SetPgid(pgid)
                     self.fg_pipeline = None  # clear to avoid confusion in subshells
                 else:
                     change = process.SetPgid(process.OWN_LEADER)

core/process.py

@@ -839,7 +839,7 @@ def Run(self):
             print('')
             status = 130  # 128 + 2
         except (IOError, OSError) as e:
-            print_stderr('osh I/O error (subprogram): %s' % pyutil.strerror(e))
+            print_stderr('oils I/O error (subprogram): %s' % pyutil.strerror(e))
             status = 2
 
         # If ProcessInit() doesn't turn off buffering, this is needed before
@@ -948,10 +948,10 @@ class Process(Job):
     def __init__(self, thunk, job_control, job_list, tracer):
         # type: (Thunk, JobControl, JobList, dev.Tracer) -> None
         """
-    Args:
-      thunk: Thunk instance
-      job_list: for process bookkeeping
-    """
+        Args:
+          thunk: Thunk instance
+          job_list: for process bookkeeping
+        """
         Job.__init__(self)
         assert isinstance(thunk, Thunk), thunk
         self.thunk = thunk
@@ -1206,12 +1206,7 @@ def __init__(self, sigpipe_status_ok, job_control, job_list):
     def ProcessGroupId(self):
         # type: () -> int
         """Returns the group ID of this pipeline."""
-
-        # TODO: return self.pgid
-
-        # This should only ever be called AFTER the pipeline has started
-        assert len(self.pids) > 0
-        return self.pids[0]  # First process is the group leader
+        return self.pgid
 
     def DisplayJob(self, job_id, f, style):
         # type: (int, mylib.Writer, int) -> None
@@ -1615,7 +1610,6 @@ def RemoveChildProcess(self, pid):
         mylib.dict_erase(self.child_procs, pid)
 
     if mylib.PYTHON:
-
         def AddPipeline(self, pi):
             # type: (Pipeline) -> None
             """For debugging only."""

core/process_test.py

@@ -35,6 +35,14 @@ def _CommandNode(code_str, arena):
     return c_parser.ParseLogicalLine()
 
 
+class FakeJobControl(object):
+    def __init__(self, enabled):
+        self.enabled = enabled
+
+    def Enabled(self):
+        return self.enabled
+
+
 class ProcessTest(unittest.TestCase):
 
     def setUp(self):
@@ -196,6 +204,42 @@ def testPipeline2(self):
         # Or technically we could fork the whole interpreter for foo|bar|baz and
         # capture stdout of that interpreter.
 
+    def makeTestPipeline(self, jc):
+        cmd_ev = test_lib.InitCommandEvaluator(arena=self.arena,
+                                               ext_prog=self.ext_prog)
+
+        pi = process.Pipeline(False, jc, self.job_list)
+
+        node1 = _CommandNode('/bin/echo testpipeline', self.arena)
+        node2 = _CommandNode('cat', self.arena)
+
+        thunk1 = process.SubProgramThunk(cmd_ev, node1, self.trap_state)
+        thunk2 = process.SubProgramThunk(cmd_ev, node2, self.trap_state)
+
+        pi.Add(Process(thunk1, jc, self.job_list, self.tracer))
+        pi.Add(Process(thunk2, jc, self.job_list, self.tracer))
+
+        return pi
+
+    def testPipelinePgidField(self):
+        jc = FakeJobControl(False)
+
+        pi = self.makeTestPipeline(jc)
+        self.assertEqual(process.INVALID_PGID, pi.ProcessGroupId())
+
+        pi.StartPipeline(self.waiter)
+        # No pgid
+        self.assertEqual(process.INVALID_PGID, pi.ProcessGroupId())
+
+        jc = FakeJobControl(True)
+
+        pi = self.makeTestPipeline(jc)
+        self.assertEqual(process.INVALID_PGID, pi.ProcessGroupId())
+
+        pi.StartPipeline(self.waiter)
+        # first process is the process group leader
+        self.assertEqual(pi.pids[0], pi.ProcessGroupId())
+
     def testOpen(self):
         # Disabled because mycpp translation can't handle it.  We do this at a
         # higher layer.
@@ -211,3 +255,5 @@ def testOpen(self):
 
 if __name__ == '__main__':
     unittest.main()
+
+# vim: sw=4