From e5550e243cec6ea508d779966b6e1da2608c2e5b Mon Sep 17 00:00:00 2001 From: Daniel Moody Date: Thu, 13 Jan 2022 09:54:08 -0600 Subject: Added error message when failure to delete partial cache retrieval occurs. --- CHANGES.txt | 3 ++- RELEASE.txt | 2 ++ SCons/Taskmaster/__init__.py | 6 ++++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 7a7b3b6..07ce0d9 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -31,7 +31,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER - Added logic to help packagers enable reproducible builds into packaging/etc/. Please read packaging/etc/README.txt if you are interested. - + From Daniel Moody: + - Add error message for failure to delete partial cache retrieval files. From Dan Mezhiborsky: - Add newline to end of compilation db (compile_commands.json). diff --git a/RELEASE.txt b/RELEASE.txt index 8160bd6..04db087 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -58,6 +58,8 @@ IMPROVEMENTS ------------ - Changed the Taskmaster trace logic to use python's logging module. +- Add cache-debug messages for push failures. +- Add error message for failure to delete partial cache retrieval files. PACKAGING --------- diff --git a/SCons/Taskmaster/__init__.py b/SCons/Taskmaster/__init__.py index 3f0e700..0fc1a41 100644 --- a/SCons/Taskmaster/__init__.py +++ b/SCons/Taskmaster/__init__.py @@ -65,6 +65,8 @@ NODE_UP_TO_DATE = SCons.Node.up_to_date NODE_EXECUTED = SCons.Node.executed NODE_FAILED = SCons.Node.failed +display = SCons.Util.display + print_prepare = False # set by option --debug=prepare # A subsystem for recording stats about how different Nodes are handled by @@ -241,8 +243,8 @@ class Task(ABC): for t in cached_targets: try: t.fs.unlink(t.get_internal_path()) - except (IOError, OSError): - pass + except (IOError, OSError) as e: + display("scons: failed to delete partial cache file %s: %s" % (t.get_internal_path(), e)) self.targets[0].build() else: for t in cached_targets: -- cgit v0.12 From 5f07558ecb53003788a80856843d5912e6c66d17 Mon Sep 17 00:00:00 2001 From: Daniel Moody Date: Wed, 19 Jan 2022 11:21:15 -0600 Subject: improve changes notes and use SCons warning --- CHANGES.txt | 4 ++++ RELEASE.txt | 10 +++++++++- SCons/Taskmaster/__init__.py | 3 ++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 07ce0d9..df8cc2f 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -36,6 +36,10 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER From Dan Mezhiborsky: - Add newline to end of compilation db (compile_commands.json). + - Added error message to handle the case when SCons attempts to retrieve all the targets + for a specified builder from the CacheDir, fails to do so, and then runs into an error + when deleting the files which were retrieved. Previously if this happened there was no + errors or warnings. From Flaviu Tamas: - Added -fsanitize support to ParseFlags(). This will propagate to CCFLAGS and LINKFLAGS. diff --git a/RELEASE.txt b/RELEASE.txt index 04db087..4efbf4a 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -13,6 +13,11 @@ A new SCons release, 4.4.1, is now available on the SCons download page: Here is a summary of the changes since 4.4.0: +NOTE: If you build with Python 3.10.0 and then rebuild with 3.10.1 (or higher), you may + see unexpected rebuilds. This is due to Python internals changing which changed + the signature of a Python Action Function. + + NEW FUNCTIONALITY ----------------- @@ -59,7 +64,10 @@ IMPROVEMENTS - Changed the Taskmaster trace logic to use python's logging module. - Add cache-debug messages for push failures. -- Add error message for failure to delete partial cache retrieval files. +- Added error message to handle the case when SCons attempts to retrieve all the targets + for a specified builder from the CacheDir, fails to do so, and then runs into an error + when deleting the files which were retrieved. Previously if this happened there was no + errors or warnings. PACKAGING --------- diff --git a/SCons/Taskmaster/__init__.py b/SCons/Taskmaster/__init__.py index 0fc1a41..8f5e1ff 100644 --- a/SCons/Taskmaster/__init__.py +++ b/SCons/Taskmaster/__init__.py @@ -244,7 +244,8 @@ class Task(ABC): try: t.fs.unlink(t.get_internal_path()) except (IOError, OSError) as e: - display("scons: failed to delete partial cache file %s: %s" % (t.get_internal_path(), e)) + SCons.Warnings.warn(SCons.Warnings.CacheCleanupErrorWarning, + "failed to delete partial cache file %s: %s" % (t.get_internal_path(), e)) self.targets[0].build() else: for t in cached_targets: -- cgit v0.12 From 5c387589622407b77fe5d2a242eca5017616b240 Mon Sep 17 00:00:00 2001 From: Daniel Moody Date: Wed, 19 Jan 2022 11:41:22 -0600 Subject: added missing warning type --- SCons/Warnings.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/SCons/Warnings.py b/SCons/Warnings.py index c460897..754f05d 100644 --- a/SCons/Warnings.py +++ b/SCons/Warnings.py @@ -46,6 +46,9 @@ class CacheVersionWarning(WarningOnByDefault): class CacheWriteErrorWarning(SConsWarning): pass +class CacheCleanupErrorWarning(SConsWarning): + pass + class CorruptSConsignWarning(WarningOnByDefault): pass -- cgit v0.12 From 1270b58b99456f44537190f7fe3eca8c0675c322 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Sat, 2 Apr 2022 13:43:56 -0700 Subject: remove unused display and change text on warning per PR review --- SCons/Taskmaster/__init__.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/SCons/Taskmaster/__init__.py b/SCons/Taskmaster/__init__.py index 8f5e1ff..9bedd35 100644 --- a/SCons/Taskmaster/__init__.py +++ b/SCons/Taskmaster/__init__.py @@ -64,9 +64,6 @@ NODE_EXECUTING = SCons.Node.executing NODE_UP_TO_DATE = SCons.Node.up_to_date NODE_EXECUTED = SCons.Node.executed NODE_FAILED = SCons.Node.failed - -display = SCons.Util.display - print_prepare = False # set by option --debug=prepare # A subsystem for recording stats about how different Nodes are handled by @@ -245,7 +242,7 @@ class Task(ABC): t.fs.unlink(t.get_internal_path()) except (IOError, OSError) as e: SCons.Warnings.warn(SCons.Warnings.CacheCleanupErrorWarning, - "failed to delete partial cache file %s: %s" % (t.get_internal_path(), e)) + "Failed copying all target files from cache, Error while attempting to remove file %s retrieved from cache: %s" % (t.get_internal_path(), e)) self.targets[0].build() else: for t in cached_targets: -- cgit v0.12 From 84b32443baaa272f62ae6ed9fe3427ba57a5e960 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Thu, 1 Dec 2022 13:40:31 -0800 Subject: fixes to CHANGES/RELEASE --- CHANGES.txt | 7 +++---- RELEASE.txt | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index df8cc2f..18cfc60 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -32,15 +32,14 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER read packaging/etc/README.txt if you are interested. From Daniel Moody: - - Add error message for failure to delete partial cache retrieval files. - - From Dan Mezhiborsky: - - Add newline to end of compilation db (compile_commands.json). - Added error message to handle the case when SCons attempts to retrieve all the targets for a specified builder from the CacheDir, fails to do so, and then runs into an error when deleting the files which were retrieved. Previously if this happened there was no errors or warnings. + From Dan Mezhiborsky: + - Add newline to end of compilation db (compile_commands.json). + From Flaviu Tamas: - Added -fsanitize support to ParseFlags(). This will propagate to CCFLAGS and LINKFLAGS. diff --git a/RELEASE.txt b/RELEASE.txt index 4efbf4a..7d9e931 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -66,7 +66,7 @@ IMPROVEMENTS - Add cache-debug messages for push failures. - Added error message to handle the case when SCons attempts to retrieve all the targets for a specified builder from the CacheDir, fails to do so, and then runs into an error - when deleting the files which were retrieved. Previously if this happened there was no + when deleting the files which were retrieved. Previously if this happened there were no errors or warnings. PACKAGING -- cgit v0.12 From 3f4a001be6f517fafdbd75e81b0a0011cdf5ab7e Mon Sep 17 00:00:00 2001 From: William Deegan Date: Sat, 3 Dec 2022 14:20:12 -0800 Subject: Added test for failing to unlink cached target files --- SCons/Taskmaster/TaskmasterTests.py | 59 +++++++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/SCons/Taskmaster/TaskmasterTests.py b/SCons/Taskmaster/TaskmasterTests.py index 9d7f959..95150fd 100644 --- a/SCons/Taskmaster/TaskmasterTests.py +++ b/SCons/Taskmaster/TaskmasterTests.py @@ -244,6 +244,12 @@ class Node: self.executor.targets = self.targets return self.executor + def get_internal_path(self): + """ + Should only be used (currently) by TaskmasterTestCase.test_cached_execute_target_unlink_fails + """ + return str(self) + class OtherError(Exception): pass @@ -1071,7 +1077,7 @@ class TaskmasterTestCase(unittest.TestCase): n1 = Node("n1") # Mark the node as being cached - n1.cached = 1 + n1.cached = True tm = SCons.Taskmaster.Taskmaster([n1]) t = tm.next_task() t.prepare() @@ -1081,6 +1087,55 @@ class TaskmasterTestCase(unittest.TestCase): has_binfo = hasattr(n1, 'binfo') assert has_binfo, has_binfo + def test_cached_execute_target_unlink_fails(self): + """Test executing a task with cached targets where unlinking one of the targets fail + """ + global cache_text + import SCons.Warnings + + cache_text = [] + n1 = Node("n1") + n2 = Node("not-cached") + + class DummyFS: + def unlink(self, _): + raise IOError + + n1.fs = DummyFS() + + # Mark the node as being cached + n1.cached = True + # Add n2 as a target for n1 + n1.targets.append(n2) + # Explicitly mark n2 as not cached + n2.cached = False + + # Save SCons.Warnings.warn so we can mock it and catch it being called for unlink failures + _save_warn = SCons.Warnings.warn + issued_warnings = [] + + def fake_warnings_warn(clz, message): + nonlocal issued_warnings + issued_warnings.append((clz, message)) + SCons.Warnings.warn = fake_warnings_warn + + tm = SCons.Taskmaster.Taskmaster([n1, n2]) + t = tm.next_task() + t.prepare() + t.execute() + + # Restore saved warn + SCons.Warnings.warn = _save_warn + + self.assertTrue(len(issued_warnings) == 1, + msg='More than expected warnings (1) were issued %d' % len(issued_warnings)) + self.assertEqual(issued_warnings[0][0], SCons.Warnings.CacheCleanupErrorWarning, + msg='Incorrect warning class') + self.assertEqual(issued_warnings[0][1], + 'Failed copying all target files from cache, Error while attempting to remove file n1 retrieved from cache: ') + self.assertEqual(cache_text, ["n1 retrieved"], msg=cache_text) + + def test_exception(self): """Test generic Taskmaster exception handling @@ -1104,8 +1159,6 @@ class TaskmasterTestCase(unittest.TestCase): t.exception_set(None) # pass - # import pdb; pdb.set_trace() - # Having this here works for python 2.x, # but it is a tuple (None, None, None) when called outside # an except statement -- cgit v0.12