Merge pull request #4655 from bdbaddog/fix_new_ninja_package

Fixed ninja binary location logic to use ninja.BIN_DIR. 

38 files changed, 154 insertions, 196 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index e449bba..54def0a 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -85,6 +85,14 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       via top-level `from __future__ import annotations`.
     - Implemented type hints for Nodes.
 
+  From William Deegan:
+    - Update ninja tool to use ninja.BIN_DIR to find pypi packaged ninja binary.
+      python ninja package version 1.11.1.2 changed the location and previous
+      logic no longer worked.
+    - Added TestSCons.NINJA_BINARY to TestSCons to centralize logic to find ninja binary
+    - Refactored SCons.Tool.ninja -> SCons.Tool.ninja_tool, and added alias so
+      env.Tool('ninja') will still work. This avoids conflicting with the pypi module ninja.
+
   From Alex James:
     - On Darwin, PermissionErrors are now handled while trying to access
       /etc/paths.d. This may occur if SCons is invoked in a sandboxed
diff --git a/RELEASE.txt b/RELEASE.txt
index 8e88a0b..af5fb2f 100644
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -140,6 +140,11 @@ FIXES
 - Make sure unknown variables from a Variables file are recognized
   as such (issue #4645)
 
+- Update ninja tool to use ninja.BIN_DIR to find pypi packaged ninja binary.
+  python ninja package version 1.11.1.2 changed the location and previous
+  logic no longer worked.
+
+
 IMPROVEMENTS
 ------------
 
@@ -196,6 +201,11 @@ DEVELOPMENT
 
 - Implemented type hints for Nodes.
 
+- Added TestSCons.NINJA_BINARY to TestSCons to centralize logic to find ninja binary
+
+- Refactored SCons.Tool.ninja -> SCons.Tool.ninja_tool, and added alias so env.Tool('ninja')
+  will still work. This avoids conflicting with the pypi module ninja.
+
 Thanks to the following contributors listed below for their contributions to this release.
 ==========================================================================================
 .. code-block:: text
diff --git a/SCons/Tool/__init__.py b/SCons/Tool/__init__.py
index a7bc927..23b7eeb 100644
--- a/SCons/Tool/__init__.py
+++ b/SCons/Tool/__init__.py
@@ -102,6 +102,7 @@ TOOL_ALIASES = {
     'gettext': 'gettext_tool',
     'clang++': 'clangxx',
     'as': 'asm',
+    'ninja' : 'ninja_tool'
 }
 
 
diff --git a/SCons/Tool/ninja/Globals.py b/SCons/Tool/ninja_tool/Globals.py
index 6b079ef..6b079ef 100644
--- a/SCons/Tool/ninja/Globals.py
+++ b/SCons/Tool/ninja_tool/Globals.py
diff --git a/SCons/Tool/ninja/Methods.py b/SCons/Tool/ninja_tool/Methods.py
index ff006c0..01866e5 100644
--- a/SCons/Tool/ninja/Methods.py
+++ b/SCons/Tool/ninja_tool/Methods.py
@@ -30,9 +30,9 @@ from typing import TYPE_CHECKING
 
 import SCons
 from SCons.Subst import SUBST_CMD
-from SCons.Tool.ninja import NINJA_CUSTOM_HANDLERS, NINJA_RULES, NINJA_POOLS
-from SCons.Tool.ninja.Globals import __NINJA_RULE_MAPPING
-from SCons.Tool.ninja.Utils import get_targets_sources, get_dependencies, get_order_only, get_outputs, get_inputs, \
+from SCons.Tool.ninja_tool import NINJA_CUSTOM_HANDLERS, NINJA_RULES, NINJA_POOLS
+from SCons.Tool.ninja_tool.Globals import __NINJA_RULE_MAPPING
+from SCons.Tool.ninja_tool.Utils import get_targets_sources, get_dependencies, get_order_only, get_outputs, get_inputs, \
     get_rule, get_path, generate_command, get_command_env, get_comstr
 
 if TYPE_CHECKING:
@@ -46,7 +46,7 @@ def register_custom_handler(env, name, handler) -> None:
 
 def register_custom_rule_mapping(env, pre_subst_string, rule) -> None:
     """Register a function to call for a given rule."""
-    SCons.Tool.ninja.Globals.__NINJA_RULE_MAPPING[pre_subst_string] = rule
+    __NINJA_RULE_MAPPING[pre_subst_string] = rule
 
 
 def register_custom_rule(env, rule, command, description: str="", deps=None, pool=None, use_depfile: bool=False, use_response_file: bool=False, response_file_content: str="$rspc") -> None:
diff --git a/SCons/Tool/ninja/NinjaState.py b/SCons/Tool/ninja_tool/NinjaState.py
index 549af78..274331e 100644
--- a/SCons/Tool/ninja/NinjaState.py
+++ b/SCons/Tool/ninja_tool/NinjaState.py
@@ -60,12 +60,11 @@ class NinjaState:
         if not self.ninja_bin_path:
             # default to using ninja installed with python module
             ninja_bin = 'ninja.exe' if env["PLATFORM"] == "win32" else 'ninja'
+
             self.ninja_bin_path = os.path.abspath(os.path.join(
-                ninja.__file__,
-                os.pardir,
-                'data',
-                'bin',
-                ninja_bin))
+                ninja.BIN_DIR, ninja_bin
+            ))
+
             if not os.path.exists(self.ninja_bin_path):
                 # couldn't find it, just give the bin name and hope
                 # its in the path later
