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