facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-11 01:07:15 +03:00
Code Issues Projects Releases Wiki Activity

smartfixup: add a config option to make it a subcommand of "amend"

Browse Source 
Summary:
Some people think this command should be a part of "amend". If you check the
meaning of the "amend" English word, that makes sense. But if you look at
the actual "amend" command, there are significant differences:

1. smartfixup rewrites a stack of changesets, and can even delete
   changesets, where amend only rewrites the working directory parent.
2. smartfixup is best-effort, i.e. does not guarantee that 100% of
   user-requested modifications will be included, where amend will just take
   100% (with "-i", it's 100% chunks selected by the user).
3. a lot of "amend" flags do not make much sense to smartfixup, like message
   editing (designed to edit a single changeset), "--addremove", "--secret",
   etc.
4. literally, smartfixup shares little code with the existing "amend" logic.
   "amend" is part of "fbamend" or "evolve". this extension should not
   depend on any of them.

So it's cleaner to be a separate command, not a part of `amend`.

However, I think it makes sense to have an option to satisfy those who want
to use "amend". So they can use "amend --related", "amend --fixups",
"amend --stack", "amend --auto" or whatever they choose. This diff adds such
a config option. We may also ship such a config option to make the command
easier for discovery.

Note the "amend" version is slightly different from the original smartfixup
command. The former targets basic users who expect amend to take all of
their changes, while the latter targets power users understanding what's
going on.

Therefore, the "amend" version will print extra information about what
changes are ignored, for example:

```
# changes not applied and left in working directory:
# M a : 1 modified chunks were ignored
# M c : unsupported file type (ex. binary or link)
# R b : removed files were ignored
```

To support the above change, `fixupstate.status` was added to avoid a second
"status" run and handles the "status" with interactive mode correctly. An
issue about symbolic links being added to `fixupstate.paths` was fixed by
the way.

Test Plan: Run the newly changed test.

Reviewers: #sourcecontrol, ttung, rmcelroy

Reviewed By: rmcelroy

Subscribers: rmcelroy, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3760498

Signature: t1:3760498:1472052376:7ddbfe763c7327d044b0d471c31a58fcb1e21dac
This commit is contained in:
Jun Wu 2016-08-24 16:28:15 +01:00
parent f99540d161
commit 350dae5587
2 changed files with 122 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
hgext3rd/smartfixup.py
View File

@ -17,6 +17,8 @@ amend modified chunks into the corresponding non-public changesets.
maxstacksize = 50
# whether to add noise to new commits to avoid obsolescence cycle
addnoise = 1
# make `amend --correlated` a shortcut to the main command
amendflag = correlated
[color]
smartfixup.node = blue bold
@ -34,6 +36,7 @@ from mercurial import (
context,
crecord,
error,
extensions,
mdiff,
node,
obsolete,
@ -437,6 +440,7 @@ class fixupstate(object):
# following fields will be filled later
self.paths = [] # [str]
self.status = None # ctx.status output
self.fixupmap = {} # {path: filefixupstate}
self.replacemap = {} # {oldnode: newnode or None}
self.finalnode = None # head after all fixups
@ -444,10 +448,11 @@ class fixupstate(object):
def diffwith(self, targetctx, match=None, showchanges=False):
"""diff and prepare fixups. update self.fixupmap, self.paths"""
# only care about modified files
self.paths = self.stack[-1].status(targetctx, match).modified
self.status = self.stack[-1].status(targetctx, match)
self.paths = []
# prepare the filefixupstate
pctx = self.stack[0].p1()
for path in self.paths:
for path in self.status.modified:
if self.ui.debugflag:
self.ui.write(_('calculating fixups for %s\n') % path)
targetfctx = targetctx[path]
@ -470,6 +475,7 @@ class fixupstate(object):
self.ui.write(header + '\n')
fstate.diffwith(targetfctx, showchanges=showchanges)
self.fixupmap[path] = fstate
self.paths.append(path)
def apply(self):
"""apply fixups to individual filefixupstates"""
@ -770,3 +776,73 @@ def smartfixupcmd(ui, repo, *pats, **opts):
state = smartfixup(ui, repo, pats=pats, opts=opts)
if sum(s[0] for s in state.chunkstats.values()) == 0:
return 1
def _wrapamend(flag):
"""add flag to amend, which will be a shortcut to the smartfixup command"""
if not flag:
return
amendcmd = extensions.bind(_amendcmd, flag)
# the amend command can exist in evolve, or fbamend
for extname in ['evolve', 'fbamend', None]:
try:
if extname is None:
cmdtable = commands.table
else:
ext = extensions.find(extname)
cmdtable = ext.cmdtable
except (KeyError, AttributeError):
continue
try:
entry = extensions.wrapcommand(cmdtable, 'amend', amendcmd)
options = entry[1]
msg = _('incorporate corrections into stack. '
'see \'hg help smartfixup\' for details')
options.append(('', flag, None, msg))
return
except error.UnknownCommand:
pass
def _amendcmd(flag, orig, ui, repo, *pats, **opts):
if not opts.get(flag):
return orig(ui, repo, *pats, **opts)
# use smartfixup
for k, v in opts.iteritems(): # check unsupported flags
if v and k not in ['interactive', flag]:
raise error.Abort(_('smartfixup does not support --%s')
% k.replace('_', '-'))
state = smartfixup(ui, repo, pats=pats, opts=opts)
# different from the original smartfixup, tell users what chunks were
# ignored and were left. it's because users usually expect "amend" to
# take all of their changes and will feel strange otherwise.
# the original "smartfixup" command faces more-advanced users knowing
# what's going on and is less verbose.
adoptedsum = 0
messages = []
for path, (adopted, total) in state.chunkstats.iteritems():
adoptedsum += adopted
if adopted == total:
continue
reason = _('%d modified chunks were ignored') % (total - adopted)
messages.append(('M', 'modified', path, reason))
for idx, word, symbol in [(0, 'modified', 'M'), (1, 'added', 'A'),
(2, 'removed', 'R'), (3, 'deleted', '!')]:
paths = set(state.status[idx]) - set(state.paths)
for path in sorted(paths):
if word == 'modified':
reason = _('unsupported file type (ex. binary or link)')
else:
reason = _('%s files were ignored') % word
messages.append((symbol, word, path, reason))
if messages:
ui.write(_('\n# changes not applied and left in '
'working directory:\n'))
for symbol, word, path, reason in messages:
ui.write(_('# %s %s : %s\n') % (
ui.label(symbol, 'status.' + word),
ui.label(path, 'status.' + word), reason))
if adoptedsum == 0:
return 1
def extsetup(ui):
_wrapamend(ui.config('smartfixup', 'amendflag', None))

44
tests/test-smartfixup.t
View File

@ -316,3 +316,47 @@ Test obsolete markers creation:
9:9354aeb6e762 commit b 1 851732d1c4d433cdd984d6b295158224b81dd717
8:568249511984 commit a 2 4438fcf42c600562ce2e74062b0a8ad7d246573f
7:e56aca308c01 commit a 1 f9a81da8dc53380ed91902e5b82c1b36255a4bd0
Test config option smartfixup.amendflag and running as a sub command of amend:
$ cat >> $TESTTMP/dummyamend.py << EOF
> from mercurial import cmdutil, commands
> cmdtable = {}
> command = cmdutil.command(cmdtable)
> @command('amend', [], '')
> def amend(ui, repo, *pats, **opts):
> return 3
> EOF
$ cat >> $HGRCPATH << EOF
> [extensions]
> fbamend=$TESTTMP/dummyamend.py
> [smartfixup]
> amendflag = correlated
> EOF
$ hg amend -h
hg amend
(no help text available)
options:
--correlated incorporate corrections into stack. see 'hg help smartfixup'
for details
(some details hidden, use --verbose to show complete help)
$ $PYTHON -c 'print("".join(map(chr, range(0,3))))' > c
$ hg commit -A c -m 'c is a binary file'
$ echo c >> c
$ sedi '2iINS' b
$ echo END >> b
$ hg rm a
$ hg amend --correlated
1 of 2 chunks(s) applied
# changes not applied and left in working directory:
# M b : 1 modified chunks were ignored
# M c : unsupported file type (ex. binary or link)
# R a : removed files were ignored