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

CHANGES.txt

@@ -135,6 +135,10 @@ 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 option "--skip-ninja-regen" to enable skipping regeneration of the ninja file
+      if scons can determine the ninja file doesnot need to be regenerated, which will also
+      skip restarting the scons daemon. Note this option is could result in incorrect rebuilds
+      if scons Glob or scons generated files are used in ninja build target's command lines.
     - 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.

RELEASE.txt

@@ -107,7 +107,10 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
         require delayed expansion to be enabled which is currently not supported and is
         typically not enabled by default on the host system. The batch files may also require
         environment variables that are not included by default in the msvc environment.
-
+- Ninja: added option "--skip-ninja-regen" to enable skipping regeneration of the ninja file
+  if scons can determine the ninja file doesnot need to be regenerated, which will also
+  skip restarting the scons daemon. Note this option is could result in incorrect rebuilds
+  if scons Glob or scons generated files are used in ninja build target's command lines.
 FIXES
 -----
 

SCons/Script/SConsOptions.py

@@ -147,11 +147,12 @@ def __getattr__(self, attr):
         'warn',
 
         # TODO: Remove these once we update the AddOption() API to allow setting
-        #       added flag as setable.
-        # Requested setable flag in : https://github.com/SCons/scons/issues/3983
+        #       added flag as settable.
+        # Requested settable 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):

SCons/Tool/ninja/NinjaState.py

@@ -29,6 +29,8 @@
 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 @@
     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 @@ def __init__(self, env, ninja_file, ninja_syntax):
         # 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
@@ -387,13 +408,13 @@ def generate(self):
 
         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)
@@ -535,8 +556,9 @@ def check_generated_source_deps(build):
                 )
 
                 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
@@ -554,12 +576,18 @@ def check_generated_source_deps(build):
             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
@@ -569,17 +597,19 @@ def check_generated_source_deps(build):
         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,
@@ -590,8 +620,9 @@ def check_generated_source_deps(build):
             # 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)],
@@ -607,12 +638,14 @@ def check_generated_source_deps(build):
                 },
             )
 
-            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",
         )
 
@@ -631,39 +664,45 @@ def check_generated_source_deps(build):
 
         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
 

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 SCons 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):
     """

SCons/Tool/ninja/__init__.py

@@ -26,7 +26,7 @@
 
 import importlib
 import os
-import random
+import traceback
 import subprocess
 import sys
 
@@ -66,7 +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:
+        raise SCons.Errors.BuildError(
+            errstr=f"ERROR: an exception 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?
@@ -202,7 +207,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'))