This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1232

rebase: add the --inmemory option flag; assign a wctx object for the rebase
ClosedPublic

Authored by phillco on Oct 26 2017, 12:48 AM.

Details

Reviewers
dlax
durin42
Group Reviewers
hg-reviewers
Commits
rHG482614b3802a: rebase: add the --inmemory option flag; assign a wctx object for the rebase
Summary

In the future, the --inmemory flag might be deprecated in favor of something more
intelligent (for example, always rebasing in-memory if the working copy parent
isn't in the rebaseset). But we might keep it as a way to explicitly force IMM
on or off.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

phillco updated this revision to Diff 4042.Dec 1 2017, 3:09 AM
dlax requested changes to this revision.Dec 1 2017, 4:01 AM
dlax added a subscriber: dlax.

In the future, the --inmemory flag might be deprecated in favor of something more intelligent

Perhaps the option should be flagged experimental then?

Also, would be nice to have some test coverage.

hgext/rebase.py
738

Not sure this is needed.

This revision now requires changes to proceed.Dec 1 2017, 4:01 AM
durin42 added a subscriber: durin42.Dec 7 2017, 2:47 PM

I've gone through this series and landed some of the easy stuff. Let's get the comments on this one addressed and rebase the rest and see if we can get it in before everyone goes on Christmas vacations?

phillco updated this revision to Diff 4178.Dec 7 2017, 4:22 PM
phillco updated this revision to Diff 4196.Dec 7 2017, 4:29 PM
phillco added a comment.Dec 7 2017, 4:29 PM

Perhaps the option should be flagged experimental then?

Good point, thanks.

durin42 accepted this revision.Dec 7 2017, 4:31 PM
Closed by commit rHG482614b3802a: rebase: add the --inmemory option flag; assign a wctx object for the rebase (authored by phillco). · Explain WhyDec 7 2017, 5:14 PM
This revision was automatically updated to reflect the committed changes.
dlax added inline comments.Dec 8 2017, 3:43 AM
hgext/rebase.py
738

This is useless because "inmemory" will be in opts and because you wrote opts.get('inmemory', False) in rebaseruntime.__init__() above.

dlax added a comment.Dec 8 2017, 5:01 AM

dlax (Denis Laxalde) wrote:

Also, would be nice to have some test coverage.

Unless I missed it, there's still no test at all for the --inmemory
option in this series.

smf added a subscriber: smf.Dec 11 2017, 6:59 PM

dlax (Denis Laxalde) <phabricator@mercurial-scm.org> writes:

dlax requested changes to this revision.
dlax added a comment.
This revision now requires changes to proceed.

> In the future, the --inmemory flag might be deprecated in favor of something more intelligent
Perhaps the option should be flagged experimental then?
Also, would be nice to have some test coverage.

We can't easily get rid of command-line options so this should be
'experimental.inmemory' or similar. Also, I view inmemory as an
implementation detail that shouldn't be exposed to the user.

durin42 added a comment.Dec 11 2017, 7:14 PM

Phil is planning to follow up with the config knob, and I hope to test this aggressively at Google and make it default if we don't find any bugs.

smf added a comment.Dec 11 2017, 7:24 PM

durin42 (Augie Fackler) <phabricator@mercurial-scm.org> writes:

durin42 added a comment.

Phil is planning to follow up with the config knob, and I hope to test this aggressively at Google and make it default if we don't find any bugs.

Same for me here (it's what we already do basically).

phillco added a comment.Dec 11 2017, 7:48 PM

Given that most users will be using the config knob (and eventually, nothing at all), I can just send a followup to rename to experimental.inmmemory. Nobody is using it yet.

hgext/rebase.py
738

Thanks. I'll send a followup.

Revision Contents
Changeset List

PathPackages
Mhgext/rebase.py (10 lines)

Diff 4220

hgext/rebase.py

self.keepf = opts.get('keep', False) self.keepf = opts.get('keep', False)
self.keepbranchesf = opts.get('keepbranches', False) self.keepbranchesf = opts.get('keepbranches', False)
# keepopen is not meant for use on the command line, but by # keepopen is not meant for use on the command line, but by
# other extensions # other extensions
self.keepopen = opts.get('keepopen', False) self.keepopen = opts.get('keepopen', False)
self.obsoletenotrebased = {} self.obsoletenotrebased = {}
self.obsoletewithoutsuccessorindestination = set() self.obsoletewithoutsuccessorindestination = set()
self.inmemory = opts.get('inmemory', False)
@property @property
def repo(self): def repo(self):
if self.prepared: if self.prepared:
return self._repo.unfiltered() return self._repo.unfiltered()
else: else:
return self._repo return self._repo
dest = self.repo[destrev] dest = self.repo[destrev]
if dest.closesbranch() and not self.keepbranchesf: if dest.closesbranch() and not self.keepbranchesf:
self.ui.status(_('reopening closed branch head %s\n') % dest) self.ui.status(_('reopening closed branch head %s\n') % dest)
self.prepared = True self.prepared = True
def _performrebase(self, tr): def _performrebase(self, tr):
repo, ui = self.repo, self.ui repo, ui = self.repo, self.ui
# Assign a working copy object.
if self.inmemory:
from mercurial.context import overlayworkingctx
self.wctx = overlayworkingctx(self.repo)
else:
self.wctx = self.repo[None]
if self.keepbranchesf: if self.keepbranchesf:
# insert _savebranch at the start of extrafns so if # insert _savebranch at the start of extrafns so if
# there's a user-provided extrafn it can clobber branch if # there's a user-provided extrafn it can clobber branch if
# desired # desired
self.extrafns.insert(0, _savebranch) self.extrafns.insert(0, _savebranch)
if self.collapsef: if self.collapsef:
branches = set() branches = set()
for rev in self.state: for rev in self.state:
('l', 'logfile', '', ('l', 'logfile', '',
_('read collapse commit message from file'), _('FILE')), _('read collapse commit message from file'), _('FILE')),
('k', 'keep', False, _('keep original changesets')), ('k', 'keep', False, _('keep original changesets')),
('', 'keepbranches', False, _('keep original branch names')), ('', 'keepbranches', False, _('keep original branch names')),
('D', 'detach', False, _('(DEPRECATED)')), ('D', 'detach', False, _('(DEPRECATED)')),
('i', 'interactive', False, _('(DEPRECATED)')), ('i', 'interactive', False, _('(DEPRECATED)')),
('t', 'tool', '', _('specify merge tool')), ('t', 'tool', '', _('specify merge tool')),
('c', 'continue', False, _('continue an interrupted rebase')), ('c', 'continue', False, _('continue an interrupted rebase')),
('', 'inmemory', False, _('run rebase in-memory (EXPERIMENTAL)')),
('a', 'abort', False, _('abort an interrupted rebase'))] + ('a', 'abort', False, _('abort an interrupted rebase'))] +
cmdutil.formatteropts, cmdutil.formatteropts,
_('[-s REV | -b REV] [-d REV] [OPTION]')) _('[-s REV | -b REV] [-d REV] [OPTION]'))
def rebase(ui, repo, **opts): def rebase(ui, repo, **opts):
"""move changeset (and descendants) to a different branch """move changeset (and descendants) to a different branch
Rebase uses repeated merging to graft changesets from one part of Rebase uses repeated merging to graft changesets from one part of
history (the source) onto another (the destination). This can be history (the source) onto another (the destination). This can be
Return Values: Return Values:
Returns 0 on success, 1 if nothing to rebase or there are Returns 0 on success, 1 if nothing to rebase or there are
unresolved conflicts. unresolved conflicts.
""" """
opts = pycompat.byteskwargs(opts) opts = pycompat.byteskwargs(opts)
if 'inmemory' not in opts:
opts['inmemory'] = False
dlaxUnsubmitted
Not Done

Not sure this is needed.

dlax: Not sure this is needed.
dlaxUnsubmitted
Not Done

This is useless because "inmemory" will be in opts and because you wrote opts.get('inmemory', False) in rebaseruntime.__init__() above.

dlax: This is useless because "inmemory" will be in `opts` and because you wrote `opts.get('inmemory'…
phillcoAuthorUnsubmitted
Not Done

Thanks. I'll send a followup.

phillco: Thanks. I'll send a followup.
rbsrt = rebaseruntime(repo, ui, opts) rbsrt = rebaseruntime(repo, ui, opts)
with repo.wlock(), repo.lock(): with repo.wlock(), repo.lock():
# Validate input and define rebasing points # Validate input and define rebasing points
destf = opts.get('dest', None) destf = opts.get('dest', None)
srcf = opts.get('source', None) srcf = opts.get('source', None)
basef = opts.get('base', None) basef = opts.get('base', None)
revf = opts.get('rev', []) revf = opts.get('rev', [])