Attempt 4: Prevent reparenting a block with itself (#38747)

* More concisely reparent, ensuring we don't go too shallow or too deep in this process. Fixes #38357

* More explicit reparenting, with a short circuit for a common case

* We need new_block to have a parent, otherwise we lose context with this approach

* Remove duplicate parent assignment

* Change callers of Block.copy to not use exclude_parent=True, when including the parent, exclude tasks
This commit is contained in:
Matt Martz 2018-04-16 12:33:08 -05:00 committed by GitHub
parent d2ce1d3c26
commit f474195a3b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 18 additions and 21 deletions

View file

@ -524,7 +524,7 @@ class PlayIterator:
if state.tasks_child_state:
state.tasks_child_state = self._insert_tasks_into_state(state.tasks_child_state, task_list)
else:
target_block = state._blocks[state.cur_block].copy(exclude_parent=True)
target_block = state._blocks[state.cur_block].copy()
before = target_block.block[:state.cur_regular_task]
after = target_block.block[state.cur_regular_task:]
target_block.block = before + task_list + after
@ -533,7 +533,7 @@ class PlayIterator:
if state.rescue_child_state:
state.rescue_child_state = self._insert_tasks_into_state(state.rescue_child_state, task_list)
else:
target_block = state._blocks[state.cur_block].copy(exclude_parent=True)
target_block = state._blocks[state.cur_block].copy()
before = target_block.rescue[:state.cur_rescue_task]
after = target_block.rescue[state.cur_rescue_task:]
target_block.rescue = before + task_list + after
@ -542,7 +542,7 @@ class PlayIterator:
if state.always_child_state:
state.always_child_state = self._insert_tasks_into_state(state.always_child_state, task_list)
else:
target_block = state._blocks[state.cur_block].copy(exclude_parent=True)
target_block = state._blocks[state.cur_block].copy()
before = target_block.always[:state.cur_always_task]
after = target_block.always[state.cur_always_task:]
target_block.always = before + task_list + after

View file

@ -69,6 +69,10 @@ class Block(Base, Become, Conditional, Taggable):
'''object comparison based on _uuid'''
return self._uuid == other._uuid
def __ne__(self, other):
'''object comparison based on _uuid'''
return self._uuid != other._uuid
def get_vars(self):
'''
Blocks do not store variables directly, however they may be a member
@ -173,21 +177,16 @@ class Block(Base, Become, Conditional, Taggable):
new_task = task.copy(exclude_parent=True)
if task._parent:
new_task._parent = task._parent.copy(exclude_tasks=True)
# go up the parentage tree until we find an
# object without a parent and make this new
# block their parent
cur_obj = new_task
while cur_obj._parent:
if cur_obj._parent:
prev_obj = cur_obj
cur_obj = cur_obj._parent
# Ensure that we don't make the new_block the parent of itself
if cur_obj != new_block:
cur_obj._parent = new_block
if task._parent == new_block:
# If task._parent is the same as new_block, just replace it
new_task._parent = new_block
else:
# prev_obj._parent is cur_obj, to allow for mutability we need to use prev_obj
prev_obj._parent = new_block
# task may not be a direct child of new_block, search for the correct place to insert new_block
cur_obj = new_task._parent
while cur_obj._parent and cur_obj._parent != new_block:
cur_obj = cur_obj._parent
cur_obj._parent = new_block
else:
new_task._parent = new_block
new_task_list.append(new_task)
@ -203,7 +202,7 @@ class Block(Base, Become, Conditional, Taggable):
new_me._parent = None
if self._parent and not exclude_parent:
new_me._parent = self._parent.copy(exclude_tasks=exclude_tasks)
new_me._parent = self._parent.copy(exclude_tasks=True)
if not exclude_tasks:
new_me.block = _dupe_task_list(self.block or [], new_me)

View file

@ -410,9 +410,7 @@ class Role(Base, Become, Conditional, Taggable):
block_list.extend(dep_blocks)
for idx, task_block in enumerate(self._task_blocks):
new_task_block = task_block.copy(exclude_parent=True)
if task_block._parent:
new_task_block._parent = task_block._parent.copy()
new_task_block = task_block.copy()
new_task_block._dep_chain = new_dep_chain
new_task_block._play = play
if idx == len(self._task_blocks) - 1: