Merge pull request #4170 from dmoody256/ninja_exit_daemon

[NINJA] Added new alias 'shutdown-ninja-scons-daemon' to allow ninja to shutdown the daemon

11 files changed, 162 insertions, 9 deletions

diff --git a/.github/workflows/experimental_tests.yml b/.github/workflows/experimental_tests.yml
index 3672144..52078d5 100644
--- a/.github/workflows/experimental_tests.yml
+++ b/.github/workflows/experimental_tests.yml
@@ -43,8 +43,7 @@ jobs:
       - name: Install dependencies including ninja ${{ matrix.os }}
         run: |
           python -m pip install --upgrade pip setuptools wheel
-          python -m pip install ninja
-          # if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
+          python -m pip install ninja psutil
           # sudo apt-get update
       - name: Test experimental packages ${{ matrix.os }}
         run: |
diff --git a/.github/workflows/runtest.yml b/.github/workflows/runtest.yml
index cfc585e..dd18013 100644
--- a/.github/workflows/runtest.yml
+++ b/.github/workflows/runtest.yml
@@ -37,8 +37,7 @@ jobs:
       - name: Install dependencies including ninja ${{ matrix.os }}
         run: |
           python -m pip install --upgrade pip setuptools wheel
-          python -m pip install ninja
-          # if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
+          python -m pip install ninja psutil
           # sudo apt-get update
       - name: runtest ${{ matrix.os }}
         run: |
diff --git a/CHANGES.txt b/CHANGES.txt
index a9006a5..3ff8c30 100755
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -135,10 +135,12 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       Note that these are called for every build command run by SCons. It could have considerable
       performance impact if not used carefully.
       to connect to the server during start up.
+    - Ninja: Added new alias "shutdown-ninja-scons-daemon" to allow ninja to shutdown the daemon.
+      Also added cleanup to test framework to kill ninja scons daemons and clean ip daemon logs.
+      NOTE: Test for this requires python psutil module. It will be skipped if not present.
     - Ninja: Added command line variable NINJA_CMD_ARGS that allows to pass through ninja command line args.
       This can also be set in your Environment().
 
-
   From Mats Wichmann:
     - Tweak the way default site_scons paths on Windows are expressed to
       conform to conventions (what they actually resolve to is unchanged),
diff --git a/RELEASE.txt b/RELEASE.txt
index c2bd2b4..a8c1d48 100755
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -25,6 +25,9 @@ NEW FUNCTIONALITY
   performance impact if not used carefully.
 - Added MSVC_USE_SETTINGS variable to pass a dictionary to configure the msvc compiler
   system environment as an alternative to bypassing Visual Studio autodetection entirely.
+- Ninja: Added new alias "shutdown-ninja-scons-daemon" to allow ninja to shutdown the daemon.
+  Also added cleanup to test framework to kill ninja scons daemons and clean ip daemon logs.
+  NOTE: Test for this requires python psutil module. It will be skipped if not present.
 - Ninja: Added command line variable NINJA_CMD_ARGS that allows to pass through ninja command line args.
   This can also be set in your Environment().
 - Added a global policy setting and an environment policy variable for specifying the action to
diff --git a/SCons/Tool/ninja/NinjaState.py b/SCons/Tool/ninja/NinjaState.py
index c168c7f..553d8cc 100644
--- a/SCons/Tool/ninja/NinjaState.py
+++ b/SCons/Tool/ninja/NinjaState.py
@@ -214,6 +214,12 @@ class NinjaState:
                 "pool": "local_pool",
                 "restat": 1
             },
+            "EXIT_SCONS_DAEMON": {
+                "command": "$PYTHON_BIN $NINJA_TOOL_DIR/ninja_daemon_build.py $PORT $NINJA_DIR_PATH --exit",
+                "description": "Shutting down ninja scons daemon server",
+                "pool": "local_pool",
+                "restat": 1
+            },
             "SCONS": {
                 "command": "$SCONS_INVOCATION $out",
                 "description": "$SCONS_INVOCATION $out",
@@ -610,6 +616,11 @@ class NinjaState:
             rule="SCONS_DAEMON",
         )
 
+        ninja.build(
+            "shutdown_ninja_scons_daemon_phony",
+            rule="EXIT_SCONS_DAEMON",
+        )
+
 
         if all_targets is None:
             # Look in SCons's list of DEFAULT_TARGETS, find the ones that
