[PATCH] rebase: update _rebaseset inside transaction

[Pierre-Yves David]

On 11/10/2016 11:56 AM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham at fb.com>
> # Date 1478778918 28800
> #      Thu Nov 10 03:55:18 2016 -0800
> # Node ID f3bfa374cea66cd5bf17645ccb6eef60c4d8a5aa
> # Parent  3fd53cc1aad882ac9191d7388885acdbbc2d7103
> rebase: update _rebaseset inside transaction
>
> Rebase has the concept of the _rebaseset, which is used to prevent the commits
> being rebased from being hidden until after the rebase is complete. Previously,
> the _rebaseset was being cleared at the very end of rebase, after the last
> transaction had already closed. This meant that the repo filteredrevs cache was
> not updated (since no invalidation had been triggered, since we're outside of a
> transaction), which meant future changelog reads saw commits as visible that
> should've been hidden.
>
> This patch moves the _rebaseset clearing to be inside the last rebase
> transaction, so that after the transaction the filteredrevs is appropriately
> invalidated and will be up-to-date on the next read.
>
> This showed up in an extension that combines rebase, obsolete, and inhibit, so
> I'm not sure how to create a test case in hg core to repro it. But we have a
> test case in the extension repo.
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -495,6 +495,12 @@ class rebaseruntime(object):
>                  if self.activebookmark not in repo._bookmarks:
>                      # active bookmark was divergent one and has been deleted
>                      self.activebookmark = None
> +            # Clear the _rebaseset as part of the transaction, so
> +            # post-transaction hooks can see the final repo visibility state.
> +            # (We could've moved clearstatus() into the transaction to do this,
> +            # but it contains non-transactional changes, like unlinking files).
> +            _clearrebasesetvisibiliy(repo)
> +
>          clearstatus(repo)
>          clearcollapsemsg(repo)

The clearing of the "rebased" revision happens a couple of line higher 
(line 489). Would it make more sense to clear the visible set at the 
same time instead of the proposed spot ?

Cheers,

-- 
Pierre-Yves David