SERVER-50376 Ninja should rebuild on compiler and tool changes

2 files changed, 51 insertions, 8 deletions

diff --git a/site_scons/site_tools/next/icecream.py b/site_scons/site_tools/next/icecream.py
index b1fb49543b5..7456ed0cc8f 100644
--- a/site_scons/site_tools/next/icecream.py
+++ b/site_scons/site_tools/next/icecream.py
@@ -283,7 +283,7 @@ def generate(env):
         )
     ))
 
-    # We can't allow these to interact with the cache beacuse the
+    # We can't allow these to interact with the cache because the
     # second action produces a file unknown to SCons. If caching were
     # permitted, the other two files could be retrieved from cache but
     # the file produced by the second action could not (and would not)
@@ -368,6 +368,14 @@ def generate(env):
             # This dependency is necessary so that we build into this
             # string before we create the file.
             icecc_version_string_value,
+
+            # TODO: SERVER-50587 We need to make explicit depends here because of NINJA_SKIP. Any
+            # dependencies in the nodes created in setupEnv with NINJA_SKIP would have
+            # that dependency chain hidden from ninja, so they won't be rebuilt unless
+            # added as dependencies here on this node that has NINJA_SKIP=False.
+            '$CC',
+            '$CXX',
+            icecc_version_file,
         ],
     )
 
diff --git a/site_scons/site_tools/next/ninja.py b/site_scons/site_tools/next/ninja.py
index 0506fa658ef..6448bf428b5 100644
--- a/site_scons/site_tools/next/ninja.py
+++ b/site_scons/site_tools/next/ninja.py
@@ -996,7 +996,7 @@ def gen_get_response_file_command(env, rule, tool, tool_is_dynamic=False):
         variables[rule] = cmd
         if use_command_env:
             variables["env"] = get_command_env(env)
-        return rule, variables
+        return rule, variables, [tool_command]
 
     return get_response_file_command
 
@@ -1026,13 +1026,21 @@ def generate_command(env, node, action, targets, sources, executor=None):
     return cmd.replace("$", "$$")
 
 
-def get_shell_command(env, node, action, targets, sources, executor=None):
+def get_generic_shell_command(env, node, action, targets, sources, executor=None):
     return (
         "CMD",
         {
             "cmd": generate_command(env, node, action, targets, sources, executor=None),
             "env": get_command_env(env),
         },
+        # Since this function is a rule mapping provider, it must return a list of dependencies,
+        # and usually this would be the path to a tool, such as a compiler, used for this rule.
+        # However this function is to generic to be able to reliably extract such deps
+        # from the command, so we return a placeholder empty list. It should be noted that
+        # generally this function will not be used soley and is more like a template to generate
+        # the basics for a custom provider which may have more specific options for a provier
+        # function for a custom NinjaRuleMapping.
+        []
     )
 
 
@@ -1068,12 +1076,39 @@ def get_command(env, node, action):  # pylint: disable=too-many-branches
     if not comstr:
         return None
 
-    provider = __NINJA_RULE_MAPPING.get(comstr, get_shell_command)
-    rule, variables = provider(sub_env, node, action, tlist, slist, executor=executor)
+    provider = __NINJA_RULE_MAPPING.get(comstr, get_generic_shell_command)
+    rule, variables, provider_deps = provider(sub_env, node, action, tlist, slist, executor=executor)
 
     # Get the dependencies for all targets
     implicit = list({dep for tgt in tlist for dep in get_dependencies(tgt)})
 
+    # Now add in the other dependencies related to the command,
+    # e.g. the compiler binary. The ninja rule can be user provided so
+    # we must do some validation to resolve the dependency path for ninja.
+    for provider_dep in provider_deps:
+
+        provider_dep = sub_env.subst(provider_dep)
+        if not provider_dep:
+            continue
+
+        # If the tool is a node, then SCons will resolve the path later, if its not
+        # a node then we assume it generated from build and make sure it is existing.
+        if isinstance(provider_dep, SCons.Node.Node) or os.path.exists(provider_dep):
+            implicit.append(provider_dep)
+            continue
+
+        # Many commands will assume the binary is in the path, so
+        # we accept this as a possible input from a given command.
+        provider_dep_abspath = sub_env.WhereIs(provider_dep)
+        if provider_dep_abspath:
+            implicit.append(provider_dep_abspath)
+            continue
+
+        # Possibly these could be ignore and the build would still work, however it may not always
+        # rebuild correctly, so we hard stop, and force the user to fix the issue with the provided
+        # ninja rule.
+        raise Exception(f"Could not resolve path for {provider_dep} dependency on node '{node}'")
+
     ninja_build = {
         "order_only": get_order_only(node),
         "outputs": get_outputs(node),
@@ -1136,7 +1171,7 @@ def register_custom_handler(env, name, handler):
 
 
 def register_custom_rule_mapping(env, pre_subst_string, rule):
-    """Register a custom handler for SCons function actions."""
+    """Register a function to call for a given rule."""
     global __NINJA_RULE_MAPPING
     __NINJA_RULE_MAPPING[pre_subst_string] = rule
 
@@ -1149,7 +1184,7 @@ def register_custom_rule(env, rule, command, description="", deps=None, pool=Non
     }
 
     if use_depfile:
-        rule_obj["depfile"] = os.path.join(get_path(env['NINJA_BUILDDIR']),'$out.depfile')
+        rule_obj["depfile"] = os.path.join(get_path(env['NINJA_BUILDDIR']), '$out.depfile')
 
     if deps is not None:
         rule_obj["deps"] = deps
@@ -1344,7 +1379,7 @@ def generate(env):
 
     # Provide a way for custom rule authors to easily access command
     # generation.
-    env.AddMethod(get_shell_command, "NinjaGetShellCommand")
+    env.AddMethod(get_generic_shell_command, "NinjaGetGenericShellCommand")
     env.AddMethod(gen_get_response_file_command, "NinjaGenResponseFileProvider")
 
     # Provides a way for users to handle custom FunctionActions they