diff --git a/SCons/Tool/ninja/__init__.py b/SCons/Tool/ninja/__init__.py
index ca659dc..7f1b9e8 100644
--- a/SCons/Tool/ninja/__init__.py
+++ b/SCons/Tool/ninja/__init__.py
@@ -465,7 +465,6 @@ def generate(env):
     # date-ness.
     SCons.Script.Main.BuildTask.needs_execute = lambda x: True
 
-
     def ninja_Set_Default_Targets(env, tlist):
         """
             Record the default targets if they were ever set by the user. Ninja
@@ -501,5 +500,5 @@ def generate(env):
     env['TEMPFILEDIR'] = "$NINJA_DIR/.response_files"
     env["TEMPFILE"] = NinjaNoResponseFiles
 
-
     env.Alias('run-ninja-scons-daemon', 'run_ninja_scons_daemon_phony')
+    env.Alias('shutdown-ninja-scons-daemon', 'shutdown_ninja_scons_daemon_phony')
diff --git a/SCons/Tool/ninja/ninja_daemon_build.py b/SCons/Tool/ninja/ninja_daemon_build.py
index 48156f5..32c375d 100644
--- a/SCons/Tool/ninja/ninja_daemon_build.py
+++ b/SCons/Tool/ninja/ninja_daemon_build.py
@@ -54,17 +54,30 @@ logging.basicConfig(
     level=logging.DEBUG,
 )
 
+
 def log_error(msg):
     logging.debug(msg)
     sys.stderr.write(msg)
 
+
 while True:
     try:
+        if not os.path.exists(daemon_dir / "pidfile"):
+            if sys.argv[3] != '--exit':
+                logging.debug(f"ERROR: Server pid not found {daemon_dir / 'pidfile'} for request {sys.argv[3]}")
+                exit(1)
+            else:
+                logging.debug("WARNING: Unnecessary request to shutdown server, it's already shutdown.")
+                exit(0)
+
         logging.debug(f"Sending request: {sys.argv[3]}")
         conn = http.client.HTTPConnection(
             "127.0.0.1", port=int(sys.argv[1]), timeout=60
         )
-        conn.request("GET", "/?build=" + sys.argv[3])
+        if sys.argv[3] == '--exit':
+            conn.request("GET", "/?exit=1")
+        else:
+            conn.request("GET", "/?build=" + sys.argv[3])
         response = None
 
         while not response:
@@ -81,6 +94,7 @@ while True:
                 if status != 200:
                     log_error(msg.decode("utf-8"))
                     exit(1)
+                    
                 logging.debug(f"Request Done: {sys.argv[3]}")
                 exit(0)
 
diff --git a/SCons/Tool/ninja/ninja_scons_daemon.py b/SCons/Tool/ninja/ninja_scons_daemon.py
index c4a1d11..6802af2 100644
--- a/SCons/Tool/ninja/ninja_scons_daemon.py
+++ b/SCons/Tool/ninja/ninja_scons_daemon.py
@@ -168,6 +168,7 @@ def daemon_thread_func():
         te.start()
 
         daemon_ready = False
+        
         building_node = None
         startup_complete = False
 
@@ -218,12 +219,14 @@ def daemon_thread_func():
                 except queue.Empty:
                     break
                 if "exit" in building_node:
+                    daemon_log("input: " + "exit")
                     p.stdin.write("exit\n".encode("utf-8"))
                     p.stdin.flush()
                     with building_cv:
                         shared_state.finished_building += [building_node]
                     daemon_ready = False
-                    raise
+                    shared_state.daemon_needs_to_shutdown = True
+                    break
 
                 else:
                     input_command = "build " + building_node + "\n"
diff --git a/requirements.txt b/requirements.txt
index 1be070c..e05c610 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -14,4 +14,5 @@ rst2pdf
 ninja
 
 # Needed for test/Parallel/failed-build/failed-build.py
+# Also for test/ninja/shutdown_scons_daemon.py
 psutil
diff --git a/test/ninja/shutdown_scons_daemon.py b/test/ninja/shutdown_scons_daemon.py
new file mode 100644
index 0000000..64ec2c7
--- /dev/null
+++ b/test/ninja/shutdown_scons_daemon.py
@@ -0,0 +1,84 @@
+#!/usr/bin/env python
+#
+# Copyright The SCons Foundation
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
+# KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+# WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+#
+
+import os
+from timeit import default_timer as timer
+
+import TestSCons
+from TestCmd import IS_WINDOWS
+
+test = TestSCons.TestSCons()
+
+try:
+    import ninja
+except ImportError:
+    test.skip_test("Could not find ninja module in python")
+
+try:
+    import psutil
+except ImportError:
+    test.skip_test("Could not find psutil module in python")
+
+
+_python_ = TestSCons._python_
+_exe = TestSCons._exe
+
+ninja_bin = os.path.abspath(
+    os.path.join(ninja.__file__, os.pardir, "data", "bin", "ninja" + _exe)
+)
+
+test.dir_fixture("ninja-fixture")
+
+test.file_fixture(
+    "ninja_test_sconscripts/sconstruct_force_scons_callback", "SConstruct"
+)
+
+test.run(stdout=None)
+pid = None
+test.must_exist(test.workpath('.ninja/scons_daemon_dirty'))
+with open(test.workpath('.ninja/scons_daemon_dirty')) as f:
+    pid = int(f.read())
+    if pid not in [proc.pid for proc in psutil.process_iter()]:
+        test.fail_test(message="daemon not running!")
+
+program = test.workpath("run_ninja_env.bat") if IS_WINDOWS else ninja_bin
+test.run(program=program, arguments='shutdown-ninja-scons-daemon', stdout=None)
+
+wait_time = 10
+start_time = timer()
+while True:
+    if wait_time > (timer() - start_time):
+        if pid not in [proc.pid for proc in psutil.process_iter()]:
+            break
+    else:
+        test.fail_test(message=f"daemon still not shutdown after {wait_time} seconds.")
+
+test.must_not_exist(test.workpath('.ninja/scons_daemon_dirty'))
+test.pass_test()
+
+# Local Variables:
+# tab-width:4
+# indent-tabs-mode:nil
+# End:
+# vim: set expandtab tabstop=4 shiftwidth=4:
diff --git a/testing/framework/TestCmd.py b/testing/framework/TestCmd.py
index a70df0f..fc5e328 100644
--- a/testing/framework/TestCmd.py
+++ b/testing/framework/TestCmd.py
@@ -299,9 +299,12 @@ __version__ = "1.3"
 import atexit
 import difflib
 import errno
+import hashlib
 import os
 import re
+import psutil
 import shutil
+import signal
 import stat
 import subprocess
 import sys
@@ -310,6 +313,7 @@ import threading
 import time
 import traceback
 from collections import UserList, UserString
+from pathlib import Path
 from subprocess import PIPE, STDOUT
 from typing import Optional
 
@@ -383,6 +387,37 @@ def _caller(tblist, skip):
     return string
 
 
+def clean_up_ninja_daemon(self, result_type):
+    """
+    Kill any running scons daemon started by ninja and clean up it's working dir and
+    temp files.
+    """
+    if self:
+        for path in Path(self.workdir).rglob('.ninja'):
+            daemon_dir = Path(tempfile.gettempdir()) / (
+                "scons_daemon_" + str(hashlib.md5(str(path.resolve()).encode()).hexdigest())
+            )
+            pidfiles = [daemon_dir / 'pidfile', path / 'scons_daemon_dirty']
+            for pidfile in pidfiles:
+                if pidfile.exists():
+                    with open(pidfile) as f:
+                        try:
+                            pid = int(f.read())
+                            os.kill(pid, signal.SIGINT)
+                        except OSError:
+                            pass
+                    
+                        while True:
+                            if pid not in [proc.pid for proc in psutil.process_iter()]:
+                                break
+                            else:
+                                time.sleep(0.1)
+                
+            if not self._preserve[result_type]:
+                if daemon_dir.exists():
+                    shutil.rmtree(daemon_dir)
+
+
 def fail_test(self=None, condition=True, function=None, skip=0, message=None):
     """Causes a test to exit with a fail.
 
@@ -402,6 +437,7 @@ def fail_test(self=None, condition=True, function=None, skip=0, message=None):
         return
     if function is not None:
         function()
+    clean_up_ninja_daemon(self, 'fail_test')
     of = ""
     desc = ""
     sep = " "
@@ -447,6 +483,7 @@ def no_result(self=None, condition=True, function=None, skip=0):
         return
     if function is not None:
         function()
+    clean_up_ninja_daemon(self, 'no_result')
     of = ""
     desc = ""
     sep = " "
@@ -483,6 +520,7 @@ def pass_test(self=None, condition=True, function=None):
         return
     if function is not None:
         function()
+    clean_up_ninja_daemon(self, 'pass_test')
     sys.stderr.write("PASSED\n")
     sys.exit(0)