Merge pull request SCons#4485 from Raymo111/patch-1

Fix SCons#3935 OSErrors being hidden

Author: bdbaddog

CHANGES.txt

@@ -9,8 +9,16 @@ NOTE: 4.3.0 now requires Python 3.6.0 and above. Python 3.5.x is no longer suppo
 
 RELEASE  VERSION/DATE TO BE FILLED IN LATER
 
-  From Mats Wichmann:
+  From Raymond Li:
+    - Fix issue #3935: OSErrors are now no longer hidden during execution of
+      Actions. All exceptions during the execution of an Action are now
+      returned by value rather than by raising an exception, for more
+      consistent behavior.
+      NOTE: With this change, user created Actions should now catch and handle
+      expected exceptions (whereas previously many of these were silently
+      caught and suppressed by the SCons Action exection code).
 
+  From Mats Wichmann:
     - Updated Value Node docs and tests.
     - Python 3.13 compat: re.sub deprecated count, flags as positional args,
       caused update-release-info test to fail.

RELEASE.txt

@@ -31,11 +31,16 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
   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.
+- All exceptions during the execution of an Action are now returned by value
+  rather than by raising an exception, for more consistent behavior.
+  NOTE: With this change, user created Actions should now catch and handle
+  expected exceptions (whereas previously many of these were silently caught
+  and suppressed by the SCons Action exection code).
 
 FIXES
 -----
 
-- List fixes of outright bugs
+- OSErrors are now no longer hidden during the execution of Actions.
 
 IMPROVEMENTS
 ------------

SCons/Action.py

@@ -1454,16 +1454,6 @@ def execute(self, target, source, env, executor: Optional[ExecutorType] = None):
                 except TypeError:
                     result.command=self.strfunction(target, source, env)
 
-                # FIXME: This maintains backward compatibility with respect to
-                # which type of exceptions were returned by raising an
-                # exception and which ones were returned by value. It would
-                # probably be best to always return them by value here, but
-                # some codes do not check the return value of Actions and I do
-                # not have the time to modify them at this point.
-                if (exc_info[1] and
-                    not isinstance(exc_info[1], EnvironmentError)):
-                    raise result
-
             return result
         finally:
             # Break the cycle between the traceback object and this

SCons/CacheDirTests.py

@@ -164,7 +164,7 @@ def __setitem__(self, name, value) -> None:
         # so that _readconfig* will try to rewrite it
         old_config = os.path.join(self._CacheDir.path, "config")
         os.remove(old_config)
-        
+
         try:
             self._CacheDir._readconfig(self._CacheDir.path)
             assert False, "Should have raised exception and did not"
@@ -315,23 +315,17 @@ def mkdir(dir, mode: int=0) -> None:
         old_warn_exceptions = SCons.Warnings.warningAsException(1)
         SCons.Warnings.enableWarningClass(SCons.Warnings.CacheWriteErrorWarning)
 
-        try:
-            cd_f7 = self.test.workpath("cd.f7")
-            self.test.write(cd_f7, "cd.f7\n")
-            f7 = self.File(cd_f7, 'f7_bsig')
-
-            warn_caught = 0
-            try:
-                f7.push_to_cache()
-            except SCons.Errors.BuildError as e:
-                assert e.exc_info[0] == SCons.Warnings.CacheWriteErrorWarning
-                warn_caught = 1
-            assert warn_caught
-        finally:
-            shutil.copy2 = save_copy2
-            os.mkdir = save_mkdir
-            SCons.Warnings.warningAsException(old_warn_exceptions)
-            SCons.Warnings.suppressWarningClass(SCons.Warnings.CacheWriteErrorWarning)
+        cd_f7 = self.test.workpath("cd.f7")
+        self.test.write(cd_f7, "cd.f7\n")
+        f7 = self.File(cd_f7, 'f7_bsig')
+
+        warn_caught = 0
+        r = f7.push_to_cache()
+        assert r.exc_info[0] == SCons.Warnings.CacheWriteErrorWarning
+        shutil.copy2 = save_copy2
+        os.mkdir = save_mkdir
+        SCons.Warnings.warningAsException(old_warn_exceptions)
+        SCons.Warnings.suppressWarningClass(SCons.Warnings.CacheWriteErrorWarning)
 
     def test_no_strfunction(self) -> None:
         """Test handling no strfunction() for an action."""

SCons/Defaults.py

@@ -291,7 +291,15 @@ def copy_func(dest, src, symlinks: bool=True) -> int:
 
     elif os.path.islink(src):
         if symlinks:
-            os.symlink(os.readlink(src), dest)
+            try:
+                os.symlink(os.readlink(src), dest)
+            except FileExistsError:
+                raise SCons.Errors.BuildError(
+                    errstr=(
+                        f'Error: Copy() called to create symlink at "{dest}",'
+                        ' but a file already exists at that location.'
+                    )
+                )
             return 0
 
         return copy_func(dest, os.path.realpath(src))

SCons/Node/FS.py

@@ -326,7 +326,11 @@ def LocalString(target, source, env) -> str:
 
 def UnlinkFunc(target, source, env) -> int:
     t = target[0]
-    t.fs.unlink(t.get_abspath())
+    file = t.get_abspath()
+    try:
+        t.fs.unlink(file)
+    except FileNotFoundError:
+        pass
     return 0
 
 Unlink = SCons.Action.Action(UnlinkFunc, None)
@@ -2995,7 +2999,7 @@ def _createDir(self) -> None:
         # created.
         self.dir._create()
 
-    def push_to_cache(self) -> None:
+    def push_to_cache(self) -> bool:
         """Try to push the node into a cache
         """
         # This should get called before the Nodes' .built() method is
@@ -3006,10 +3010,10 @@ def push_to_cache(self) -> None:
         # the node to cache so that the memoization of the self.exists()
         # return value doesn't interfere.
         if self.nocache:
-            return
+            return None
         self.clear_memoized_values()
         if self.exists():
-            self.get_build_env().get_CacheDir().push(self)
+            return self.get_build_env().get_CacheDir().push(self)
 
     def retrieve_from_cache(self) -> bool:
         """Try to retrieve the node's content from a cache
@@ -3185,12 +3189,16 @@ def remove(self):
         return None
 
     def do_duplicate(self, src):
+        """Create a duplicate of this file from the specified source."""
         self._createDir()
         if SCons.Node.print_duplicate:
             print(f"dup: relinking variant '{self}' from '{src}'")
         Unlink(self, None, None)
-        e = Link(self, src, None)
-        if isinstance(e, SCons.Errors.BuildError):
+        try:
+            e = Link(self, src, None)
+            if isinstance(e, SCons.Errors.BuildError):
+                raise e
+        except SCons.Errors.BuildError as e:
             raise SCons.Errors.StopError(f"Cannot duplicate `{src.get_internal_path()}' in `{self.dir._path}': {e.errstr}.")
         self.linked = 1
         # The Link() action may or may not have actually

SCons/Node/__init__.py

@@ -678,7 +678,7 @@ def reset_executor(self) -> None:
         except AttributeError:
             pass
 
-    def push_to_cache(self) -> None:
+    def push_to_cache(self) -> bool:
         """Try to push a node into a cache
         """
         pass

SCons/SConfTests.py

@@ -207,7 +207,7 @@ def is_up_to_date(self) -> bool:
                         return False
                     def prepare(self) -> None:
                         pass
-                    def push_to_cache(self) -> None:
+                    def push_to_cache(self) -> bool:
                         pass
                     def retrieve_from_cache(self) -> bool:
                         return False

SCons/Taskmaster/TaskmasterTests.py

@@ -72,7 +72,7 @@ def targets(self, node):
     def disambiguate(self):
         return self
 
-    def push_to_cache(self) -> None:
+    def push_to_cache(self) -> bool:
         pass
 
     def retrieve_from_cache(self) -> bool:

SCons/Variables/PathVariable.py

@@ -109,6 +109,9 @@ def PathIsDirCreate(key, val, env) -> None:
         except PermissionError:
             m = 'Path for option %s could not be created: %s'
             raise SCons.Errors.UserError(m % (key, val))
+        except OSError:
+            m = 'Path for option %s could not be created: %s'
+            raise SCons.Errors.UserError(m % (key, val))
 
     @staticmethod
     def PathIsFile(key, val, env) -> None: