Merge pull request #4838 from mwichmann/tool-init

Do not duplicate Tool class creation

Author: bdbaddog

CHANGES.txt

@@ -121,6 +121,10 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Adjust race protection for CacheDir - now uses a unique temporary
       directory for each writer before doing the move, instead of depending
       on a one-time uuid to make a path to the file.
+    - Clarify VariantDir behavior when switching to not duplicate sources
+      and tweak wording a bit.
+    - Don't duplicate the creation of Tool objects if "default" is part of
+      the tool list.
     - Test suite: add support for Python 3.15 in the Action unit tests.
     - Add a test for FindFile to be sure it locates non-existing derived
       files as advertised.

RELEASE.txt

@@ -87,12 +87,14 @@ IMPROVEMENTS
 - Switch remaining "original style" docstring parameter listings to Google style.
 
 - Additional small tweaks to Environment.py type hints, fold some overly
-long function signature lines, and some linting-insipired cleanups.
+long function signature lines, and some linting-inspired cleanups.
 
 - Test framework: tweak module docstrings
 
 - Test suite: end to end tests don't use assert in result checks
 
+- Don't duplicate the creation of Tool objects if "default" is part of
+  the tool list.
 - Test suite: add support for Python 3.15 in the Action unit tests.
 - zip tool now uses zipfile module's "from_file" method to populate ZipInfo
   records, rather than doing manually.
@@ -174,7 +176,7 @@ DEVELOPMENT
 - Update pyproject.toml to support Python 3.14 and remove restrictions on lxml version install
 - Unify internal "_null" sentinel usage.
 - Docbook tests: improve skip message, more clearly indicate which test
-  need actual installed system programs (add -live suffix).
+  needs actual installed system programs (add -live suffix).
 - Implement type hints for Environment and environment utilities.
 
 - MSVC: Added a host/target batch file configuration table for Visual

SCons/Tool/ToolTests.py

@@ -100,6 +100,28 @@ def test_Tool(self) -> None:
         assert exc_caught, "did not catch expected UserError"
 
 
+    def test_Tool_instance(self) -> None:
+        """Test Tool instance passthrough and equality/hash behavior."""
+
+        # Passing an existing Tool instance back to Tool() returns the
+        # same instance rather than creating a duplicate.
+        t = SCons.Tool.Tool('g++')
+        assert SCons.Tool.Tool(t) is t, "Tool() did not return the passed-in instance"
+
+        # A Tool compares equal to its name string and to another Tool
+        # with the same name, but not to unrelated types.
+        assert t == 'g++', t
+        assert t == SCons.Tool.Tool('g++'), t
+        assert t != 'gcc', t
+        assert t != 1, t
+        assert t != None, t  # noqa: E711  (explicitly exercising __eq__)
+
+        # __hash__ is consistent with the name, so a Tool and its name
+        # collapse together in a set.
+        assert hash(t) == hash('g++'), hash(t)
+        assert len({t, 'g++', SCons.Tool.Tool('g++')}) == 1
+
+
     def test_pathfind(self) -> None:
         """Test that find_program_path() alters PATH only if add_path is true"""
 

SCons/Tool/__init__.py

@@ -22,16 +22,31 @@
 # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-"""SCons tool selection.
-
-Looks for modules that define a callable object that can modify a
-construction environment as appropriate for a given tool (or tool chain).
-
-Note that because this subsystem just *selects* a callable that can
-modify a construction environment, it's possible for people to define
-their own "tool specification" in an arbitrary callable function.  No
-one needs to use or tie in to this subsystem in order to roll their own
-tool specifications.
+"""SCons tool subsystem.
+
+Tool specification modules are callable objects that modify construction
+environments to dynamically enable specific types of builds. This module
+provides the support for handling tool modules:
+
+  - a Tool class to locate and load a tool module.
+  - lists of default tools for supported platform types and a mechanism to
+    select the available ones for the current platform.
+  - various rules for name remapping, special-cased toolchains, etc.
+  - utility functions for creating common Builders (eliminating duplication
+    when multiple tool modules could potentially create a Builder).
+  - create Scanners for common types used by the Builder utilities.
+
+This is not set up like a traditional Python package: this file implements
+the part that other subsystems can import and call. The remaining files
+are either dynamically loaded tool modules that present entry points
+(``exists()`` and ``generate()``) called through the Tool instance,
+or support files/common logic that can be imported by those tool
+modules. Neither are expected to be directly imported by any other SCons
+subsystem (test code may reach in and do so).
+
+Tool modules are simply callable objects that modify a construction
+environment. You can define custom tool specifications in any callable
+without needing to integrate with this subsystem.
 """
 
 from __future__ import annotations
