From e60435be9b8f51eac5f095c23e83ca16c0434bcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Farkas?= Date: Sat, 2 Jan 2016 18:49:41 +0100 Subject: [PATCH] Keep the last of the post tasks during deduplication. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The problem was that if both `a` and `b` tasks have same `pre` and `post` hooks, the `a b` was extended and deduplicated to `pre a post b`. But the post hook means that it must be run after the original task. With this commit there are 3 rules of deduplication: - don't touch direct task calls (`a a b` must remain the same) - keep only the first of the same pre-hooked tasks - keep only the last of the same post-hooked tasks Fixes #298 Signed-off-by: Máté Farkas --- invoke/executor.py | 28 +++++++++++++++++++--------- invoke/tasks.py | 6 ++++++ sites/www/changelog.rst | 3 +++ tests/executor.py | 13 ++++++++++++- 4 files changed, 40 insertions(+), 10 deletions(-) diff --git a/invoke/executor.py b/invoke/executor.py index 955f33d0a..3015f3125 100644 --- a/invoke/executor.py +++ b/invoke/executor.py @@ -94,15 +94,20 @@ def execute(self, *tasks): # Expand pre/post tasks & then dedupe the entire run. # Load config at this point to get latest value of dedupe option config = self.config.clone() - expanded = self.expand_calls(calls, config) + expanded = self.expand_calls(calls, 'direct', config) # Get some good value for dedupe option, even if config doesn't have # the tree we expect. (This is a concession to testing.) try: dedupe = config.tasks.dedupe except AttributeError: dedupe = True + calls = expanded # Actual deduping here - calls = self.dedupe(expanded) if dedupe else expanded + if dedupe: + # Keep the first 'pre' tasks + calls = self.dedupe(calls, 'pre') + # Keep the last 'post' tasks + calls = reversed(self.dedupe(reversed(calls), 'post')) # Execute results = {} for call in calls: @@ -141,7 +146,7 @@ def normalize(self, tasks): calls = [Call(task=self.collection[self.collection.default])] return calls - def dedupe(self, calls): + def dedupe(self, calls, hook_type): """ Deduplicate a list of `tasks <.Call>`. @@ -150,16 +155,19 @@ def dedupe(self, calls): :returns: A list of `.Call` objects. """ deduped = [] - debug("Deduplicating tasks...") + debug("Deduplicating {0!r} tasks...".format(hook_type)) for call in calls: - if call not in deduped: + if call.hook_type != hook_type: + debug("{0!r}: different type, don't handle now".format(call)) + deduped.append(call) + elif call not in deduped: debug("{0!r}: no duplicates found, ok".format(call)) deduped.append(call) else: debug("{0!r}: found in list already, skipping".format(call)) return deduped - def expand_calls(self, calls, config): + def expand_calls(self, calls, hook_type, config): """ Expand a list of `.Call` objects into a near-final list of same. @@ -175,7 +183,9 @@ def expand_calls(self, calls, config): # Normalize to Call (this method is sometimes called with pre/post # task lists, which may contain 'raw' Task objects) if isinstance(call, Task): - call = Call(task=call) + call = Call(task=call, hook_type=hook_type) + else: + call.hook_type = hook_type debug("Expanding task-call {0!r}".format(call)) if call.contextualized: debug("Task was contextualized, loading additional configuration") # noqa @@ -185,9 +195,9 @@ def expand_calls(self, calls, config): # NOTE: handing in original config, not the mutated one handed to # the Context above. Pre/post tasks may well come from a different # collection, etc. Also just cleaner. - ret.extend(self.expand_calls(call.pre, config)) + ret.extend(self.expand_calls(call.pre, 'pre', config)) ret.append(call) - ret.extend(self.expand_calls(call.post, config)) + ret.extend(self.expand_calls(call.post, 'post', config)) return ret def config_for(self, call, config, anonymous=False): diff --git a/invoke/tasks.py b/invoke/tasks.py index 8e00e18b8..e48f903ba 100644 --- a/invoke/tasks.py +++ b/invoke/tasks.py @@ -339,6 +339,7 @@ def __init__( args=None, kwargs=None, context=None, + hook_type=None ): """ Create a new `.Call` object. @@ -359,12 +360,17 @@ def __init__( :param context: `.Context` instance to be used if the wrapped `.Task` is :ref:`contextualized `. Default: ``None``. + + :param hook_type: + The type of the actual task calling. Used for deduplication. + Can be 'direct', 'pre', or 'post'. """ self.task = task self.called_as = called_as self.args = args or tuple() self.kwargs = kwargs or dict() self.context = context + self.hook_type = hook_type def __getattr__(self, name): return getattr(self.task, name) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 90c41e25a..4f6126579 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,9 @@ Changelog ========= +* :bug:`298 major` During the deduplication keep the last of the post-hooked + task, not the first. (Make sure that the post task run after the original + task) * :support:`144` Add code-coverage reporting to our CI builds (albeit `CodeCov `_ instead of `coveralls.io `_). Includes rejiggering our project-specific coverage-generating tasks. Thanks diff --git a/tests/executor.py b/tests/executor.py index 0b9bf7eeb..24e61c849 100644 --- a/tests/executor.py +++ b/tests/executor.py @@ -169,8 +169,8 @@ def deduping(self): foo bar boz -post2 post1 +post2 """) def no_deduping(self): @@ -182,6 +182,17 @@ def no_deduping(self): post2 post1 post2 +""") + + class shared_pre_and_post_hooks: + def deduping(self): + self._expect('biz boz', """ +foo +bar +biz +boz +post1 +post2 """) # AKA, a (foo) (foo -> bar) scenario arising from foo + bar