From a4dcf5406f3abd84496ed21bd2f72c53574bad4a Mon Sep 17 00:00:00 2001 From: stevenknight Date: Sat, 14 May 2005 14:32:44 +0000 Subject: [PATCH] Move pre- and post-actions lists from Node to Executor so expansions of ${TARGETS[1:]} work, and the actions aren't executed multiple times. git-svn-id: http://scons.tigris.org/svn/scons/trunk@1295 fdb21ef1-2011-0410-befe-b5e4ea1792b1 --- src/CHANGES.txt | 3 + src/engine/SCons/Environment.py | 16 +++-- src/engine/SCons/Executor.py | 62 ++++++++++------ src/engine/SCons/ExecutorTests.py | 71 ++++++++++++++++--- src/engine/SCons/Node/FS.py | 7 +- src/engine/SCons/Node/FSTests.py | 16 +++++ src/engine/SCons/Node/NodeTests.py | 49 ------------- src/engine/SCons/Node/__init__.py | 20 +----- test/{ => Actions}/actions.py | 0 test/{append-action.py => Actions/append.py} | 0 .../pre-post.py} | 35 ++++++++- 11 files changed, 171 insertions(+), 108 deletions(-) rename test/{ => Actions}/actions.py (100%) rename test/{append-action.py => Actions/append.py} (100%) rename test/{pre-post-actions.py => Actions/pre-post.py} (82%) diff --git a/src/CHANGES.txt b/src/CHANGES.txt index 0763a00e..5edaafb5 100644 --- a/src/CHANGES.txt +++ b/src/CHANGES.txt @@ -279,6 +279,9 @@ RELEASE 0.97 - XXX of Python function actions, so that changing the location of an otherwise unmodified Python function doesn't cause rebuilds. + - Fix AddPreAction() and AddPostAction() when an action has more than + one target file: attach the actions to the Executor, not the Node. + From Wayne Lee: - Avoid "maximum recursion limit" errors when removing $(-$) pairs diff --git a/src/engine/SCons/Environment.py b/src/engine/SCons/Environment.py index 58440d6b..0e9b642b 100644 --- a/src/engine/SCons/Environment.py +++ b/src/engine/SCons/Environment.py @@ -1125,15 +1125,21 @@ class Base(SubstitutionEnvironment): def AddPreAction(self, files, action): nodes = self.arg2nodes(files, self.fs.Entry) action = SCons.Action.Action(action) - for n in nodes: - n.add_pre_action(action) + uniq = {} + for executor in map(lambda n: n.get_executor(), nodes): + uniq[executor] = 1 + for executor in uniq.keys(): + executor.add_pre_action(action) return nodes - + def AddPostAction(self, files, action): nodes = self.arg2nodes(files, self.fs.Entry) action = SCons.Action.Action(action) - for n in nodes: - n.add_post_action(action) + uniq = {} + for executor in map(lambda n: n.get_executor(), nodes): + uniq[executor] = 1 + for executor in uniq.keys(): + executor.add_post_action(action) return nodes def Alias(self, target, source=[], action=None, **kw): diff --git a/src/engine/SCons/Executor.py b/src/engine/SCons/Executor.py index afd6c499..d8bcafbf 100644 --- a/src/engine/SCons/Executor.py +++ b/src/engine/SCons/Executor.py @@ -30,6 +30,7 @@ Nodes. __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__" +import string from SCons.Debug import logInstanceCreation import SCons.Util @@ -48,15 +49,25 @@ class Executor: def __init__(self, action, env=None, overridelist=[{}], targets=[], sources=[], builder_kw={}): if __debug__: logInstanceCreation(self, 'Executor.Executor') - if not action: - raise SCons.Errors.UserError, "Executor must have an action." - self.action = action + self.set_action_list(action) + self.pre_actions = [] + self.post_actions = [] self.env = env self.overridelist = overridelist self.targets = targets self.sources = sources[:] self.builder_kw = builder_kw + def set_action_list(self, action): + if not SCons.Util.is_List(action): + if not action: + raise SCons.Errors.UserError, "Executor must have an action." + action = [action] + self.action_list = action + + def get_action_list(self): + return self.pre_actions + self.action_list + self.post_actions + def get_build_env(self): """Fetch or create the appropriate build Environment for this Executor. @@ -98,9 +109,12 @@ class Executor: def do_execute(self, target, exitstatfunc, kw): """Actually execute the action list.""" - apply(self.action, - (self.targets, self.sources, self.get_build_env(), exitstatfunc), - self.get_kw(kw)) + env = self.get_build_env() + kw = self.get_kw(kw) + for act in self.get_action_list(): + apply(act, + (self.targets, self.sources, env, exitstatfunc), + kw) # use extra indirection because with new-style objects (Python 2.2 # and above) we can't override special methods, and nullify() needs @@ -120,12 +134,20 @@ class Executor: slist = filter(lambda x, s=self.sources: x not in s, sources) self.sources.extend(slist) + def add_pre_action(self, action): + self.pre_actions.append(action) + + def add_post_action(self, action): + self.post_actions.append(action) + # another extra indirection for new-style objects and nullify... def my_str(self): - return self.action.genstring(self.targets, - self.sources, - self.get_build_env()) + env = self.get_build_env() + get = lambda action, t=self.targets, s=self.sources, e=env: \ + action.genstring(t, s, e) + return string.join(map(get, self.get_action_list()), "\n") + def __str__(self): "__cacheable__" @@ -143,9 +165,10 @@ class Executor: or source Nodes there are. __cacheable__ """ - return self.action.get_contents(self.targets, - self.sources, - self.get_build_env()) + env = self.get_build_env() + get = lambda action, t=self.targets, s=self.sources, e=env: \ + action.get_contents(t, s, e) + return string.join(map(get, self.get_action_list()), "") def get_timestamp(self): """Fetch a time stamp for this Executor. We don't have one, of @@ -208,8 +231,9 @@ class Executor: return map(func, self.get_unignored_sources(ignore)) +_Executor = Executor -class Null: +class Null(_Executor): """A null Executor, with a null build Environment, that does nothing when the rest of the methods call it. @@ -217,8 +241,10 @@ class Null: disassociate Builders from Nodes entirely, so we're not going to worry about unit tests for this--at least for now. """ - def __init__(self): + def __init__(self, *args, **kw): if __debug__: logInstanceCreation(self, 'Executor.Null') + kw['action'] = [] + apply(_Executor.__init__, (self,), kw) def get_build_env(self): class NullEnvironment: def get_scanner(self, key): @@ -226,16 +252,8 @@ class Null: return NullEnvironment() def get_build_scanner_path(self): return None - def __call__(self, *args, **kw): - pass def cleanup(self): pass - def get_missing_sources(self): - return [] - def get_unignored_sources(self, ignore=[]): - return [] - def process_sources(self, func, ignore=[]): - return [] diff --git a/src/engine/SCons/ExecutorTests.py b/src/engine/SCons/ExecutorTests.py index 9ff30e58..fd163221 100644 --- a/src/engine/SCons/ExecutorTests.py +++ b/src/engine/SCons/ExecutorTests.py @@ -103,7 +103,7 @@ class ExecutorTestCase(unittest.TestCase): """Test creating an Executor""" source_list = ['s1', 's2'] x = SCons.Executor.Executor('a', 'e', ['o'], 't', source_list) - assert x.action == 'a', x.builder + assert x.action_list == ['a'], x.action_list assert x.env == 'e', x.env assert x.overridelist == ['o'], x.overridelist assert x.targets == 't', x.targets @@ -116,6 +116,26 @@ class ExecutorTestCase(unittest.TestCase): else: raise "Did not catch expected UserError" + def test__action_list(self): + """Test the {get,set}_action_list() methods""" + x = SCons.Executor.Executor('a', 'e', 'o', 't', ['s1', 's2']) + + l = x.get_action_list() + assert l == ['a'], l + + x.add_pre_action('pre') + x.add_post_action('post') + l = x.get_action_list() + assert l == ['pre', 'a', 'post'], l + + x.set_action_list('b') + l = x.get_action_list() + assert l == ['pre', 'b', 'post'], l + + x.set_action_list(['c']) + l = x.get_action_list() + assert l == ['pre', 'c', 'post'], l + def test_get_build_env(self): """Test fetching and generating a build environment""" x = SCons.Executor.Executor(MyAction(), MyEnvironment(e=1), [], @@ -196,11 +216,12 @@ class ExecutorTestCase(unittest.TestCase): env = MyEnvironment() a = MyAction([action1, action2]) - b = MyBuilder(env, {}) - b.action = a - n = MyNode('n', [pre], [post]) - n.builder = b - n.build() + t = MyNode('t') + + x = SCons.Executor.Executor(a, env, [], t, ['s1', 's2']) + x.add_pre_action(pre) + x.add_post_action(post) + x(t, lambda x: x) assert result == ['pre', 'action1', 'action2', 'post'], result del result[:] @@ -210,9 +231,10 @@ class ExecutorTestCase(unittest.TestCase): errfunc(1) return 1 - n = MyNode('n', [pre_err], [post]) - n.builder = b - n.build() + x = SCons.Executor.Executor(a, env, [], t, ['s1', 's2']) + x.add_pre_action(pre_err) + x.add_post_action(post) + x(t, lambda x: x) assert result == ['pre_err', 'action1', 'action2', 'post'], result del result[:] @@ -220,7 +242,7 @@ class ExecutorTestCase(unittest.TestCase): raise "errfunc %s" % stat try: - n.build(errfunc) + x(t, errfunc) except: assert sys.exc_type == "errfunc 1", sys.exc_type else: @@ -257,6 +279,22 @@ class ExecutorTestCase(unittest.TestCase): x.add_sources(['s3', 's1', 's4']) assert x.sources == ['s1', 's2', 's3', 's4'], x.sources + def test_add_pre_action(self): + """Test adding pre-actions to an Executor""" + x = SCons.Executor.Executor('b', 'e', 'o', 't', ['s1', 's2']) + x.add_pre_action('a1') + assert x.pre_actions == ['a1'] + x.add_pre_action('a2') + assert x.pre_actions == ['a1', 'a2'] + + def test_add_post_action(self): + """Test adding post-actions to an Executor""" + x = SCons.Executor.Executor('b', 'e', 'o', 't', ['s1', 's2']) + x.add_post_action('a1') + assert x.post_actions == ['a1'] + x.add_post_action('a2') + assert x.post_actions == ['a1', 'a2'] + def test___str__(self): """Test the __str__() method""" env = MyEnvironment(S='string') @@ -265,6 +303,15 @@ class ExecutorTestCase(unittest.TestCase): c = str(x) assert c == 'GENSTRING action1 action2 t s', c + x = SCons.Executor.Executor(MyAction(), env, [], ['t'], ['s']) + x.add_pre_action(MyAction(['pre'])) + x.add_post_action(MyAction(['post'])) + c = str(x) + expect = 'GENSTRING pre t s\n' + \ + 'GENSTRING action1 action2 t s\n' + \ + 'GENSTRING post t s' + assert c == expect, c + def test_nullify(self): """Test the nullify() method""" env = MyEnvironment(S='string') @@ -301,8 +348,10 @@ class ExecutorTestCase(unittest.TestCase): x = SCons.Executor.Executor(MyAction(actions=['grow']), env, [], ['t'], ['s']) + x.add_pre_action(MyAction(['pre'])) + x.add_post_action(MyAction(['post'])) c = x.get_contents() - assert c == 'grow t s', c + assert c == 'pre t sgrow t spost t s', c def test_get_timestamp(self): """Test fetching the "timestamp" """ diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py index f2234f76..5ae14fc3 100644 --- a/src/engine/SCons/Node/FS.py +++ b/src/engine/SCons/Node/FS.py @@ -1058,11 +1058,16 @@ class Dir(Base): self.entries['.'] = self self.entries['..'] = self.dir self.cwd = self - self.builder = get_MkdirBuilder() self.searched = 0 self._sconsign = None self.build_dirs = [] + # Don't just reset the executor, replace its action list, + # because it might have some pre-or post-actions that need to + # be preserved. + self.builder = get_MkdirBuilder() + self.get_executor().set_action_list(self.builder.action) + def disambiguate(self): return self diff --git a/src/engine/SCons/Node/FSTests.py b/src/engine/SCons/Node/FSTests.py index 86261856..6978a03a 100644 --- a/src/engine/SCons/Node/FSTests.py +++ b/src/engine/SCons/Node/FSTests.py @@ -307,6 +307,7 @@ class BuildDirTestCase(unittest.TestCase): try: dir_made = [] d9.builder = Builder(fs.Dir, action=MkdirAction(dir_made)) + d9.reset_executor() f9.exists() expect = os.path.join('build', 'var2', 'new_dir') assert dir_made[0].path == expect, dir_made[0].path @@ -895,6 +896,7 @@ class FSTestCase(_tempdirTestCase): assert not built_it d1.add_source([SCons.Node.Node()]) # XXX FAKE SUBCLASS ATTRIBUTE d1.builder_set(Builder(fs.File)) + d1.reset_executor() d1.env_set(Environment()) d1.build() assert built_it @@ -903,6 +905,7 @@ class FSTestCase(_tempdirTestCase): assert not built_it f1.add_source([SCons.Node.Node()]) # XXX FAKE SUBCLASS ATTRIBUTE f1.builder_set(Builder(fs.File)) + f1.reset_executor() f1.env_set(Environment()) f1.build() assert built_it @@ -1343,6 +1346,18 @@ class FSTestCase(_tempdirTestCase): class DirTestCase(_tempdirTestCase): + def test__morph(self): + """Test handling of actions when morphing an Entry into a Dir""" + test = self.test + e = self.fs.Entry('eee') + x = e.get_executor() + x.add_pre_action('pre') + x.add_post_action('post') + e.must_be_a_Dir() + a = x.get_action_list() + assert a[0] == 'pre', a + assert a[2] == 'post', a + def test_entry_exists_on_disk(self): """Test the Dir.entry_exists_on_disk() method """ @@ -2179,6 +2194,7 @@ class prepareTestCase(unittest.TestCase): dir_made = [] new_dir = fs.Dir("new_dir") new_dir.builder = Builder(fs.Dir, action=MkdirAction(dir_made)) + new_dir.reset_executor() xyz = fs.File(os.path.join("new_dir", "xyz")) xyz.set_state(SCons.Node.up_to_date) diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py index 67ac05f6..70b9672a 100644 --- a/src/engine/SCons/Node/NodeTests.py +++ b/src/engine/SCons/Node/NodeTests.py @@ -106,24 +106,6 @@ class MyListAction(MyActionBase): def __call__(self, target, source, env, errfunc): for A in self.list: A(target, source, env, errfunc) - -class MyNonGlobalAction(MyActionBase): - def __init__(self): - self.order = 0 - self.built_it = None - self.built_target = None - self.built_source = None - - def __call__(self, target, source, env, errfunc): - # Okay, so not ENTIRELY non-global... - global built_order - self.built_it = 1 - self.built_target = target - self.built_source = source - self.built_args = env - built_order = built_order + 1 - self.order = built_order - return 0 class Environment: def __init__(self, **kw): @@ -310,37 +292,6 @@ class NodeTestCase(unittest.TestCase): assert built_args["on"] == 3, built_args assert built_args["off"] == 4, built_args - built_it = None - built_order = 0 - node = MyNode("xxx") - node.builder_set(Builder()) - node.env_set(Environment()) - node.sources = ["yyy", "zzz"] - pre1 = MyNonGlobalAction() - pre2 = MyNonGlobalAction() - post1 = MyNonGlobalAction() - post2 = MyNonGlobalAction() - node.add_pre_action(pre1) - node.add_pre_action(pre2) - node.add_post_action(post1) - node.add_post_action(post2) - node.build() - assert built_it - assert pre1.built_it - assert pre2.built_it - assert post1.built_it - assert post2.built_it - assert pre1.order == 1, pre1.order - assert pre2.order == 2, pre1.order - # The action of the builder itself is order 3... - assert post1.order == 4, pre1.order - assert post2.order == 5, pre1.order - - for act in [ pre1, pre2, post1, post2 ]: - assert type(act.built_target[0]) == type(MyNode("bar")), type(act.built_target[0]) - assert str(act.built_target[0]) == "xxx", str(act.built_target[0]) - assert act.built_source == ["yyy", "zzz"], act.built_source - def test_get_build_scanner_path(self): """Test the get_build_scanner_path() method""" n = SCons.Node.Node() diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py index f9390eab..4a1faaa0 100644 --- a/src/engine/SCons/Node/__init__.py +++ b/src/engine/SCons/Node/__init__.py @@ -179,12 +179,8 @@ class Node: try: act = self.builder.action except AttributeError: - executor = SCons.Executor.Null() + executor = SCons.Executor.Null(targets=[self]) else: - if self.pre_actions: - act = self.pre_actions + act - if self.post_actions: - act = act + self.post_actions executor = SCons.Executor.Executor(act, self.env or self.builder.env, [self.builder.overrides], @@ -833,20 +829,6 @@ class Node: the command interpreter literally.""" return 1 - def add_pre_action(self, act): - """Adds an Action performed on this Node only before - building it.""" - self.pre_actions.append(act) - # executor must be recomputed to include new pre-actions - self.reset_executor() - - def add_post_action(self, act): - """Adds and Action performed on this Node only after - building it.""" - self.post_actions.append(act) - # executor must be recomputed to include new pre-actions - self.reset_executor() - def render_include_tree(self): """ Return a text representation, suitable for displaying to the diff --git a/test/actions.py b/test/Actions/actions.py similarity index 100% rename from test/actions.py rename to test/Actions/actions.py diff --git a/test/append-action.py b/test/Actions/append.py similarity index 100% rename from test/append-action.py rename to test/Actions/append.py diff --git a/test/pre-post-actions.py b/test/Actions/pre-post.py similarity index 82% rename from test/pre-post-actions.py rename to test/Actions/pre-post.py index 2d8458c6..e25def50 100644 --- a/test/pre-post-actions.py +++ b/test/Actions/pre-post.py @@ -34,10 +34,11 @@ import sys import TestSCons _exe = TestSCons._exe +python = TestSCons.python test = TestSCons.TestSCons() -test.subdir('work1', 'work2', 'work3') +test.subdir('work1', 'work2', 'work3', 'work4') @@ -170,4 +171,36 @@ test.must_match(['work3', 'dir', 'file'], "build()\n") +test.write(['work4', 'build.py'], """\ +import sys +outfp = open(sys.argv[1], 'wb') +for f in sys.argv[2:]: + outfp.write(open(f, 'rb').read()) +outfp.close() +""") + +test.write(['work4', 'SConstruct'], """\ +def pre_action(target, source, env): + open(str(target[0]), 'ab').write('pre %%s\\n' %% source[0]) +def post_action(target, source, env): + open(str(target[0]), 'ab').write('post %%s\\n' %% source[0]) +env = Environment() +o = env.Command(['pre-post', 'file.out'], + 'file.in', + "%(python)s build.py ${TARGETS[1]} $SOURCE") +env.AddPreAction(o, pre_action) +env.AddPostAction(o, post_action) +""" % locals()) + +test.write(['work4', 'file.in'], "file.in\n") + +test.run(chdir='work4', arguments='.') + +test.must_match(['work4', 'file.out'], "file.in\n") +test.must_match(['work4', 'pre-post'], "pre file.in\npost file.in\n") + +test.pass_test() + + + test.pass_test() -- 2.26.2