Fix isabs() check in NodeInfo for Py 3.13

Python 3.13 (alpha) changes the behavior of isabs() on Windows. SCons
has places where it does splitdrive on an absolute path, then checks
if the path part is absolute - this answer is now False, which caused
some interesting test fails.  Do the check on the original path to get
a more accurate answer.  There may be more subtle issues with the Python
change, but first fix the ones we can see.

Simplify the setup of _my_splitdrive() a bit: every caller is supposed to
check do_splitdrive but a couple of locations did not. Remove the special
check for UNC support, all Python versions SCons runs on do UNC handling,
so just eliminate that bit.

Fixes #4502, #4504.

Signed-off-by: Mats Wichmann <mats@linux.com>

CHANGES.txt

@@ -17,6 +17,8 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Dump() with json format selected now recognizes additional compound types
       (UserDict and UserList), which improves the detail of the display.
       json output is also sorted, to match the default display.
+    - Python 3.13 (alpha) changes the behavior of isabs() on Windows. Adjust
+      SCons usage of in NodeInfo classes to match.  Fixes #4502, #4504.
 
 
 RELEASE 4.7.0 -  Sun, 17 Mar 2024 17:22:20 -0700

RELEASE.txt

@@ -29,6 +29,8 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
 - Dump() with json format selected now recognizes additional compound types
   (UserDict and UserList), which improves the detail of the display.
   json output is also sorted, to match the default display.
+- Python 3.13 changes the behavior of isabs() on Windows. Adjust SCons
+  usage of this in NodeInfo classes to avoid test problems.
 
 FIXES
 -----

SCons/Node/FS.py

@@ -120,43 +120,41 @@ def save_strings(val) -> None:
     global Save_Strings
     Save_Strings = val
 
-#
-# Avoid unnecessary function calls by recording a Boolean value that
-# tells us whether or not os.path.splitdrive() actually does anything
-# on this system, and therefore whether we need to bother calling it
-# when looking up path names in various methods below.
-#
 
 do_splitdrive = None
-_my_splitdrive =None
+_my_splitdrive = None
 
 def initialize_do_splitdrive() -> None:
-    global do_splitdrive
-    global has_unc
-    drive, path = os.path.splitdrive('X:/foo')
-    # splitunc is removed from python 3.7 and newer
-    # so we can also just test if splitdrive works with UNC
-    has_unc = (hasattr(os.path, 'splitunc')
-        or os.path.splitdrive(r'\\split\drive\test')[0] == r'\\split\drive')
-
-    do_splitdrive = not not drive or has_unc
-
-    global _my_splitdrive
-    if has_unc:
-        def splitdrive(p):
+    """Set up splitdrive usage.
+
+    Avoid unnecessary function calls by recording a flag that tells us whether
+    or not :func:`os.path.splitdrive` actually does anything on this system,
+    and therefore whether we need to bother calling it when looking up path
+    names in various methods below.
+
+    If :data:`do_splitdrive` is True, :func:`_my_splitdrive` will be a real
+    function which we can call. As all supported Python versions' ntpath module
+    now handle UNC paths correctly, we no longer special-case that.
+
+    Deferring the setup of ``_my_splitdrive`` also lets unit tests do
+    their thing and test UNC path handling on a POSIX host.
+    """
+    global do_splitdrive, _my_splitdrive
+
+    do_splitdrive = bool(os.path.splitdrive('X:/foo')[0])
+
+    if do_splitdrive:
+        def _my_splitdrive(p):
             if p[1:2] == ':':
                 return p[:2], p[2:]
             if p[0:2] == '//':
                 # Note that we leave a leading slash in the path
                 # because UNC paths are always absolute.
                 return '//', p[1:]
             return '', p
-    else:
-        def splitdrive(p):
-            if p[1:2] == ':':
-                return p[:2], p[2:]
-            return '', p
-    _my_splitdrive = splitdrive
+    # TODO: the os routine should work and be better debugged than ours,
+    #   but unit test test_unc_path fails on POSIX platforms. Resolve someday.
+    # _my_splitdrive = os.path.splitdrive
 
     # Keep some commonly used values in global variables to skip to
     # module look-up costs.
@@ -1238,7 +1236,10 @@ def __init__(self, path = None) -> None:
             self.pathTop = os.getcwd()
         else:
             self.pathTop = path
-        self.defaultDrive = _my_normcase(_my_splitdrive(self.pathTop)[0])
+        if do_splitdrive:
+            self.defaultDrive = _my_normcase(_my_splitdrive(self.pathTop)[0])
+        else:
+            self.defaultDrive = ""
 
         self.Top = self.Dir(self.pathTop)
         self.Top._path = '.'
@@ -1554,11 +1555,15 @@ class DirNodeInfo(SCons.Node.NodeInfoBase):
     def str_to_node(self, s):
         top = self.fs.Top
         root = top.root
+        # Python 3.13/Win changed isabs() - after you split C:/foo/bar,
+        # the path part is no longer considerd absolute. Save the passed
+        # path for the isabs check so we can get the right answer.
+        path = s
         if do_splitdrive:
             drive, s = _my_splitdrive(s)
             if drive:
                 root = self.fs.get_root(drive)
-        if not os.path.isabs(s):
+        if not os.path.isabs(path):
             s = top.get_labspath() + '/' + s
         return root._lookup_abs(s, Entry)
 
@@ -2380,7 +2385,7 @@ def __init__(self, drive, fs) -> None:
         # The // entry is necessary because os.path.normpath()
         # preserves double slashes at the beginning of a path on Posix
         # platforms.
-        if not has_unc:
+        if not do_splitdrive:
             self._lookupDict['//'] = self
 
     def _morph(self) -> None:
@@ -2511,11 +2516,15 @@ class FileNodeInfo(SCons.Node.NodeInfoBase):
     def str_to_node(self, s):
         top = self.fs.Top
         root = top.root
+        # Python 3.13/Win changed isabs() - after you split C:/foo/bar,
+        # the path part is no longer considerd absolute. Save the passed
+        # path for the isabs check so we can get the right answer.
+        path = s
         if do_splitdrive:
             drive, s = _my_splitdrive(s)
             if drive:
                 root = self.fs.get_root(drive)
-        if not os.path.isabs(s):
+        if not os.path.isabs(path):
             s = top.get_labspath() + '/' + s
         return root._lookup_abs(s, Entry)
 
@@ -3534,7 +3543,7 @@ def is_up_to_date(self) -> bool:
 
         In all cases self is the target we're checking to see if it's up to date
         """
-        T = 0
+        T = False
         if T: Trace('is_up_to_date(%s):' % self)
         if not self.exists():
             if T: Trace(' not self.exists():')
@@ -3730,7 +3739,10 @@ def filedir_lookup(self, p, fd=None):
         if fd is None:
             fd = self.default_filedir
         dir, name = os.path.split(fd)
-        drive, d = _my_splitdrive(dir)
+        if do_splitdrive:
+            drive, d = _my_splitdrive(dir)
+        else:
+            drive, d = "", dir
         if not name and d[:1] in ('/', OS_SEP):
             #return p.fs.get_root(drive).dir_on_disk(name)
             return p.fs.get_root(drive)

SCons/Node/__init__.py

@@ -1063,6 +1063,7 @@ def scan(self) -> None:
         # Don't bother scanning non-derived files, because we don't
         # care what their dependencies are.
         # Don't scan again, if we already have scanned.
+        T = False
         if self.implicit is not None:
             return
         self.implicit = []
@@ -1087,7 +1088,12 @@ def scan(self) -> None:
                 # essentially short-circuits an N*M scan of the
                 # sources for each individual target, which is a hell
                 # of a lot more efficient.
+                def print_nodelist(n):
+                    tgts = [f"{t.path!r}" for t in n]
+                    return f"[{', '.join(tgts)}]"
+
                 for tgt in executor.get_all_targets():
+                    if T: Trace(f"adding implicit {print_nodelist(implicit)} to {tgt!s}\n")
                     tgt.add_to_implicit(implicit)
 
                 if implicit_deps_unchanged or self.is_up_to_date():
@@ -1472,8 +1478,8 @@ def changed(self, node=None, allowcache: bool=False):
 
         @see: FS.File.changed(), FS.File.release_target_info()
         """
-        t = 0
-        if t: Trace('changed(%s [%s], %s)' % (self, classname(self), node))
+        T = False
+        if T: Trace('changed(%s [%s], %s)' % (self, classname(self), node))
         if node is None:
             node = self
 
@@ -1491,25 +1497,24 @@ def changed(self, node=None, allowcache: bool=False):
             # entries to equal the new dependency list, for the benefit
             # of the loop below that updates node information.
             then.extend([None] * diff)
-            if t: Trace(': old %s new %s' % (len(then), len(children)))
+            if T: Trace(': old %s new %s' % (len(then), len(children)))
             result = True
 
         for child, prev_ni in zip(children, then):
             if _decider_map[child.changed_since_last_build](child, self, prev_ni, node):
-                if t: Trace(': %s changed' % child)
+                if T: Trace(f": '{child!s}' changed")
                 result = True
 
         if self.has_builder():
             contents = self.get_executor().get_contents()
             newsig = hash_signature(contents)
             if bi.bactsig != newsig:
-                if t: Trace(': bactsig %s != newsig %s' % (bi.bactsig, newsig))
+                if T: Trace(': bactsig %s != newsig %s' % (bi.bactsig, newsig))
                 result = True
 
         if not result:
-            if t: Trace(': up to date')
-
-        if t: Trace('\n')
+            if T: Trace(': up to date')
+        if T: Trace('\n')
 
         return result