Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 4 additions & 2 deletions RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions SCons/Tool/ToolTests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""

Expand Down
100 changes: 84 additions & 16 deletions SCons/Tool/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -40,6 +55,7 @@
import os
import importlib.util

import SCons.Action
import SCons.Builder
import SCons.Errors
import SCons.Node.FS
Expand All @@ -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 = []
Expand Down Expand Up @@ -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 = []

Expand Down Expand Up @@ -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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do this, the method annotation is no longer correct.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked the AI about that.. apparently mypy any pyright will handle NotImplemented even if bool is the specified return value.. thoughts?

In CPython's typeshed (the canonical type stubs that mypy/pyright use), the object.__eq__ signature is:

  def __eq__(self, value: object, /) -> bool: ...

  It's annotated -> bool — even though object.__eq__ and virtually every real __eq__ can return NotImplemented. Typeshed deliberately does not write -> bool |
  type(NotImplemented).

  Why they do it that way:

  1. NotImplemented is a protocol sentinel, not a normal return value. When a.__eq__(b) returns NotImplemented, the Python interpreter intercepts it and tries the
  reflected b.__eq__(a), ultimately falling back to identity. Code essentially never does result = a.__eq__(b) and then inspects result — you write a == b, which the
  interpreter resolves to a real bool. So from a caller's perspective, the effective type of == is bool, and that's what typeshed annotates.
  2. The honest "full" type is ugly and rarely used. type(NotImplemented) is types.NotImplementedType (exposed as typing-friendly only in 3.10+). Writing -> bool |
  NotImplementedType is technically more precise but typeshed chose bool for ergonomics, and the type checkers special-case NotImplemented in __eq__/__lt__/etc. so
  returning it from a -> bool-annotated dunder is not flagged as an error.

  So "correct per typeshed" means: the established convention — the one mypy and pyright are tuned to expect — is to annotate __eq__ as -> bool and let the type
  checker handle the NotImplemented special-case. My proposed -> bool | type(NotImplemented) was more literally accurate but against that convention, and it's also
  slightly off as a type expression (type(NotImplemented) is a runtime value, not the clean typing form types.NotImplementedType).

  Bottom line: the existing -> bool is already the conventional, checker-approved annotation. There's nothing to fix on __eq__. My earlier framing of -> bool |
  type(NotImplemented) as "more correct" was misleading — I'd recommend leaving it as -> bool.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh. So "this has happened before" (unsurprisingly) and there's a special case for it. I didn't know that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(at least according to claude.. ;) I'll double check independently.. hmm other sources seem to say the same, but the mypy docs don't explicitly say the above.
I think it's probably ok as is.


def __hash__(self) -> int:
return hash(self.name)


LibSymlinksAction = SCons.Action.Action(LibSymlinksActionFunction, LibSymlinksStrFun)

Expand Down Expand Up @@ -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):
Comment thread
bdbaddog marked this conversation as resolved.
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 = []
Expand Down Expand Up @@ -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
Expand All @@ -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]
Expand Down
3 changes: 2 additions & 1 deletion SCons/Tool/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading