From f8e0d6d523f37a813eae06bc1d3a24cf49aa8d49 Mon Sep 17 00:00:00 2001 From: Steven Knight Date: Wed, 30 Mar 2005 04:05:49 +0000 Subject: Make sure scans are added to all targets in a builder call, to prevent out-of-order -j builds. --- src/CHANGES.txt | 4 +++ src/engine/SCons/Executor.py | 33 +++++++++++++-------- src/engine/SCons/ExecutorTests.py | 41 +++++++++++++++++++------- src/engine/SCons/Node/NodeTests.py | 42 ++++++++++++++++++++------- src/engine/SCons/Node/__init__.py | 17 +++++------ src/engine/SCons/Taskmaster.py | 58 +++++++++++++++++-------------------- src/engine/SCons/TaskmasterTests.py | 43 ++++++++++++++++++++++++--- 7 files changed, 160 insertions(+), 78 deletions(-) diff --git a/src/CHANGES.txt b/src/CHANGES.txt index f677819..7e2e68a 100644 --- a/src/CHANGES.txt +++ b/src/CHANGES.txt @@ -248,6 +248,10 @@ RELEASE 0.97 - XXX - Fix re-scanning of generated source files for implicit dependencies when the -j option is used. + - Fix a dependency problem that caused $LIBS scans to not be added + to all of the targets in a multiple-target builder call, which + could cause out-of-order builds when the -j option is used. + From Wayne Lee: - Avoid "maximum recursion limit" errors when removing $(-$) pairs diff --git a/src/engine/SCons/Executor.py b/src/engine/SCons/Executor.py index fb15661..ef460ef 100644 --- a/src/engine/SCons/Executor.py +++ b/src/engine/SCons/Executor.py @@ -154,29 +154,38 @@ class Executor: """ return 0 - def scan(self, scanner): - """Scan this Executor's source files for implicit dependencies - and update all of the targets with them. This essentially - short-circuits an N^2 scan of the sources for each individual - targets, which is a hell of a lot more efficient. + def scan_targets(self, scanner): + self.scan(scanner, self.targets) + + def scan_sources(self, scanner): + self.scan(scanner, self.sources) + + def scan(self, scanner, node_list): + """Scan a list of this Executor's files (targets or sources) for + implicit dependencies and update all of the targets with them. + This essentially short-circuits an N*M scan of the sources for + each individual target, which is a hell of a lot more efficient. """ env = self.get_build_env() select_specific_scanner = lambda t: (t[0], t[1].select(t[0])) remove_null_scanners = lambda t: not t[1] is None - add_scanner_path = lambda t, s=self: (t[0], t[1], s.get_build_scanner_path(t[1])) + add_scanner_path = lambda t, s=self: \ + (t[0], t[1], s.get_build_scanner_path(t[1])) if scanner: - initial_scanners = lambda src, s=scanner: (src, s) + scanner_list = map(lambda src, s=scanner: (src, s), node_list) else: kw = self.get_kw() - initial_scanners = lambda src, e=env, kw=kw: (src, src.get_scanner(e, kw)) - scanner_list = map(initial_scanners, self.sources) - scanner_list = filter(remove_null_scanners, scanner_list) + get_initial_scanners = lambda src, e=env, kw=kw: \ + (src, src.get_scanner(e, kw)) + scanner_list = map(get_initial_scanners, node_list) + scanner_list = filter(remove_null_scanners, scanner_list) + scanner_list = map(select_specific_scanner, scanner_list) scanner_list = filter(remove_null_scanners, scanner_list) scanner_path_list = map(add_scanner_path, scanner_list) deps = [] - for src, scanner, path in scanner_path_list: - deps.extend(src.get_implicit_deps(env, scanner, path)) + for node, scanner, path in scanner_path_list: + deps.extend(node.get_implicit_deps(env, scanner, path)) for tgt in self.targets: tgt.add_to_implicit(deps) diff --git a/src/engine/SCons/ExecutorTests.py b/src/engine/SCons/ExecutorTests.py index d16b137..62c1eab 100644 --- a/src/engine/SCons/ExecutorTests.py +++ b/src/engine/SCons/ExecutorTests.py @@ -310,22 +310,43 @@ class ExecutorTestCase(unittest.TestCase): ts = x.get_timestamp() assert ts == 0, ts - def test_scan(self): + def test_scan_targets(self): + """Test scanning the targets for implicit dependencies""" + env = MyEnvironment(S='string') + t1 = MyNode('t1') + t2 = MyNode('t2') + sources = [MyNode('s1'), MyNode('s2')] + x = SCons.Executor.Executor('b', env, [{}], [t1, t2], sources) + + deps = x.scan_targets(None) + assert t1.implicit == ['dep-t1', 'dep-t2'], t1.implicit + assert t2.implicit == ['dep-t1', 'dep-t2'], t2.implicit + + t1.implicit = [] + t2.implicit = [] + + deps = x.scan_targets(MyScanner('scanner-')) + assert t1.implicit == ['scanner-t1', 'scanner-t2'], t1.implicit + assert t2.implicit == ['scanner-t1', 'scanner-t2'], t2.implicit + + def test_scan_sources(self): """Test scanning the sources for implicit dependencies""" env = MyEnvironment(S='string') - targets = [MyNode('t')] + t1 = MyNode('t1') + t2 = MyNode('t2') sources = [MyNode('s1'), MyNode('s2')] - x = SCons.Executor.Executor('b', env, [{}], targets, sources) + x = SCons.Executor.Executor('b', env, [{}], [t1, t2], sources) - deps = x.scan(None) - t = targets[0] - assert t.implicit == ['dep-s1', 'dep-s2'], t.implicit + deps = x.scan_sources(None) + assert t1.implicit == ['dep-s1', 'dep-s2'], t1.implicit + assert t2.implicit == ['dep-s1', 'dep-s2'], t2.implicit - t.implicit = [] + t1.implicit = [] + t2.implicit = [] - deps = x.scan(MyScanner('scanner-')) - t = targets[0] - assert t.implicit == ['scanner-s1', 'scanner-s2'], t.implicit + deps = x.scan_sources(MyScanner('scanner-')) + assert t1.implicit == ['scanner-s1', 'scanner-s2'], t1.implicit + assert t2.implicit == ['scanner-s1', 'scanner-s2'], t2.implicit def test_get_missing_sources(self): """Test the ability to check if any sources are missing""" diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py index 3660b11..a8bb2fa 100644 --- a/src/engine/SCons/Node/NodeTests.py +++ b/src/engine/SCons/Node/NodeTests.py @@ -77,10 +77,28 @@ class MyAction(MyActionBase): return 0 class MyExecutor: + def __init__(self, env=None, targets=[], sources=[]): + self.env = env + self.targets = targets + self.sources = sources + def get_build_env(self): + return self.env def get_build_scanner_path(self, scanner): return 'executor would call %s' % scanner def cleanup(self): self.cleaned_up = 1 + def scan_targets(self, scanner): + if not scanner: + return + d = scanner(self.targets) + for t in self.targets: + t.implicit.extend(d) + def scan_sources(self, scanner): + if not scanner: + return + d = scanner(self.sources) + for t in self.targets: + t.implicit.extend(d) class MyListAction(MyActionBase): def __init__(self, list): @@ -794,38 +812,40 @@ class NodeTestCase(unittest.TestCase): assert deps == [], deps s = Scanner() - d = MyNode("ddd") - node.found_includes = [d] + d1 = MyNode("d1") + d2 = MyNode("d2") + node.found_includes = [d1, d2] # Simple return of the found includes deps = node.get_implicit_deps(env, s, target) - assert deps == [d], deps + assert deps == [d1, d2], deps # By default, our fake scanner recurses e = MyNode("eee") f = MyNode("fff") g = MyNode("ggg") - d.found_includes = [e, f] + d1.found_includes = [e, f] + d2.found_includes = [e, f] f.found_includes = [g] deps = node.get_implicit_deps(env, s, target) - assert deps == [d, e, f, g], map(str, deps) + assert deps == [d1, d2, e, f, g], map(str, deps) # Recursive scanning eliminates duplicates e.found_includes = [f] deps = node.get_implicit_deps(env, s, target) - assert deps == [d, e, f, g], map(str, deps) + assert deps == [d1, d2, e, f, g], map(str, deps) # Scanner method can select specific nodes to recurse def no_fff(nodes): return filter(lambda n: str(n)[0] != 'f', nodes) s.recurse_nodes = no_fff deps = node.get_implicit_deps(env, s, target) - assert deps == [d, e, f], map(str, deps) + assert deps == [d1, d2, e, f], map(str, deps) # Scanner method can short-circuit recursing entirely s.recurse_nodes = lambda nodes: [] deps = node.get_implicit_deps(env, s, target) - assert deps == [d], map(str, deps) + assert deps == [d1, d2], map(str, deps) def test_get_scanner(self): """Test fetching the environment scanner for a Node @@ -881,11 +901,13 @@ class NodeTestCase(unittest.TestCase): def test_scan(self): """Test Scanner functionality """ + env = Environment() node = MyNode("nnn") node.builder = Builder() - node.env_set(Environment()) - s = Scanner() + node.env_set(env) + x = MyExecutor(env, [node]) + s = Scanner() d = MyNode("ddd") node.found_includes = [d] diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py index e1f872f..8cac2c8 100644 --- a/src/engine/SCons/Node/__init__.py +++ b/src/engine/SCons/Node/__init__.py @@ -495,19 +495,16 @@ class Node: self._children_reset() self.del_binfo() - scanner = self.builder.source_scanner - self.get_executor().scan(scanner) + executor = self.get_executor() + + # Have the executor scan the sources. + executor.scan_sources(self.builder.source_scanner) - # scan this node itself for implicit dependencies + # If there's a target scanner, have the executor scan the target + # node itself and associated targets that might be built. scanner = self.builder.target_scanner if scanner: - path = self.get_build_scanner_path(scanner) - deps = self.get_implicit_deps(build_env, scanner, path) - self._add_child(self.implicit, self.implicit_dict, deps) - - # XXX See note above re: --implicit-cache. - #if implicit_cache: - # self.store_implicit() + executor.scan_targets(scanner) def scanner_key(self): return None diff --git a/src/engine/SCons/Taskmaster.py b/src/engine/SCons/Taskmaster.py index 7b58f4b..2f7da74 100644 --- a/src/engine/SCons/Taskmaster.py +++ b/src/engine/SCons/Taskmaster.py @@ -55,18 +55,17 @@ class Stats: StatsNodes = [] -fmt = "%(considered)5d" \ - " %(already_handled)5d" \ - " %(problem)5d" \ - " %(child_failed)5d" \ - " %(not_started)5d" \ - " %(not_built)5d" \ - " %(side_effects)5d" \ - " %(build)5d" \ - " " +fmt = "%(considered)3d "\ + "%(already_handled)3d " \ + "%(problem)3d " \ + "%(child_failed)3d " \ + "%(not_started)3d " \ + "%(not_built)3d " \ + "%(side_effects)3d " \ + "%(build)3d " def dump_stats(): - StatsNodes.sort(lambda a, b: cmp(a.name, b.name)) + StatsNodes.sort(lambda a, b: cmp(str(a), str(b))) for n in StatsNodes: print (fmt % n.stats.__dict__) + str(n) @@ -164,15 +163,13 @@ class Task: things. Most importantly, this calls back to the Taskmaster to put any node tasks waiting on this one back on the pending list.""" - - if self.targets[0].get_state() == SCons.Node.executing: - for t in self.targets: + for t in self.targets: + if t.get_state() == SCons.Node.executing: for side_effect in t.side_effects: side_effect.set_state(SCons.Node.no_state) t.set_state(SCons.Node.executed) t.built() - else: - for t in self.targets: + else: t.visited() self.tm.executed(self.node) @@ -210,16 +207,6 @@ class Task: self.tm.executed(self.node) - def mark_targets(self, state): - for t in self.targets: - t.set_state(state) - - def mark_targets_and_side_effects(self, state): - for t in self.targets: - for side_effect in t.side_effects: - side_effect.set_state(state) - t.set_state(state) - def make_ready_all(self): """Mark all targets in a task ready for execution. @@ -227,7 +214,10 @@ class Task: visited--the canonical example being the "scons -c" option. """ self.out_of_date = self.targets[:] - self.mark_targets_and_side_effects(SCons.Node.executing) + for t in self.targets: + for s in t.side_effects: + s.set_state(SCons.Node.executing) + t.set_state(SCons.Node.executing) def make_ready_current(self): """Mark all targets in a task ready for execution if any target @@ -235,11 +225,15 @@ class Task: This is the default behavior for building only what's necessary. """ - self.out_of_date = filter(lambda T: not T.current(), self.targets) - if self.out_of_date: - self.mark_targets_and_side_effects(SCons.Node.executing) - else: - self.mark_targets(SCons.Node.up_to_date) + self.out_of_date = [] + for t in self.targets: + if t.current(): + t.set_state(SCons.Node.up_to_date) + else: + self.out_of_date.append(t) + for s in t.side_effects: + s.set_state(SCons.Node.executing) + t.set_state(SCons.Node.executing) make_ready = make_ready_current @@ -423,7 +417,7 @@ class Taskmaster: # put back on the candidates list when appropriate. self.pending.append(node) node.set_state(SCons.Node.pending) - if S: S.not_built = S.built + 1 + if S: S.not_built = S.not_built + 1 continue # Skip this node if it has side-effects that are diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py index e30ca19..62d3d6a 100644 --- a/src/engine/SCons/TaskmasterTests.py +++ b/src/engine/SCons/TaskmasterTests.py @@ -45,9 +45,10 @@ class Node: self.cached = 0 self.scanned = 0 self.scanner = None + self.targets = [self] class Builder: def targets(self, node): - return [node] + return node.targets self.builder = Builder() self.bsig = None self.csig = None @@ -430,6 +431,19 @@ class TaskmasterTestCase(unittest.TestCase): tm = SCons.Taskmaster.Taskmaster([n2]) assert tm.next_task() is None + n1 = Node("n1") + n2 = Node("n2") + n1.targets = [n1, n2] + n1._current_val = 1 + tm = SCons.Taskmaster.Taskmaster([n1]) + t = tm.next_task() + t.executed() + + s = n1.get_state() + assert s == SCons.Node.up_to_date, s + s = n2.get_state() + assert s == SCons.Node.executed, s + def test_make_ready_out_of_date(self): """Test the Task.make_ready() method's list of out-of-date Nodes @@ -701,7 +715,9 @@ class TaskmasterTestCase(unittest.TestCase): built_text = "xxx" visited_nodes = [] n1.set_state(SCons.Node.executing) + t.executed() + s = n1.get_state() assert s == SCons.Node.executed, s assert built_text == "xxx really", built_text @@ -713,11 +729,30 @@ class TaskmasterTestCase(unittest.TestCase): built_text = "should_not_change" visited_nodes = [] n2.set_state(None) + t.executed() - s = n1.get_state() - assert s == SCons.Node.executed, s + + s = n2.get_state() + assert s is None, s assert built_text == "should_not_change", built_text - assert visited_nodes == ["n2"], visited_nodes + assert visited_nodes == ['n2'], visited_nodes + + n3 = Node("n3") + n4 = Node("n4") + n3.targets = [n3, n4] + tm = SCons.Taskmaster.Taskmaster([n3]) + t = tm.next_task() + visited_nodes = [] + n3.set_state(SCons.Node.up_to_date) + n4.set_state(SCons.Node.executing) + + t.executed() + + s = n3.get_state() + assert s == SCons.Node.up_to_date, s + s = n4.get_state() + assert s == SCons.Node.executed, s + assert visited_nodes == ['n3'], visited_nodes def test_prepare(self): """Test preparation of multiple Nodes for a task -- cgit v0.12