From 18d748a479233b689144db11a2ac3643a6577273 Mon Sep 17 00:00:00 2001 From: Steven Knight Date: Sat, 17 Jan 2004 14:56:46 +0000 Subject: Fix retrieving multiple target files from cache. (Bob Halley) --- src/CHANGES.txt | 6 +++++ src/engine/SCons/Node/FS.py | 28 +++++++++++----------- src/engine/SCons/Node/FSTests.py | 43 ++++++++++++++++++++++------------ src/engine/SCons/Node/NodeTests.py | 6 +++++ src/engine/SCons/Node/__init__.py | 11 +++++++++ src/engine/SCons/Taskmaster.py | 8 ++++++- src/engine/SCons/TaskmasterTests.py | 46 +++++++++++++++++++++++++++++++++++++ test/CacheDir.py | 28 ++++++++++++++++++++++ 8 files changed, 147 insertions(+), 29 deletions(-) diff --git a/src/CHANGES.txt b/src/CHANGES.txt index 9252736..3e0feaa 100644 --- a/src/CHANGES.txt +++ b/src/CHANGES.txt @@ -78,6 +78,12 @@ RELEASE 0.95 - XXX environments so long as the builders have the same signature for the environments in questions (that is, they're the same action). + From Bob Halley: + + - When multiple targets are built by a single action, retrieve all + of them from cache, not just the first target, and exec the build + command if any of the targets isn't present in the cache. + From Steven Knight: - Fix EnsureSConsVersion() so it checks against the SCons version, diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py index 7c4b383..46d72d7 100644 --- a/src/engine/SCons/Node/FS.py +++ b/src/engine/SCons/Node/FS.py @@ -48,6 +48,7 @@ import SCons.Action import SCons.Errors import SCons.Node import SCons.Util +import SCons.Sig.MD5 import SCons.Warnings # @@ -1311,20 +1312,18 @@ class File(Base): except OSError: pass - def build(self): - """Actually build the file. - - This overrides the base class build() method to check for the - existence of derived files in a CacheDir before going ahead and - building them. + def retrieve_from_cache(self): + """Try to retrieve the node's content from a cache This method is called from multiple threads in a parallel build, so only do thread safe stuff here. Do thread unsafe stuff in built(). + + Returns true iff the node was successfully retrieved. """ b = self.is_derived() if not b and not self.has_src_builder(): - return + return None if b and self.fs.CachePath: if self.fs.cache_show: if CacheRetrieveSilent(self, None, None) == 0: @@ -1336,10 +1335,10 @@ class File(Base): for a in al: action.show(a) self._for_each_action(do_print) - return + return 1 elif CacheRetrieve(self, None, None) == 0: - return - SCons.Node.Node.build(self) + return 1 + return None def built(self): """Called just after this node is sucessfully built.""" @@ -1509,10 +1508,13 @@ class File(Base): bsig = self.get_bsig() if bsig is None: raise SCons.Errors.InternalError, "cachepath(%s) found a bsig of None" % self.path - bsig = str(bsig) - subdir = string.upper(bsig[0]) + # Add the path to the cache signature, because multiple + # targets built by the same action will all have the same + # build signature, and we have to differentiate them somehow. + cache_sig = SCons.Sig.MD5.collect([bsig, self.path]) + subdir = string.upper(cache_sig[0]) dir = os.path.join(self.fs.CachePath, subdir) - return dir, os.path.join(dir, bsig) + return dir, os.path.join(dir, cache_sig) return None, None def target_from_source(self, prefix, suffix, splitext=SCons.Util.splitext): diff --git a/src/engine/SCons/Node/FSTests.py b/src/engine/SCons/Node/FSTests.py index 739d4e9..132e8f6 100644 --- a/src/engine/SCons/Node/FSTests.py +++ b/src/engine/SCons/Node/FSTests.py @@ -1528,7 +1528,8 @@ class CacheDirTestCase(unittest.TestCase): self.retrieved = [] built_it = None - f1.build() + r = f1.retrieve_from_cache() + assert r == 1, r assert self.retrieved == [f1], self.retrieved assert built_it is None, built_it @@ -1536,9 +1537,10 @@ class CacheDirTestCase(unittest.TestCase): self.retrieved = [] built_it = None - f1.build() + r = f1.retrieve_from_cache() + assert r is None, r assert self.retrieved == [f1], self.retrieved - assert built_it, built_it + assert built_it is None, built_it finally: SCons.Node.FS.CacheRetrieve = save_CacheRetrieve @@ -1554,7 +1556,8 @@ class CacheDirTestCase(unittest.TestCase): self.retrieved = [] built_it = None - f2.build() + r = f2.retrieve_from_cache() + assert r == 1, r assert self.retrieved == [f2], self.retrieved assert built_it is None, built_it @@ -1562,9 +1565,10 @@ class CacheDirTestCase(unittest.TestCase): self.retrieved = [] built_it = None - f2.build() + r = f2.retrieve_from_cache() + assert r is None, r assert self.retrieved == [f2], self.retrieved - assert built_it, built_it + assert built_it is None, built_it finally: SCons.Node.FS.CacheRetrieveSilent = save_CacheRetrieveSilent @@ -1602,12 +1606,19 @@ class CacheDirTestCase(unittest.TestCase): # Verify how the cachepath() method determines the name # of the file in cache. - f5 = fs.File("cd.f5") - f5.set_bsig('a_fake_bsig') - cp = f5.cachepath() - dirname = os.path.join('cache', 'A') - filename = os.path.join(dirname, 'a_fake_bsig') - assert cp == (dirname, filename), cp + def my_collect(list): + return list[0] + save_collect = SCons.Sig.MD5.collect + SCons.Sig.MD5.collect = my_collect + try: + f5 = fs.File("cd.f5") + f5.set_bsig('a_fake_bsig') + cp = f5.cachepath() + dirname = os.path.join('cache', 'A') + filename = os.path.join(dirname, 'a_fake_bsig') + assert cp == (dirname, filename), cp + finally: + SCons.Sig.MD5.collect = save_collect # Verify that no bsig raises an InternalERror f6 = fs.File("cd.f6") @@ -1661,7 +1672,8 @@ class CacheDirTestCase(unittest.TestCase): self.retrieved = [] built_it = None - f8.build() + r = f8.retrieve_from_cache() + assert r == 1, r assert self.retrieved == [f8], self.retrieved assert built_it is None, built_it @@ -1669,9 +1681,10 @@ class CacheDirTestCase(unittest.TestCase): self.retrieved = [] built_it = None - f8.build() + r = f8.retrieve_from_cache() + assert r is None, r assert self.retrieved == [f8], self.retrieved - assert built_it, built_it + assert built_it is None, built_it finally: SCons.Node.FS.CacheRetrieveSilent = save_CacheRetrieveSilent diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py index 08e4260..6b8c8ae 100644 --- a/src/engine/SCons/Node/NodeTests.py +++ b/src/engine/SCons/Node/NodeTests.py @@ -253,6 +253,12 @@ class NodeTestCase(unittest.TestCase): assert str(act.built_target[0]) == "xxx", str(act.built_target[0]) assert act.built_source == ["yyy", "zzz"], act.built_source + def test_retrieve_from_cache(self): + """Test the base retrieve_from_cache() method""" + n = SCons.Node.Node() + r = n.retrieve_from_cache() + assert r == 0, r + def test_visited(self): """Test the base visited() method diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py index 28264ce..0e453d2 100644 --- a/src/engine/SCons/Node/__init__.py +++ b/src/engine/SCons/Node/__init__.py @@ -176,6 +176,17 @@ class Node: executor = self.get_executor() executor(self, func) + def retrieve_from_cache(self): + """Try to retrieve the node's content from a cache + + This method is called from multiple threads in a parallel build, + so only do thread safe stuff here. Do thread unsafe stuff in + built(). + + Returns true iff the node was successfully retrieved. + """ + return 0 + def build(self): """Actually build the node. diff --git a/src/engine/SCons/Taskmaster.py b/src/engine/SCons/Taskmaster.py index 90d1452..66492de 100644 --- a/src/engine/SCons/Taskmaster.py +++ b/src/engine/SCons/Taskmaster.py @@ -92,7 +92,13 @@ class Task: prepare(), executed() or failed().""" try: - self.targets[0].build() + everything_was_cached = 1 + for t in self.targets: + if not t.retrieve_from_cache(): + everything_was_cached = 0 + break + if not everything_was_cached: + self.targets[0].build() except KeyboardInterrupt: raise except SystemExit: diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py index 019611a..63190a5 100644 --- a/src/engine/SCons/TaskmasterTests.py +++ b/src/engine/SCons/TaskmasterTests.py @@ -31,6 +31,7 @@ import SCons.Errors built_text = None +cache_text = [] visited_nodes = [] executed = None scan_called = 0 @@ -40,6 +41,7 @@ class Node: self.name = name self.kids = kids self.scans = scans + self.cached = 0 self.scanned = 0 self.scanner = None self.builder = Node.build @@ -55,6 +57,12 @@ class Node: for kid in kids: kid.parents.append(self) + def retrieve_from_cache(self): + global cache_text + if self.cached: + cache_text.append(self.name + " retrieved") + return self.cached + def build(self): global built_text built_text = self.name + " built" @@ -654,6 +662,7 @@ class TaskmasterTestCase(unittest.TestCase): """ global built_text + global cache_text n1 = Node("n1") tm = SCons.Taskmaster.Taskmaster([n1]) @@ -708,6 +717,43 @@ class TaskmasterTestCase(unittest.TestCase): else: raise TestFailed, "did not catch expected BuildError" + built_text = None + cache_text = [] + n5 = Node("n5") + n6 = Node("n6") + n6.cached = 1 + tm = SCons.Taskmaster.Taskmaster([n5]) + t = tm.next_task() + # This next line is moderately bogus. We're just reaching + # in and setting the targets for this task to an array. The + # "right" way to do this would be to have the next_task() call + # set it up by having something that approximates a real Builder + # return this list--but that's more work than is probably + # warranted right now. + t.targets = [n5, n6] + t.execute() + assert built_text == "n5 built", built_text + assert cache_text == [], cache_text + + built_text = None + cache_text = [] + n7 = Node("n7") + n8 = Node("n8") + n7.cached = 1 + n8.cached = 1 + tm = SCons.Taskmaster.Taskmaster([n7]) + t = tm.next_task() + # This next line is moderately bogus. We're just reaching + # in and setting the targets for this task to an array. The + # "right" way to do this would be to have the next_task() call + # set it up by having something that approximates a real Builder + # return this list--but that's more work than is probably + # warranted right now. + t.targets = [n7, n8] + t.execute() + assert built_text is None, built_text + assert cache_text == ["n7 retrieved", "n8 retrieved"], cache_text + def test_exception(self): """Test generic Taskmaster exception handling diff --git a/test/CacheDir.py b/test/CacheDir.py index a906b6a..44dd708 100644 --- a/test/CacheDir.py +++ b/test/CacheDir.py @@ -270,5 +270,33 @@ test.run(chdir = 'subdir') test.fail_test(os.path.exists(test.workpath('cache3', 'N', 'None'))) +############################################################################# +# Test that multiple target files get retrieved from cache correctly. + +test.subdir('multiple', 'cache4') + +test.write(['multiple', 'SConstruct'], """\ +CacheDir(r'%s') +env = Environment() +env.Command(['foo', 'bar'], ['input'], 'touch foo bar') +""" % (test.workpath('cache4'))) + +test.write(['multiple', 'input'], "multiple/input\n") + +test.run(chdir = 'multiple') + +test.fail_test(not os.path.exists(test.workpath('multiple', 'foo'))) +test.fail_test(not os.path.exists(test.workpath('multiple', 'bar'))) + +test.run(chdir = 'multiple', arguments = '-c') + +test.fail_test(os.path.exists(test.workpath('multiple', 'foo'))) +test.fail_test(os.path.exists(test.workpath('multiple', 'bar'))) + +test.run(chdir = 'multiple') + +test.fail_test(not os.path.exists(test.workpath('multiple', 'foo'))) +test.fail_test(not os.path.exists(test.workpath('multiple', 'bar'))) + # All done. test.pass_test() -- cgit v0.12