@@ -40,6 +55,7 @@
 import os
 import importlib.util
 
+import SCons.Action
 import SCons.Builder
 import SCons.Errors
 import SCons.Node.FS
@@ -50,6 +66,7 @@
 import SCons.Scanner.LaTeX
 import SCons.Scanner.Prog
 import SCons.Scanner.SWIG
+import SCons.Util
 from SCons.Tool.linkCommon import LibSymlinksActionFunction, LibSymlinksStrFun
 
 DefaultToolpath = []
@@ -108,7 +125,25 @@
 
 
 class Tool:
+    """A class for loading and applying tool modules.
+
+    *name* is looked up using standard paths plus any specified *toolpath*.
+    To avoid duplicate creation of instances, recognize if *name*
+    is actually an existing instance, if so, just return ourselves
+    without further setup.
+
+    .. versionchanged:: NEXT_RELEASE
+       Accept an existing instance at creation time and don't duplicate it.
+    """
+
+    def __new__(cls, name, toolpath=None, **kwargs) -> "Tool":
+        if isinstance(name, Tool):
+            return name
+        return super().__new__(cls)
+
     def __init__(self, name, toolpath=None, **kwargs) -> None:
+        if isinstance(name, Tool):
+            return
         if toolpath is None:
             toolpath = []
 
@@ -270,6 +305,23 @@ def __call__(self, env, *args, **kw) -> None:
     def __str__(self) -> str:
         return self.name
 
+    def __eq__(self, other) -> bool:
+        """Compare a Tool by name.
+
+        A Tool is equal to another Tool with the same name, and to a
+        string matching its name (so a Tool can be used interchangeably
+        with its name in membership tests and sets). Comparison against
+        any other type is deferred (returns ``NotImplemented``).
+        """
+        if isinstance(other, Tool):
+            return self.name == other.name
+        if isinstance(other, str):
+            return self.name == other
+        return NotImplemented
+
+    def __hash__(self) -> int:
+        return hash(self.name)
+
 
 LibSymlinksAction = SCons.Action.Action(LibSymlinksActionFunction, LibSymlinksStrFun)
 
@@ -671,18 +723,34 @@ def InstallVersionedLib(self, *args, **kw):
 
 def FindTool(tools, env):
     for tool in tools:
+        if not SCons.Util.is_String(tool):
+            # Already a Tool instance
+            if tool.exists(env):
+                return tool
+            continue
         t = Tool(tool)
         if t.exists(env):
-            return tool
+            return t
     return None
 
 
 def FindAllTools(tools, env):
     def ToolExists(tool, env=env):
-        return Tool(tool).exists(env)
-
-    return list(filter(ToolExists, tools))
+        if not SCons.Util.is_String(tool):
+            return tool.exists(env)
+        t = Tool(tool)
+        return t.exists(env)
 
+    results = []
+    for tool in tools:
+        if not SCons.Util.is_String(tool):
+            if tool.exists(env):
+                results.append(tool)
+            continue
+        t = Tool(tool)
+        if t.exists(env):
+            results.append(t)
+    return results
 
 def tool_list(platform, env):
     other_plat_tools = []
@@ -773,7 +841,7 @@ def tool_list(platform, env):
 
     # XXX this logic about what tool provides what should somehow be
     #     moved into the tool files themselves.
-    if c_compiler and c_compiler == 'mingw':
+    if c_compiler and str(c_compiler) == 'mingw':
         # MinGW contains a linker, C compiler, C++ compiler,
         # Fortran compiler, archiver and assembler:
         cxx_compiler = None
@@ -783,7 +851,7 @@ def tool_list(platform, env):
         ar = None
     else:
         # Don't use g++ if the C compiler has built-in C++ support:
-        if c_compiler in ('msvc', 'intelc', 'icc'):
+        if str(c_compiler) in ('msvc', 'intelc', 'icc'):
             cxx_compiler = None
         else:
             cxx_compiler = FindTool(cxx_compilers, env) or cxx_compilers[0]

SCons/Tool/default.py

@@ -29,11 +29,12 @@
 selection method.
 """
 
+import SCons.Platform
 import SCons.Tool
 
 def generate(env) -> None:
     """Add default tools."""
-    for t in SCons.Tool.tool_list(env['PLATFORM'], env):
+    for t in SCons.Platform.DefaultToolList(env['PLATFORM'], env):
         SCons.Tool.Tool(t)(env)
 
 def exists(env) -> bool: