Added option to allow scons to determine if it should skip ninja regeneration.

4 files changed, 112 insertions, 46 deletions

diff --git a/SCons/Script/SConsOptions.py b/SCons/Script/SConsOptions.py
index e2631fb..0210b79 100644
--- a/SCons/Script/SConsOptions.py
+++ b/SCons/Script/SConsOptions.py
@@ -151,7 +151,8 @@ class SConsValues(optparse.Values):
         # Requested setable flag in : https://github.com/SCons/scons/issues/3983
         # From experimental ninja
         'disable_execute_ninja',
-        'disable_ninja'
+        'disable_ninja',
+        'skip_ninja_regen'
     ]
 
     def set_option(self, name, value):
diff --git a/SCons/Tool/ninja/NinjaState.py b/SCons/Tool/ninja/NinjaState.py
index c168c7f..c75c966 100644
--- a/SCons/Tool/ninja/NinjaState.py
+++ b/SCons/Tool/ninja/NinjaState.py
@@ -29,6 +29,8 @@ import signal
 import tempfile
 import shutil
 import sys
+import random
+import filecmp
 from os.path import splitext
 from tempfile import NamedTemporaryFile
 import ninja
@@ -42,7 +44,7 @@ from .Globals import COMMAND_TYPES, NINJA_RULES, NINJA_POOLS, \
     NINJA_CUSTOM_HANDLERS, NINJA_DEFAULT_TARGETS
 from .Rules import _install_action_function, _mkdir_action_function, _lib_symlink_action_function, _copy_action_function
 from .Utils import get_path, alias_to_ninja_build, generate_depfile, ninja_noop, get_order_only, \
-    get_outputs, get_inputs, get_dependencies, get_rule, get_command_env, to_escaped_list
+    get_outputs, get_inputs, get_dependencies, get_rule, get_command_env, to_escaped_list, ninja_sorted_build
 from .Methods import get_command
 
 
@@ -82,7 +84,26 @@ class NinjaState:
         # to make the SCONS_INVOCATION variable properly quoted for things
         # like CCFLAGS
         scons_escape = env.get("ESCAPE", lambda x: x)
-        scons_daemon_port = int(env.get('NINJA_SCONS_DAEMON_PORT',-1))
+
+        # The daemon port should be the same across runs, unless explicitly set
+        # or if the portfile is deleted. This ensures the ninja file is deterministic
+        # across regen's if nothings changed. The construction var should take preference,
+        # then portfile is next, and then otherwise create a new random port to persist in
+        # use.
+        scons_daemon_port = None
+        os.makedirs(get_path(self.env.get("NINJA_DIR")), exist_ok=True)
+        scons_daemon_port_file = str(pathlib.Path(get_path(self.env.get("NINJA_DIR"))) / "scons_daemon_portfile")
+
+        if env.get('NINJA_SCONS_DAEMON_PORT') is not None:
+            scons_daemon_port = int(env.get('NINJA_SCONS_DAEMON_PORT'))
+        elif os.path.exists(scons_daemon_port_file):
+            with open(scons_daemon_port_file) as f:
+                scons_daemon_port = int(f.read())
+        else:
+            scons_daemon_port = random.randint(10000, 60000)
+
+        with open(scons_daemon_port_file, 'w') as f:
+            f.write(str(scons_daemon_port))
 
         # if SCons was invoked from python, we expect the first arg to be the scons.py
         # script, otherwise scons was invoked from the scons script
@@ -381,13 +402,13 @@ class NinjaState:
 
         ninja.variable("builddir", get_path(self.env.Dir(self.env['NINJA_DIR']).path))
 
-        for pool_name, size in self.pools.items():
+        for pool_name, size in sorted(self.pools.items()):
             ninja.pool(pool_name, min(self.env.get('NINJA_MAX_JOBS', size), size))
 
-        for var, val in self.variables.items():
+        for var, val in sorted(self.variables.items()):
             ninja.variable(var, val)
 
-        for rule, kwargs in self.rules.items():
+        for rule, kwargs in sorted(self.rules.items()):
             if self.env.get('NINJA_MAX_JOBS') is not None and 'pool' not in kwargs:
                 kwargs['pool'] = 'local_pool'
             ninja.rule(rule, **kwargs)
@@ -529,8 +550,9 @@ class NinjaState:
                 )
 
                 if remaining_outputs:
-                    ninja.build(
-                        outputs=sorted(remaining_outputs), rule="phony", implicit=first_output,
+                    ninja_sorted_build(
+                        ninja,
+                        outputs=remaining_outputs, rule="phony", implicit=first_output,
                     )
 
                 build["outputs"] = first_output
@@ -548,12 +570,18 @@ class NinjaState:
             if "inputs" in build:
                 build["inputs"].sort()
 
-            ninja.build(**build)
+            ninja_sorted_build(
+                ninja,
+                **build
+            )
 
         scons_daemon_dirty = str(pathlib.Path(get_path(self.env.get("NINJA_DIR"))) / "scons_daemon_dirty")
         for template_builder in template_builders:
             template_builder["implicit"] += [scons_daemon_dirty]
-            ninja.build(**template_builder)
+            ninja_sorted_build(
+                ninja,
+                **template_builder
+            )
 
         # We have to glob the SCons files here to teach the ninja file
         # how to regenerate itself. We'll never see ourselves in the
@@ -563,17 +591,19 @@ class NinjaState:
         ninja_file_path = self.env.File(self.ninja_file).path
         regenerate_deps = to_escaped_list(self.env, self.env['NINJA_REGENERATE_DEPS'])
 
-        ninja.build(
-            ninja_file_path,
+        ninja_sorted_build(
+            ninja,
+            outputs=ninja_file_path,
             rule="REGENERATE",
             implicit=regenerate_deps,
             variables={
-                "self": ninja_file_path,
+                "self": ninja_file_path
             }
         )
 
-        ninja.build(
-            regenerate_deps,
+        ninja_sorted_build(
+            ninja,
+            outputs=regenerate_deps,
             rule="phony",
             variables={
                 "self": ninja_file_path,
@@ -584,8 +614,9 @@ class NinjaState:
             # If we ever change the name/s of the rules that include
             # compile commands (i.e. something like CC) we will need to
             # update this build to reflect that complete list.
-            ninja.build(
-                "compile_commands.json",
+            ninja_sorted_build(
+                ninja,
+                outputs="compile_commands.json",
                 rule="CMD",
                 pool="console",
                 implicit=[str(self.ninja_file)],
@@ -601,12 +632,14 @@ class NinjaState:
                 },
             )
 
-            ninja.build(
-                "compiledb", rule="phony", implicit=["compile_commands.json"],
+            ninja_sorted_build(
+                ninja,
+                outputs="compiledb", rule="phony", implicit=["compile_commands.json"],
             )
 
-        ninja.build(
-            ["run_ninja_scons_daemon_phony", scons_daemon_dirty],
+        ninja_sorted_build(
+            ninja,
+            outputs=["run_ninja_scons_daemon_phony", scons_daemon_dirty],
             rule="SCONS_DAEMON",
         )
 
@@ -620,39 +653,45 @@ class NinjaState:
         
         if len(all_targets) == 0:
             all_targets = ["phony_default"]
-            ninja.build(
+            ninja_sorted_build(
+                ninja,
                 outputs=all_targets,
                 rule="phony",
             )
         
         ninja.default([self.ninja_syntax.escape_path(path) for path in sorted(all_targets)])
 
-        daemon_dir = pathlib.Path(tempfile.gettempdir()) / ('scons_daemon_' + str(hashlib.md5(str(get_path(self.env["NINJA_DIR"])).encode()).hexdigest()))
-        pidfile = None
-        if os.path.exists(scons_daemon_dirty):
-            pidfile = scons_daemon_dirty
-        elif os.path.exists(daemon_dir / 'pidfile'):
-            pidfile = daemon_dir / 'pidfile'
-
-        if pidfile:
-            with open(pidfile) as f:
-                pid = int(f.readline())
-                try:
-                    os.kill(pid, signal.SIGINT)
-                except OSError:
-                    pass
+        with NamedTemporaryFile(delete=False, mode='w') as temp_ninja_file:
+            temp_ninja_file.write(content.getvalue())
+
+        if self.env.GetOption('skip_ninja_regen') and os.path.exists(ninja_file_path) and filecmp.cmp(temp_ninja_file.name, ninja_file_path):
+            os.unlink(temp_ninja_file.name)
+        else:
+
+            daemon_dir = pathlib.Path(tempfile.gettempdir()) / ('scons_daemon_' + str(hashlib.md5(str(get_path(self.env["NINJA_DIR"])).encode()).hexdigest()))
+            pidfile = None
+            if os.path.exists(scons_daemon_dirty):
+                pidfile = scons_daemon_dirty
+            elif os.path.exists(daemon_dir / 'pidfile'):
+                pidfile = daemon_dir / 'pidfile'
+
+            if pidfile:
+                with open(pidfile) as f:
+                    pid = int(f.readline())
+                    try:
+                        os.kill(pid, signal.SIGINT)
+                    except OSError:
+                        pass
 
                 # wait for the server process to fully killed
                 # TODO: update wait_for_process_to_die() to handle timeout and then catch exception
                 #       here and do something smart.
                 wait_for_process_to_die(pid)
 
-        if os.path.exists(scons_daemon_dirty):
-            os.unlink(scons_daemon_dirty)
+            if os.path.exists(scons_daemon_dirty):
+                os.unlink(scons_daemon_dirty)
 
-        with NamedTemporaryFile(delete=False, mode='w') as temp_ninja_file:
-            temp_ninja_file.write(content.getvalue())
-        shutil.move(temp_ninja_file.name, ninja_file_path)
+            shutil.move(temp_ninja_file.name, ninja_file_path)
 
         self.__generated = True
 
diff --git a/SCons/Tool/ninja/Utils.py b/SCons/Tool/ninja/Utils.py
index b2c3cd3..dec6d2c 100644
--- a/SCons/Tool/ninja/Utils.py
+++ b/SCons/Tool/ninja/Utils.py
@@ -23,6 +23,7 @@
 import os
 import shutil
 from os.path import join as joinpath
+from collections import OrderedDict
 
 import SCons
 from SCons.Action import get_default_ENV, _string_from_cmd_list
@@ -52,6 +53,15 @@ def ninja_add_command_line_options():
               help='Disable ninja generation and build with scons even if tool is loaded. '+
                    'Also used by ninja to build targets which only scons can build.')
 
+    AddOption('--skip-ninja-regen',
+              dest='skip_ninja_regen',
+              metavar='BOOL',
+              action="store_true",
+              default=False,
+              help='Allow scons to skip regeneration of the ninja file and restarting of the daemon. ' +
+                    'Care should be taken in cases where Glob is in use or generated files are used in ' + 
+                    'command lines.')
+
 
 def is_valid_dependent_node(node):
     """
@@ -126,7 +136,7 @@ def get_dependencies(node, skip_sources=False):
             get_path(src_file(child))
             for child in filter_ninja_nodes(node.children())
             if child not in node.sources
-        ]    
+        ]
     return [get_path(src_file(child)) for child in filter_ninja_nodes(node.children())]
 
 
@@ -261,6 +271,19 @@ def ninja_noop(*_args, **_kwargs):
     """
     return None
 
+def ninja_recursive_sorted_dict(build):
+    sorted_dict = OrderedDict()
+    for key, val in sorted(build.items()):
+        if isinstance(val, dict):
+            sorted_dict[key] = ninja_recursive_sorted_dict(val)
+        else:
+            sorted_dict[key] = val
+    return sorted_dict
+
+
+def ninja_sorted_build(ninja, **build):
+    sorted_dict = ninja_recursive_sorted_dict(build)
+    ninja.build(**sorted_dict)
 
 def get_command_env(env, target, source):
     """
diff --git a/SCons/Tool/ninja/__init__.py b/SCons/Tool/ninja/__init__.py
index 04a7abb..ad75978 100644
--- a/SCons/Tool/ninja/__init__.py
+++ b/SCons/Tool/ninja/__init__.py
@@ -26,7 +26,7 @@
 
 import importlib
 import os
-import random
+import traceback
 import subprocess
 import sys
 
@@ -66,8 +66,12 @@ def ninja_builder(env, target, source):
     print("Generating:", str(target[0]))
 
     generated_build_ninja = target[0].get_abspath()
-    NINJA_STATE.generate()
-
+    try:
+        NINJA_STATE.generate()
+    except Exception as e:
+        raise SCons.Errors.BuildError(
+            errstr=f"ERROR: an excetion occurred while generating the ninja file:\n{traceback.format_exc()}",
+            node=target)
     if env["PLATFORM"] == "win32":
         # TODO: Is this necessary as you set env variable in the ninja build file per target?
         # this is not great, its doesn't consider specific
@@ -194,7 +198,6 @@ def generate(env):
     env["NINJA_ALIAS_NAME"] = env.get("NINJA_ALIAS_NAME", "generate-ninja")
     env['NINJA_DIR'] = env.Dir(env.get("NINJA_DIR", '#/.ninja'))
     env["NINJA_SCONS_DAEMON_KEEP_ALIVE"] = env.get("NINJA_SCONS_DAEMON_KEEP_ALIVE", 180000)
-    env["NINJA_SCONS_DAEMON_PORT"] = env.get('NINJA_SCONS_DAEMON_PORT', random.randint(10000, 60000))
 
     if GetOption("disable_ninja"):
         env.SConsignFile(os.path.join(str(env['NINJA_DIR']),'.ninja.sconsign'))