D3849: rebase: refactor dryrun implementation

Yuya Nishihara yuya at tcha.org
Thu Jun 28 08:31:16 EDT 2018


Queued, thanks.

I found a couple of nits. Can you send a follow up?

>      if dryrun:
> +        leaveunfinished = True
> +        inmemory = True
> +        rbsrt = rebaseruntime(repo, ui, inmemory, opts)
>          try:
>              overrides = {('rebase', 'singletransaction'): True}
>              with ui.configoverride(overrides, 'rebase'):
> -                _origrebase(ui, repo, inmemory=True, leaveunfinished=True,
> -                            **opts)
> +                _origrebase(ui, repo, inmemory=True, rbsrt=rbsrt,
> +                            leaveunfinished=leaveunfinished, **opts)
>          except error.InMemoryMergeConflictsError:
>              ui.status(_('hit a merge conflict\n'))
>              return 1
>          else:
>              ui.status(_('there will be no conflict, you can rebase\n'))
>              return 0
>          finally:
> -            _origrebase(ui, repo, abort=True)
> +            with repo.wlock(), repo.lock():
> +                rbsrt._prepareabortorcontinue(isabort=True)

We need a lock covering the whole dryrun process. Otherwise, another hg could
interrupt the dryrun before aborting.

Since we'll need one more indented block, I think it's time to extract dryrun
to a function.

> -def _origrebase(ui, repo, inmemory=False, leaveunfinished=False, **opts):
> +def _origrebase(ui, repo, inmemory=False, leaveunfinished=False, rbsrt=None,
> +                **opts):
>      opts = pycompat.byteskwargs(opts)
> -    rbsrt = rebaseruntime(repo, ui, inmemory, opts)
> +    if not rbsrt:
> +        rbsrt = rebaseruntime(repo, ui, inmemory, opts)

I think it's slightly better to split function than adding optional `rbsrt`
argument.

>      with repo.wlock(), repo.lock():
>          # Validate input and define rebasing points

Perhaps we can extract the post-lock block to new function, and the dryrun
function will call it in place of `_origrebase()`:

```
    rbsrt = rebaseruntime(repo, ui, inmemory, opts)
     with repo.wlock(), repo.lock():
         try:
             do in-memory rebase
         finally:
             rbsrt._prepareabortorcontinue(isabort=True)
```


More information about the Mercurial-devel mailing list