-- cgit v0.12 From 35f4086ceb163bcb036a9432b05b5c2089361c1f Mon Sep 17 00:00:00 2001 From: Dirk Baechle Date: Tue, 10 Jan 2012 23:12:51 +0100 Subject: - fixed issue #2720, applied final patch (v3) by Alexey Klimkin --- src/engine/SCons/Node/__init__.py | 1 + src/engine/SCons/Taskmaster.py | 31 +++++++++++++++++++------------ src/engine/SCons/TaskmasterTests.py | 3 +++ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py index 2fbe2c6..69d2898 100644 --- a/src/engine/SCons/Node/__init__.py +++ b/src/engine/SCons/Node/__init__.py @@ -216,6 +216,7 @@ class Node(object): self.precious = None self.noclean = 0 self.nocache = 0 + self.cached = 0 # is this node pulled from cache? self.always_build = None self.includes = None self.attributes = self.Attrs() # Generic place to stick information about the Node. diff --git a/src/engine/SCons/Taskmaster.py b/src/engine/SCons/Taskmaster.py index 58a8d90..64ab84d 100644 --- a/src/engine/SCons/Taskmaster.py +++ b/src/engine/SCons/Taskmaster.py @@ -227,20 +227,26 @@ class Task(object): if T: T.write(self.trace_message(u'Task.execute()', self.node)) try: - everything_was_cached = 1 + cached_targets = [] for t in self.targets: - if t.retrieve_from_cache(): - # Call the .built() method without calling the - # .push_to_cache() method, since we just got the - # target from the cache and don't need to push - # it back there. - t.set_state(NODE_EXECUTED) - t.built() - else: - everything_was_cached = 0 + if not t.retrieve_from_cache(): break - if not everything_was_cached: + cached_targets.append(t) + if len(cached_targets) < len(self.targets): + # Remove targets before building. It's possible that we + # partially retrieved targets from the cache, leaving + # them in read-only mode. That might cause the command + # to fail. + # + for t in cached_targets: + try: + t.fs.unlink(t.path) + except (IOError, OSError): + pass self.targets[0].build() + else: + for t in cached_targets: + t.cached = 1 except SystemExit: exc_value = sys.exc_info()[1] raise SCons.Errors.ExplicitExit(self.targets[0], exc_value.code) @@ -292,7 +298,8 @@ class Task(object): for side_effect in t.side_effects: side_effect.set_state(NODE_NO_STATE) t.set_state(NODE_EXECUTED) - t.push_to_cache() + if not t.cached: + t.push_to_cache() t.built() t.visited() diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py index 66c6b9c..4c56538 100644 --- a/src/engine/SCons/TaskmasterTests.py +++ b/src/engine/SCons/TaskmasterTests.py @@ -91,6 +91,9 @@ class Node(object): global built_text built_text = self.name + " built" + def remove(self): + pass + def built(self): global built_text if not self.cached: -- cgit v0.12 From c35fe0a420b99b701ecbac07258ce525dccda3d3 Mon Sep 17 00:00:00 2001 From: Dirk Baechle Date: Mon, 23 Apr 2012 00:22:46 +0200 Subject: - added simple test for #2720 (execute with cached targets) - fixed two typos - updated list of changes --- src/CHANGES.txt | 12 +++++ src/engine/SCons/Node/FS.py | 2 +- src/engine/SCons/Node/__init__.py | 2 +- src/engine/SCons/TaskmasterTests.py | 88 ++++++++++++++++++++++++++++++++++++- 4 files changed, 101 insertions(+), 3 deletions(-) diff --git a/src/CHANGES.txt b/src/CHANGES.txt index fbe4f25..b34350f 100644 --- a/src/CHANGES.txt +++ b/src/CHANGES.txt @@ -4,6 +4,18 @@ Change Log +RELEASE 2.X.X - + + From Alexey Klimkin: + + - Fixed the Taskmaster, curing spurious build failures in + multi-threaded runs (#2720). + + From Dirk Baechle: + + - Improved documentation of command-line variables (#2809). + - Fixed scons-doc.py to properly convert main XML files (#2812). + RELEASE 2.1.0 - Mon, 09 Sep 2011 20:54:57 -0700 diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py index f106d46..f31ca83 100644 --- a/src/engine/SCons/Node/FS.py +++ b/src/engine/SCons/Node/FS.py @@ -2715,7 +2715,7 @@ class File(Base): so only do thread safe stuff here. Do thread unsafe stuff in built(). - Returns true iff the node was successfully retrieved. + Returns true if the node was successfully retrieved. """ if self.nocache: return None diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py index 69d2898..992284d 100644 --- a/src/engine/SCons/Node/__init__.py +++ b/src/engine/SCons/Node/__init__.py @@ -307,7 +307,7 @@ class Node(object): so only do thread safe stuff here. Do thread unsafe stuff in built(). - Returns true iff the node was successfully retrieved. + Returns true if the node was successfully retrieved. """ return 0 diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py index 4c56538..85ade8d 100644 --- a/src/engine/SCons/TaskmasterTests.py +++ b/src/engine/SCons/TaskmasterTests.py @@ -86,6 +86,7 @@ class Node(object): def prepare(self): self.prepared = 1 + self.get_binfo() def build(self): global built_text @@ -94,11 +95,52 @@ class Node(object): def remove(self): pass + # The following four methods new_binfo(), del_binfo(), + # get_binfo(), clear() as well as its calls have been added + # to support the cached_execute() test (issue #2720). + # They are full copies (or snippets) of their actual + # counterparts in the Node class... + def new_binfo(self): + binfo = "binfo" + return binfo + + def del_binfo(self): + """Delete the build info from this node.""" + try: + delattr(self, 'binfo') + except AttributeError: + pass + + def get_binfo(self): + """Fetch a node's build information.""" + try: + return self.binfo + except AttributeError: + pass + + binfo = self.new_binfo() + self.binfo = binfo + + return binfo + + def clear(self): + # The del_binfo() call here isn't necessary for normal execution, + # but is for interactive mode, where we might rebuild the same + # target and need to start from scratch. + self.del_binfo() + def built(self): global built_text if not self.cached: built_text = built_text + " really" - + + # Clear the implicit dependency caches of any Nodes + # waiting for this Node to be built. + for parent in self.waiting_parents: + parent.implicit = None + + self.clear() + def has_builder(self): return not self.builder is None @@ -957,6 +999,50 @@ class TaskmasterTestCase(unittest.TestCase): assert built_text is None, built_text assert cache_text == ["n7 retrieved", "n8 retrieved"], cache_text + def test_cached_execute(self): + """Test executing a task with cached targets + """ + # In issue #2720 Alexei Klimkin detected that the previous + # workflow for execute() led to problems in a multithreaded build. + # We have: + # task.prepare() + # task.execute() + # task.executed() + # -> node.visited() + # for the Serial flow, but + # - Parallel - - Worker - + # task.prepare() + # requestQueue.put(task) + # task = requestQueue.get() + # task.execute() + # resultQueue.put(task) + # task = resultQueue.get() + # task.executed() + # ->node.visited() + # in parallel. Since execute() used to call built() when a target + # was cached, it could unblock dependent nodes before the binfo got + # restored again in visited(). This resulted in spurious + # "file not found" build errors, because files fetched from cache would + # be seen as not up to date and wouldn't be scanned for implicit + # dependencies. + # + # The following test ensures that execute() only marks targets as cached, + # but the actual call to built() happens in executed() only. + # Like this, the binfo should still be intact after calling execute()... + global cache_text + + n1 = Node("n1") + # Mark the node as being cached + n1.cached = 1 + tm = SCons.Taskmaster.Taskmaster([n1]) + t = tm.next_task() + t.prepare() + t.execute() + assert cache_text == ["n1 retrieved"], cache_text + # If no binfo exists anymore, something has gone wrong... + has_binfo = hasattr(n1, 'binfo') + assert has_binfo == True, has_binfo + def test_exception(self): """Test generic Taskmaster exception handling -- cgit v0.12