@@ -398,7 +397,7 @@ class NinjaState:
                 self.rules.update({key: non_rsp_rule})
             else:
                 self.rules.update({key: rule})
-        
+
         self.pools.update(self.env.get(NINJA_POOLS, {}))
 
         content = io.StringIO()
@@ -435,7 +434,7 @@ class NinjaState:
             generated_source_files = sorted(
                 [] if not generated_sources_build else generated_sources_build['implicit']
             )
-            
+
             def check_generated_source_deps(build):
                 return (
                     build != generated_sources_build
@@ -464,7 +463,7 @@ class NinjaState:
                     rule="phony",
                     implicit=generated_source_files
                 )
-                
+
                 def check_generated_source_deps(build):
                     return (
                         not build["rule"] == "INSTALL"
@@ -661,7 +660,7 @@ class NinjaState:
             all_targets = [str(node) for node in NINJA_DEFAULT_TARGETS]
         else:
             all_targets = list(all_targets)
-        
+
         if len(all_targets) == 0:
             all_targets = ["phony_default"]
             ninja_sorted_build(
@@ -669,7 +668,7 @@ class NinjaState:
                 outputs=all_targets,
                 rule="phony",
             )
-        
+
         ninja.default([self.ninja_syntax.escape_path(path) for path in sorted(all_targets)])
 
         with NamedTemporaryFile(delete=False, mode='w') as temp_ninja_file:
@@ -754,7 +753,7 @@ class SConsToNinjaTranslator:
         # Ninja builders out of being sources of ninja builders but I
         # can't fix every DAG problem so we just skip ninja_builders
         # if we find one
-        if SCons.Tool.ninja.NINJA_STATE.ninja_file == str(node):
+        if SCons.Tool.ninja_tool.NINJA_STATE.ninja_file == str(node):
             build = None
         elif isinstance(action, SCons.Action.FunctionAction):
             build = self.handle_func_action(node, action)
diff --git a/SCons/Tool/ninja/Overrides.py b/SCons/Tool/ninja_tool/Overrides.py
index 83d2efb..83d2efb 100644
--- a/SCons/Tool/ninja/Overrides.py
+++ b/SCons/Tool/ninja_tool/Overrides.py
diff --git a/SCons/Tool/ninja/Rules.py b/SCons/Tool/ninja_tool/Rules.py
index e7d19e3..e7d19e3 100644
--- a/SCons/Tool/ninja/Rules.py
+++ b/SCons/Tool/ninja_tool/Rules.py
diff --git a/SCons/Tool/ninja/Utils.py b/SCons/Tool/ninja_tool/Utils.py
index 24d439e..7782687 100644
--- a/SCons/Tool/ninja/Utils.py
+++ b/SCons/Tool/ninja_tool/Utils.py
@@ -413,14 +413,14 @@ def ninja_stat(_self, path):
     """
 
     try:
-        return SCons.Tool.ninja.Globals.NINJA_STAT_MEMO[path]
+        return SCons.Tool.ninja_tool.Globals.NINJA_STAT_MEMO[path]
     except KeyError:
         try:
             result = os.stat(path)
         except os.error:
             result = None
 
-        SCons.Tool.ninja.Globals.NINJA_STAT_MEMO[path] = result
+        SCons.Tool.ninja_tool.Globals.NINJA_STAT_MEMO[path] = result
         return result
 
 
@@ -430,7 +430,7 @@ def ninja_whereis(thing, *_args, **_kwargs):
     # Optimize for success, this gets called significantly more often
     # when the value is already memoized than when it's not.
     try:
-        return SCons.Tool.ninja.Globals.NINJA_WHEREIS_MEMO[thing]
+        return SCons.Tool.ninja_tool.Globals.NINJA_WHEREIS_MEMO[thing]
     except KeyError:
         # TODO: Fix this to respect env['ENV']['PATH']... WPD
         # We do not honor any env['ENV'] or env[*] variables in the
@@ -443,7 +443,7 @@ def ninja_whereis(thing, *_args, **_kwargs):
         # with shell quoting is nigh impossible. So I've decided to
         # cross that bridge when it's absolutely required.
         path = shutil.which(thing)
-        SCons.Tool.ninja.Globals.NINJA_WHEREIS_MEMO[thing] = path
+        SCons.Tool.ninja_tool.Globals.NINJA_WHEREIS_MEMO[thing] = path
         return path
 
 
diff --git a/SCons/Tool/ninja/__init__.py b/SCons/Tool/ninja_tool/__init__.py
index 7320d03..d86c2c9 100644
--- a/SCons/Tool/ninja/__init__.py
+++ b/SCons/Tool/ninja_tool/__init__.py
@@ -32,7 +32,7 @@ import sys
 
 import SCons
 import SCons.Script
-import SCons.Tool.ninja.Globals
+from SCons.Tool.ninja_tool.Globals import ninja_builder_initialized
 from SCons.Script import GetOption
 from SCons.Util import sanitize_shell_env
 
@@ -187,13 +187,13 @@ def ninja_emitter(target, source, env):
 
 def generate(env):
     """Generate the NINJA builders."""
-    global NINJA_STATE, NINJA_CMDLINE_TARGETS
+    global NINJA_STATE, NINJA_CMDLINE_TARGETS, ninja_builder_initialized
 
     if 'ninja' not in GetOption('experimental'):
         return
 
-    if not SCons.Tool.ninja.Globals.ninja_builder_initialized:
-        SCons.Tool.ninja.Globals.ninja_builder_initialized = True
+    if not ninja_builder_initialized:
+        ninja_builder_initialized = True
 
         ninja_add_command_line_options()
 
@@ -255,7 +255,7 @@ def generate(env):
         pass
     else:
         env.Append(CCFLAGS='$CCDEPFLAGS')
-    
+
     env.AddMethod(CheckNinjaCompdbExpand, "CheckNinjaCompdbExpand")
 
     # Provide a way for custom rule authors to easily access command
diff --git a/SCons/Tool/ninja/ninja.xml b/SCons/Tool/ninja_tool/ninja.xml
index f89687e..f89687e 100644
--- a/SCons/Tool/ninja/ninja.xml
+++ b/SCons/Tool/ninja_tool/ninja.xml
diff --git a/SCons/Tool/ninja/ninja_daemon_build.py b/SCons/Tool/ninja_tool/ninja_daemon_build.py
index f198d77..f198d77 100644
--- a/SCons/Tool/ninja/ninja_daemon_build.py
+++ b/SCons/Tool/ninja_tool/ninja_daemon_build.py
diff --git a/SCons/Tool/ninja/ninja_run_daemon.py b/SCons/Tool/ninja_tool/ninja_run_daemon.py
index 666be0f..666be0f 100644
--- a/SCons/Tool/ninja/ninja_run_daemon.py
+++ b/SCons/Tool/ninja_tool/ninja_run_daemon.py
diff --git a/SCons/Tool/ninja/ninja_scons_daemon.py b/SCons/Tool/ninja_tool/ninja_scons_daemon.py
index 8e297d3..8e297d3 100644
--- a/SCons/Tool/ninja/ninja_scons_daemon.py
+++ b/SCons/Tool/ninja_tool/ninja_scons_daemon.py
diff --git a/test/ninja/build_libraries.py b/test/ninja/build_libraries.py
index 0a1941a..9436053 100644
--- a/test/ninja/build_libraries.py
+++ b/test/ninja/build_libraries.py
@@ -23,8 +23,6 @@
 # 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
-
 import TestSCons
 from TestCmd import IS_WINDOWS, IS_MACOS
 from TestSCons import _exe, _lib, lib_, _dll, dll_
@@ -36,12 +34,7 @@ try:
 except ImportError:
     test.skip_test("Could not find ninja module. Skipping test.\n")
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/command_line_targets.py b/test/ninja/command_line_targets.py
index 9fa77ea..1a6033b 100644
--- a/test/ninja/command_line_targets.py
+++ b/test/ninja/command_line_targets.py
@@ -36,12 +36,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/copy_function_command.py b/test/ninja/copy_function_command.py
index 7e999b3..e8721a1 100644
--- a/test/ninja/copy_function_command.py
+++ b/test/ninja/copy_function_command.py
@@ -37,12 +37,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/default_targets.py b/test/ninja/default_targets.py
index 1b2f2b9..e809d75 100644
--- a/test/ninja/default_targets.py
+++ b/test/ninja/default_targets.py
@@ -36,9 +36,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(
-    os.path.join(ninja.__file__, os.pardir, 'data', 'bin', 'ninja' + _exe)
-)
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/force_scons_callback.py b/test/ninja/force_scons_callback.py
index e0da0dd..3ae5a5a 100644
--- a/test/ninja/force_scons_callback.py
+++ b/test/ninja/force_scons_callback.py
@@ -37,9 +37,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(
-    os.path.join(ninja.__file__, os.pardir, "data", "bin", "ninja" + _exe)
-)
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture("ninja-fixture")
 
diff --git a/test/ninja/generate_and_build.py b/test/ninja/generate_and_build.py
index e1c26d4..9d05683 100644
--- a/test/ninja/generate_and_build.py
+++ b/test/ninja/generate_and_build.py
@@ -37,12 +37,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/generate_and_build_cxx.py b/test/ninja/generate_and_build_cxx.py
index 074a5cb..80faa4b 100644
--- a/test/ninja/generate_and_build_cxx.py
+++ b/test/ninja/generate_and_build_cxx.py
@@ -37,12 +37,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/generate_source.py b/test/ninja/generate_source.py
index 8300176..b848b17 100644
--- a/test/ninja/generate_source.py
+++ b/test/ninja/generate_source.py
@@ -37,12 +37,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/generated_sources_alias.py b/test/ninja/generated_sources_alias.py
index 2c4ed36..22ca734 100644
--- a/test/ninja/generated_sources_alias.py
+++ b/test/ninja/generated_sources_alias.py
@@ -37,9 +37,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.BIN_DIR,
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/iterative_speedup.py b/test/ninja/iterative_speedup.py
index e5673b0..cbd4b6a 100644
--- a/test/ninja/iterative_speedup.py
+++ b/test/ninja/iterative_speedup.py
@@ -39,12 +39,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/mingw_command_generator_action.py b/test/ninja/mingw_command_generator_action.py
index 58c5106..ad78784 100644
--- a/test/ninja/mingw_command_generator_action.py
+++ b/test/ninja/mingw_command_generator_action.py
@@ -52,9 +52,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.BIN_DIR,
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/mingw_depfile_format.py b/test/ninja/mingw_depfile_format.py
index 5de7b5f..7792440 100644
--- a/test/ninja/mingw_depfile_format.py
+++ b/test/ninja/mingw_depfile_format.py
@@ -36,9 +36,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.BIN_DIR,
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/mkdir_function_command.py b/test/ninja/mkdir_function_command.py
index 8a17623..3fd678a 100644
--- a/test/ninja/mkdir_function_command.py
+++ b/test/ninja/mkdir_function_command.py
@@ -37,12 +37,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.write('SConstruct', """
 SetOption('experimental','ninja')
diff --git a/test/ninja/multi_env.py b/test/ninja/multi_env.py
index e5da6cf..7b113ed 100644
--- a/test/ninja/multi_env.py
+++ b/test/ninja/multi_env.py
@@ -37,12 +37,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test = TestSCons.TestSCons()
 
diff --git a/test/ninja/ninja_command_line.py b/test/ninja/ninja_command_line.py
index d8e3c08..8fc232e 100644
--- a/test/ninja/ninja_command_line.py
+++ b/test/ninja/ninja_command_line.py
@@ -37,12 +37,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture', 'src')
 
diff --git a/test/ninja/ninja_conftest.py b/test/ninja/ninja_conftest.py
index 91d2e03..60c4b03 100644
--- a/test/ninja/ninja_conftest.py
+++ b/test/ninja/ninja_conftest.py
@@ -37,12 +37,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/ninja_file_deterministic.py b/test/ninja/ninja_file_deterministic.py
index 9832f22..3d226ca 100644
--- a/test/ninja/ninja_file_deterministic.py
+++ b/test/ninja/ninja_file_deterministic.py
@@ -39,9 +39,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.BIN_DIR,
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/ninja_handle_control_c_rebuild.py b/test/ninja/ninja_handle_control_c_rebuild.py
index 9f6b413..d187c5f 100644
--- a/test/ninja/ninja_handle_control_c_rebuild.py
+++ b/test/ninja/ninja_handle_control_c_rebuild.py
@@ -23,7 +23,7 @@
 #
 """
 This test ensures if ninja gets a control-c (or other interrupting signal) while
-regenerating the build.ninja, it doesn't remove the build.ninja leaving it 
+regenerating the build.ninja, it doesn't remove the build.ninja leaving it
 in an unworkable state.
 """
 import os
@@ -40,10 +40,7 @@ except ImportError:
 
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
-
-ninja_bin = os.path.abspath(
-    os.path.join(ninja.__file__, os.pardir, "data", "bin", "ninja" + _exe)
-)
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture("ninja-fixture")
 
diff --git a/test/ninja/no_for_sig_subst.py b/test/ninja/no_for_sig_subst.py
index a0292ae..ef2c795 100644
--- a/test/ninja/no_for_sig_subst.py
+++ b/test/ninja/no_for_sig_subst.py
@@ -37,12 +37,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/response_file.py b/test/ninja/response_file.py
index 3d23c2b..f2561bb 100644
--- a/test/ninja/response_file.py
+++ b/test/ninja/response_file.py
@@ -42,12 +42,7 @@ _python_ = TestSCons._python_
 _exe = TestSCons._exe
 _obj = TestSCons._obj
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/shell_command.py b/test/ninja/shell_command.py
index a6c48c4..8b3d134 100644
--- a/test/ninja/shell_command.py
+++ b/test/ninja/shell_command.py
@@ -37,12 +37,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(os.path.join(
-    ninja.__file__,
-    os.pardir,
-    'data',
-    'bin',
-    'ninja' + _exe))
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture('ninja-fixture')
 
diff --git a/test/ninja/shutdown_scons_daemon.py b/test/ninja/shutdown_scons_daemon.py
index 64ec2c7..c646f97 100644
--- a/test/ninja/shutdown_scons_daemon.py
+++ b/test/ninja/shutdown_scons_daemon.py
@@ -44,9 +44,7 @@ except ImportError:
 _python_ = TestSCons._python_
 _exe = TestSCons._exe
 
-ninja_bin = os.path.abspath(
-    os.path.join(ninja.__file__, os.pardir, "data", "bin", "ninja" + _exe)
-)
+ninja_bin = TestSCons.NINJA_BINARY
 
 test.dir_fixture("ninja-fixture")
 
diff --git a/testing/framework/TestCmd.py b/testing/framework/TestCmd.py
index 7307078..56c282c 100644
--- a/testing/framework/TestCmd.py
+++ b/testing/framework/TestCmd.py
@@ -337,6 +337,9 @@ except AttributeError:
     IS_ROOT = False
 NEED_HELPER = os.environ.get('SCONS_NO_DIRECT_SCRIPT')
 
+
+
+
 # sentinel for cases where None won't do
 _Null = object()
 
diff --git a/testing/framework/TestSCons.py b/testing/framework/TestSCons.py
index 51aa7cf..5c95c24 100644
--- a/testing/framework/TestSCons.py
+++ b/testing/framework/TestSCons.py
@@ -40,17 +40,14 @@ import os
 import re
 import shutil
 import sys
-import time
 import subprocess as sp
+import time
 import zipfile
 from collections import namedtuple
 
+from SCons.Util import get_hash_format, get_current_hash_algorithm_used
 from TestCommon import *
 from TestCommon import __all__, _python_
-from SCons.Util import get_hash_format, get_current_hash_algorithm_used
-
-from TestCmd import Popen
-from TestCmd import PIPE
 
 # Some tests which verify that SCons has been packaged properly need to
 # look for specific version file names.  Replicating the version number
@@ -77,7 +74,8 @@ __all__.extend([
     'lib_',
     '_lib',
     'dll_',
-    '_dll'
+    '_dll',
+    'NINJA_BINARY'
 ])
 
 machine_map = {
@@ -106,6 +104,12 @@ lib_ = lib_prefix
 _dll = dll_suffix
 dll_ = dll_prefix
 
+try:
+    import ninja
+    NINJA_BINARY = os.path.abspath(os.path.join(ninja.BIN_DIR, 'ninja' + _exe))
+except ImportError:
+    NINJA_BINARY = None
+
 if sys.platform == 'cygwin':
     # On Cygwin, os.path.normcase() lies, so just report back the
     # fact that the underlying Win32 OS is case-insensitive.
@@ -224,6 +228,8 @@ def restore_sconsflags(sconsflags) -> None:
 # Helpers for Configure()'s config.log processing
 ConfigCheckInfo = namedtuple('ConfigCheckInfo',
                              ['check_string', 'result', 'cached', 'temp_filename'])
+
+
 #         check_string: the string output to for this checker
 #         results : The expected results for each check
 #         cached  : If the corresponding check is expected to be cached
@@ -234,6 +240,7 @@ class NoMatch(Exception):
     """
     Exception for matchPart to indicate there was no match found in the passed logfile
     """
+
     def __init__(self, p) -> None:
         self.pos = p
 
@@ -418,7 +425,8 @@ class TestSCons(TestCommon):
 
         return None
 
-    def wrap_stdout(self, build_str: str="", read_str: str="", error: int=0, cleaning: int=0) -> str:
+    def wrap_stdout(self, build_str: str = "", read_str: str = "", error: int = 0,
+                    cleaning: int = 0) -> str:
         """Wraps "expect" strings in SCons boilerplate.
 
         Given strings of expected output specific to a test,
@@ -444,11 +452,11 @@ class TestSCons(TestCommon):
             term = f"scons: done {lc}ing targets.\n"
 
         return "scons: Reading SConscript files ...\n" + \
-               read_str + \
-               "scons: done reading SConscript files.\n" + \
-               f"scons: {cap}ing targets ...\n" + \
-               build_str + \
-               term
+            read_str + \
+            "scons: done reading SConscript files.\n" + \
+            f"scons: {cap}ing targets ...\n" + \
+            build_str + \
+            term
 
     def run(self, *args, **kw) -> None:
         """
@@ -484,7 +492,7 @@ class TestSCons(TestCommon):
     #        kw['options'] = ' '.join(options)
     #        TestCommon.run(self, *args, **kw)
 
-    def up_to_date(self, arguments: str='.', read_str: str="", **kw) -> None:
+    def up_to_date(self, arguments: str = '.', read_str: str = "", **kw) -> None:
         """Asserts that all of the targets listed in arguments is
         up to date, but does not make any assumptions on other targets.
         This function is most useful in conjunction with the -n option.
@@ -500,7 +508,7 @@ class TestSCons(TestCommon):
         kw['match'] = self.match_re_dotall
         self.run(**kw)
 
-    def not_up_to_date(self, arguments: str='.', read_str: str="", **kw) -> None:
+    def not_up_to_date(self, arguments: str = '.', read_str: str = "", **kw) -> None:
         """Asserts that none of the targets listed in arguments is
         up to date, but does not make any assumptions on other targets.
         This function is most useful in conjunction with the -n option.
@@ -510,7 +518,8 @@ class TestSCons(TestCommon):
             s = f"{s}(?!scons: `{re.escape(arg)}' is up to date.)"
         s = f"({s}[^\n]*\n)*"
         kw['arguments'] = arguments
-        stdout = re.escape(self.wrap_stdout(read_str=read_str, build_str='ARGUMENTSGOHERE'))
+        stdout = re.escape(
+            self.wrap_stdout(read_str=read_str, build_str='ARGUMENTSGOHERE'))
         kw['stdout'] = stdout.replace('ARGUMENTSGOHERE', s)
         kw['match'] = self.match_re_dotall
         self.run(**kw)
@@ -619,15 +628,15 @@ class TestSCons(TestCommon):
 
         return warning
 
-    def diff_substr(self, expect, actual, prelen: int=20, postlen: int=40) -> str:
+    def diff_substr(self, expect, actual, prelen: int = 20, postlen: int = 40) -> str:
         i = 0
         for x, y in zip(expect, actual):
             if x != y:
                 return "Actual did not match expect at char %d:\n" \
                        "    Expect:  %s\n" \
                        "    Actual:  %s\n" \
-                       % (i, repr(expect[i - prelen:i + postlen]),
-                          repr(actual[i - prelen:i + postlen]))
+                    % (i, repr(expect[i - prelen:i + postlen]),
+                       repr(actual[i - prelen:i + postlen]))
             i = i + 1
         return "Actual matched the expected output???"
 
@@ -671,7 +680,7 @@ class TestSCons(TestCommon):
         return s
 
     @staticmethod
-    def to_bytes_re_sub(pattern, repl, string, count: int=0, flags: int=0):
+    def to_bytes_re_sub(pattern, repl, string, count: int = 0, flags: int = 0):
         """
         Wrapper around re.sub to change pattern and repl to bytes to work with
         both python 2 & 3
@@ -713,8 +722,9 @@ class TestSCons(TestCommon):
                 d = zlib.decompress(s[b:e])
                 d = self.to_bytes_re_sub(r'%%CreationDate: [^\n]*\n',
                                          r'%%CreationDate: 1970 Jan 01 00:00:00\n', d)
-                d = self.to_bytes_re_sub(r'%DVIPSSource:  TeX output \d\d\d\d\.\d\d\.\d\d:\d\d\d\d',
-                                         r'%DVIPSSource:  TeX output 1970.01.01:0000', d)
+                d = self.to_bytes_re_sub(
+                    r'%DVIPSSource:  TeX output \d\d\d\d\.\d\d\.\d\d:\d\d\d\d',
+                    r'%DVIPSSource:  TeX output 1970.01.01:0000', d)
                 d = self.to_bytes_re_sub(r'/(BaseFont|FontName) /[A-Z]{6}',
                                          r'/\1 /XXXXXX', d)
                 r.append(d)
@@ -754,10 +764,9 @@ class TestSCons(TestCommon):
         if hash_format is None and current_hash_algorithm == 'md5':
             return ".sconsign"
         else:
-            database_prefix=f".sconsign_{current_hash_algorithm}"
+            database_prefix = f".sconsign_{current_hash_algorithm}"
             return database_prefix
 
-
     def unlink_sconsignfile(self, name: str = '.sconsign.dblite') -> None:
         """Delete the sconsign file.
 
@@ -850,7 +859,8 @@ class TestSCons(TestCommon):
                         '/usr/lib/jvm/default-java/include/jni.h',
                         '/usr/lib/jvm/java-*-oracle/include/jni.h']
         else:
-            jni_dirs = [f'/System/Library/Frameworks/JavaVM.framework/Versions/{version}*/Headers/jni.h']
+            jni_dirs = [
+                f'/System/Library/Frameworks/JavaVM.framework/Versions/{version}*/Headers/jni.h']
         jni_dirs.extend([f'/usr/lib/jvm/java-*-sun-{version}*/include/jni.h',
                          f'/usr/lib/jvm/java-{version}*-openjdk*/include/jni.h',
                          f'/usr/java/jdk{version}*/include/jni.h'])
@@ -975,7 +985,8 @@ class TestSCons(TestCommon):
         where_java = self.where_is('java', ENV['PATH'])
 
         if not where_java:
-            self.skip_test("Could not find Java java, skipping test(s).\n", from_fw=True)
+            self.skip_test("Could not find Java java, skipping test(s).\n",
+                           from_fw=True)
         elif sys.platform == "darwin":
             self.java_mac_check(where_java, 'java')
 
@@ -996,7 +1007,8 @@ class TestSCons(TestCommon):
         else:
             where_javac = self.where_is('javac', ENV['PATH'])
         if not where_javac:
-            self.skip_test("Could not find Java javac, skipping test(s).\n", from_fw=True)
+            self.skip_test("Could not find Java javac, skipping test(s).\n",
+                           from_fw=True)
         elif sys.platform == "darwin":
             self.java_mac_check(where_javac, 'javac')
 
@@ -1050,7 +1062,8 @@ class TestSCons(TestCommon):
         else:
             where_javah = self.where_is('javah', ENV['PATH'])
         if not where_javah:
-            self.skip_test("Could not find Java javah, skipping test(s).\n", from_fw=True)
+            self.skip_test("Could not find Java javah, skipping test(s).\n",
+                           from_fw=True)
         return where_javah
 
     def java_where_rmic(self, version=None) -> str:
@@ -1068,7 +1081,9 @@ class TestSCons(TestCommon):
         else:
             where_rmic = self.where_is('rmic', ENV['PATH'])
         if not where_rmic:
-            self.skip_test("Could not find Java rmic, skipping non-simulated test(s).\n", from_fw=True)
+            self.skip_test(
+                "Could not find Java rmic, skipping non-simulated test(s).\n",
+                from_fw=True)
         return where_rmic
 
     def java_get_class_files(self, dir):
@@ -1079,7 +1094,7 @@ class TestSCons(TestCommon):
                     result.append(os.path.join(dirpath, fname))
         return sorted(result)
 
-    def Qt_dummy_installation(self, dir: str='qt') -> None:
+    def Qt_dummy_installation(self, dir: str = 'qt') -> None:
         # create a dummy qt installation
 
         self.subdir(dir, [dir, 'bin'], [dir, 'include'], [dir, 'lib'])
@@ -1185,11 +1200,11 @@ else:
         self.QT_UIC = f"{_python_} {self.workpath(dir, 'bin', 'myuic.py')}"
         self.QT_LIB_DIR = self.workpath(dir, 'lib')
 
-    def Qt_create_SConstruct(self, place, qt_tool: str='qt3') -> None:
+    def Qt_create_SConstruct(self, place, qt_tool: str = 'qt3') -> None:
         if isinstance(place, list):
             place = self.workpath(*place)
 
-        var_prefix=qt_tool.upper()
+        var_prefix = qt_tool.upper()
         self.write(place, f"""\
 if ARGUMENTS.get('noqtdir', 0):
     {var_prefix}DIR = None
@@ -1239,7 +1254,7 @@ SConscript(sconscript)
         """
         return 'COVERAGE_PROCESS_START' in os.environ or 'COVERAGE_FILE' in os.environ
 
-    def skip_if_not_msvc(self, check_platform: bool=True) -> None:
+    def skip_if_not_msvc(self, check_platform: bool = True) -> None:
         """ Skip test if MSVC is not available.
 
         Check whether we are on a Windows platform and skip the test if
@@ -1264,10 +1279,10 @@ SConscript(sconscript)
             pass
 
     def checkConfigureLogAndStdout(self, checks,
-                                   logfile: str='config.log',
-                                   sconf_dir: str='.sconf_temp',
-                                   sconstruct: str="SConstruct",
-                                   doCheckLog: bool=True, doCheckStdout: bool=True):
+                                   logfile: str = 'config.log',
+                                   sconf_dir: str = '.sconf_temp',
+                                   sconstruct: str = "SConstruct",
+                                   doCheckLog: bool = True, doCheckStdout: bool = True):
         """ Verify expected output from Configure.
 
         Used to verify the expected output from using Configure()
@@ -1298,7 +1313,8 @@ SConscript(sconscript)
             # sys.stderr.write("LOGFILE[%s]:%s"%(type(logfile),logfile))
 
             if (doCheckLog and
-                    logfile.find("scons: warning: The stored build information has an unexpected class.") >= 0):
+                logfile.find(
+                    "scons: warning: The stored build information has an unexpected class.") >= 0):
                 self.fail_test()
 
             log = r'file \S*%s\,line \d+:' % re.escape(sconstruct) + ls
@@ -1321,7 +1337,7 @@ SConscript(sconscript)
                 result_cached = 1
                 for bld_desc in check_info.cached:  # each TryXXX
                     for ext, flag in bld_desc:  # each file in TryBuild
-                        conf_filename = re.escape(check_info.temp_filename%ext)
+                        conf_filename = re.escape(check_info.temp_filename % ext)
 
                         if flag == self.NCR:
                             # NCR = Non Cached Rebuild
@@ -1339,8 +1355,9 @@ SConscript(sconscript)
                                   re.escape("scons: Configure: \"") + \
                                   conf_filename + \
                                   re.escape("\" is up to date.") + ls
-                            log = log + re.escape("scons: Configure: The original builder "
-                                                  "output was:") + ls
+                            log = log + re.escape(
+                                "scons: Configure: The original builder "
+                                "output was:") + ls
                             log = f"{log}(  \\|.*{ls})+"
                         if flag == self.NCF:
                             # non-cached rebuild failure
@@ -1351,8 +1368,10 @@ SConscript(sconscript)
                             log = log + \
                                   re.escape("scons: Configure: Building \"") + \
                                   conf_filename + \
-                                  re.escape("\" failed in a previous run and all its sources are up to date.") + ls
-                            log = log + re.escape("scons: Configure: The original builder output was:") + ls
+                                  re.escape(
+                                      "\" failed in a previous run and all its sources are up to date.") + ls
+                            log = log + re.escape(
+                                "scons: Configure: The original builder output was:") + ls
                             log = f"{log}(  \\|.*{ls})+"
                 if result_cached:
                     result = f"(cached) {check_info.result}"
@@ -1396,11 +1415,9 @@ SConscript(sconscript)
                 print("-----------------------------------------------------")
                 self.fail_test()
 
-
-
     def checkLogAndStdout(self, checks, results, cached,
                           logfile, sconf_dir, sconstruct,
-                          doCheckLog: bool=True, doCheckStdout: bool=True):
+                          doCheckLog: bool = True, doCheckStdout: bool = True):
         """ Verify expected output from Configure.
 
         Used to verify the expected output from using Configure()
@@ -1434,7 +1451,8 @@ SConscript(sconscript)
             # sys.stderr.write("LOGFILE[%s]:%s"%(type(logfile),logfile))
 
             if (doCheckLog and
-                    logfile.find("scons: warning: The stored build information has an unexpected class.") >= 0):
+                logfile.find(
+                    "scons: warning: The stored build information has an unexpected class.") >= 0):
                 self.fail_test()
 
             sconf_dir = sconf_dir
@@ -1462,10 +1480,12 @@ SConscript(sconscript)
                 for bld_desc in cache_desc:  # each TryXXX
                     for ext, flag in bld_desc:  # each file in TryBuild
                         if ext in ['.c', '.cpp']:
-                            conf_filename = re.escape(os.path.join(sconf_dir, "conftest")) +\
+                            conf_filename = re.escape(
+                                os.path.join(sconf_dir, "conftest")) + \
                                             r'_[a-z0-9]{32,64}_\d+%s' % re.escape(ext)
                         elif ext == '':
-                            conf_filename = re.escape(os.path.join(sconf_dir, "conftest")) +\
+                            conf_filename = re.escape(
+                                os.path.join(sconf_dir, "conftest")) + \
                                             r'_[a-z0-9]{32,64}(_\d+_[a-z0-9]{32,64})?'
 
                         else:
@@ -1477,8 +1497,10 @@ SConscript(sconscript)
                             # this shortcut should be sufficient.
                             # TODO: perhaps revisit and/or fix file naming for intermediate files in
                             #  Configure context logic
-                            conf_filename = re.escape(os.path.join(sconf_dir, "conftest")) +\
-                                            r'_[a-z0-9]{32,64}_\d+(_[a-z0-9]{32,64})?%s' % re.escape(ext)
+                            conf_filename = re.escape(
+                                os.path.join(sconf_dir, "conftest")) + \
+                                            r'_[a-z0-9]{32,64}_\d+(_[a-z0-9]{32,64})?%s' % re.escape(
+                                ext)
 
                         if flag == self.NCR:
                             # NCR = Non Cached Rebuild
@@ -1496,8 +1518,9 @@ SConscript(sconscript)
                                   re.escape("scons: Configure: \"") + \
                                   conf_filename + \
                                   re.escape("\" is up to date.") + ls
-                            log = log + re.escape("scons: Configure: The original builder "
-                                                  "output was:") + ls
+                            log = log + re.escape(
+                                "scons: Configure: The original builder "
+                                "output was:") + ls
                             log = f"{log}(  \\|.*{ls})+"
                         if flag == self.NCF:
                             # non-cached rebuild failure
@@ -1508,8 +1531,10 @@ SConscript(sconscript)
                             log = log + \
                                   re.escape("scons: Configure: Building \"") + \
                                   conf_filename + \
-                                  re.escape("\" failed in a previous run and all its sources are up to date.") + ls
-                            log = log + re.escape("scons: Configure: The original builder output was:") + ls
+                                  re.escape(
+                                      "\" failed in a previous run and all its sources are up to date.") + ls
+                            log = log + re.escape(
+                                "scons: Configure: The original builder output was:") + ls
                             log = f"{log}(  \\|.*{ls})+"
                     # cnt = cnt + 1
                 if result_cached:
@@ -1562,7 +1587,7 @@ SConscript(sconscript)
         # see also sys.prefix documentation
         return python_minor_version_string()
 
-    def get_platform_python_info(self, python_h_required: bool=False):
+    def get_platform_python_info(self, python_h_required: bool = False):
         """Return information about Python.
 
         Returns a path to a Python executable suitable for testing on
@@ -1577,7 +1602,8 @@ SConscript(sconscript)
         """
         python = os.environ.get('python_executable', self.where_is('python'))
         if not python:
-            self.skip_test('Can not find installed "python", skipping test.\n', from_fw=True)
+            self.skip_test('Can not find installed "python", skipping test.\n',
+                           from_fw=True)
 
         # construct a program to run in the intended environment
         # in order to fetch the characteristics of that Python.
@@ -1635,7 +1661,8 @@ else:
         stdout = self.stdout() or ""
         incpath, libpath, libname, python_h = stdout.strip().split('\n')
         if python_h == "False" and python_h_required:
-            self.skip_test('Can not find required "Python.h", skipping test.\n', from_fw=True)
+            self.skip_test('Can not find required "Python.h", skipping test.\n',
+                           from_fw=True)
 
         return (python, incpath, libpath, libname + _lib)
 
@@ -1656,7 +1683,7 @@ else:
             restore_sconsflags(sconsflags)
         return p
 
-    def wait_for(self, fname, timeout: float=20.0, popen=None) -> None:
+    def wait_for(self, fname, timeout: float = 20.0, popen=None) -> None:
         """
         Waits for the specified file name to exist.
         """
@@ -2004,7 +2031,7 @@ class TimeSCons(TestSCons):
                 destination = source.replace(source_dir, dest_dir)
                 shutil.copy2(source, destination)
 
-    def up_to_date(self, arguments: str='.', read_str: str="", **kw) -> None:
+    def up_to_date(self, arguments: str = '.', read_str: str = "", **kw) -> None:
         """Asserts that all of the targets listed in arguments is
         up to date, but does not make any assumptions on other targets.
         This function is most useful in conjunction with the -n option.
@@ -2025,7 +2052,6 @@ class TimeSCons(TestSCons):
         self.run(**kw)
 
 
-
 # In some environments, $AR will generate a warning message to stderr
 # if the library doesn't previously exist and is being created.  One
 # way to fix this is to tell AR to be quiet (sometimes the 'c' flag),