This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3557

extensions: new closehead module for closing arbitrary heads
ClosedPublic

Authored by joerg.sonnenberger on May 13 2018, 7:42 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGcd5f2e615262: extensions: new closehead module for closing arbitrary heads
Summary

`hg close-head` allows closing arbitrary heads. It is equivalent to
checking out the given revisions and commit an empty change with
`hg commit --close-branch`.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit added a subscriber: pulkit.May 20 2018, 3:52 PM

hg close-branch allows closing arbitrary heads. It is equivalent to
checking out the given revisions and commit an empty change with
hg commit --close-branch.

I am -1 on having a new command for this. How about adding this as a --close flag to hg branch command?

joerg.sonnenberger added a comment.May 21 2018, 5:03 PM

It doesn't seem to fit the argument schema for hg branch at all. I'm open for better places, but I couldn't think of a consistent place.

pulkit added a comment.May 25 2018, 7:30 AM

The code looks mostly good to me. Needs tests for following cases:

  • when working directory is dirty
  • when we are in an uncommitted merge state
  • there is some bookmark movement code, tests for that
mercurial/commands.py
1508

we use all lowercase error messages, so this should be "no commit ..".

pulkit added a comment.May 25 2018, 7:31 AM
In D3557#57279, @joerg.sonnenberger wrote:

It doesn't seem to fit the argument schema for hg branch at all. I'm open for better places, but I couldn't think of a consistent place.

The branch command has a --rev flag which we can use. Something like hg branch --rev <revs> --close. What do you think?

joerg.sonnenberger added a comment.May 25 2018, 7:45 AM

If we want to go with a --close flag, I think the most natural place would actually be hg heads. This is not really a branch operation after all, but about cutting off heads. At the same time, none of those options would allow specifying a commit message naturally.

indygreg added a subscriber: indygreg.May 30 2018, 9:03 PM

I'm -0.25 on this.

But I don't think we should add an argument to hg heads or hg branch because I don't like a) overloading commands to have multiple meanings b) making commands read-write and read-only. I think commands should do one thing and do them well. There should also be a set of safe commands that can be executed without meaningful side-effects.

Following that logic, a dedicated command is justified.

But I just don't feel like closing heads that aren't checked out is a frequent enough operation to justify a dedicated command.

As a potential compromise, how about hg commit --close-branch-rev <REV> that does the branch closing without needing a working directory? But this seemingly violates the expectation of hg commit, which is that it operates on the working directory. Gah - now I think I talked myself into a dedicated command.

Good UI design is hard.

joerg.sonnenberger edited the summary of this revision. (Show Details)Jun 3 2018, 11:57 AM
joerg.sonnenberger retitled this revision from commit: add new close-branch command to extensions: new closehead module for closing arbitrary heads.
joerg.sonnenberger updated this revision to Diff 8963.
pulkit added a comment.Jun 11 2018, 2:45 PM

Thanks for taking this out into a separate extension. Code looks mostly good to me. It feels to me that we need some more tests:

  • passing multiple revisions when some are head and some not
  • closing a head which has secret phase
  • checking for bookmark movement

I know the behavior in couple of the above points but having tests will help us in future when we change some behavior, like showing warning for revs which are not heads and closing others.

Also, wherever we pass revisions, we have support for --rev flag too. Can you add that too?

hgext/closehead.py
33 ↗(On Diff #8963)

close is too generic, I don't feel confident enough to have this as a command name. Can we drop that for now if you don't feel to strong?

53 ↗(On Diff #8963)

please add an opts = pycompat.byteskwargs(opts) call here for python 3 support.

69 ↗(On Diff #8963)

we don't start with uppercase in error messages

73 ↗(On Diff #8963)

nit: how about using context manager?

79 ↗(On Diff #8963)

I don't think this will ever go the else part.

tests/test-close-head.t
1 ↗(On Diff #8963)

init a new repo and cd into that. don't work in top level $TESTTMP directory.

43 ↗(On Diff #8963)

same, don't create a new inside existing repo, cd back and then create one.

pulkit added a comment.Jul 23 2018, 2:46 PM

Sorry for getting late to review this. Found a minor nit and also looks like you missed adding tests for bookmark movement and phase preservation.

hgext/closehead.py
55 ↗(On Diff #9400)

revs can be passed as opts['rev'] too now.

joerg.sonnenberger marked 5 inline comments as done.Sep 21 2018, 6:25 AM
joerg.sonnenberger updated this revision to Diff 11254.
joerg.sonnenberger marked 2 inline comments as done.Sep 21 2018, 6:26 AM

Adjusted for most of the review comments. Added a test that books are left alone, since they will not be active in the interesting cases. I'm not touching any existing phases, so I'm not sure if I should do anything about phase testing?

pulkit accepted this revision.Oct 3 2018, 8:23 AM
Closed by commit rHGcd5f2e615262: extensions: new closehead module for closing arbitrary heads (authored by joerg.sonnenberger). · Explain WhyOct 3 2018, 8:35 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 8680

mercurial/commands.py

short, short,
) )
from . import ( from . import (
archival, archival,
bookmarks, bookmarks,
bundle2, bundle2,
changegroup, changegroup,
cmdutil, cmdutil,
context,
copies, copies,
debugcommands as debugcommandsmod, debugcommands as debugcommandsmod,
destutil, destutil,
dirstateguard, dirstateguard,
discovery, discovery,
encoding, encoding,
error, error,
exchange, exchange,
stream=opts.get('stream') or opts.get('uncompressed'), stream=opts.get('stream') or opts.get('uncompressed'),
revs=opts.get('rev'), revs=opts.get('rev'),
update=opts.get('updaterev') or not opts.get('noupdate'), update=opts.get('updaterev') or not opts.get('noupdate'),
branch=opts.get('branch'), branch=opts.get('branch'),
shareopts=opts.get('shareopts')) shareopts=opts.get('shareopts'))
return r is None return r is None
@command('close-branch', commitopts + commitopts2,
_('[OPTION]... [REV]...'), inferrepo=True)
def close_branch(ui, repo, *revs, **opts):
"""close the given head revisions
This is equivalent to checking out each revision in a clean tree and running
``hg commit --close-branch``, except that it doesn't change the working
directory.
The commit message must be specified with -l or -m.
"""
def docommit(rev):
cctx = context.memctx(repo, parents=[rev, None], text=message,
files=[], filectxfn=None, user=opts.get('user'),
date=opts.get('date'), extra=extra)
tr = repo.transaction('commit')
ret = repo.commitctx(cctx, True)
bookmarks.update(repo, [rev, None], ret)
cctx.markcommitted(ret)
tr.close()
if not revs:
raise error.Abort(_('no revisions specified'))
revs = scmutil.revrange(repo, revs)
heads = []
for branch in repo.branchmap():
heads.extend(repo.branchheads(branch))
heads = set(repo[h].rev() for h in heads)
for rev in revs:
if rev not in heads:
raise error.Abort(_('revision is not an open head: %s') % rev)
message = cmdutil.logmessage(ui, opts)
if not message:
raise error.Abort(_("No commit message specified with -l or -m"))
pulkitUnsubmitted
Not Done

we use all lowercase error messages, so this should be "no commit ..".

pulkit: we use all lowercase error messages, so this should be "no commit ..".
extra = { 'close': '1' }
wlock = lock = None
try:
wlock = repo.wlock()
lock = repo.lock()
for rev in revs:
docommit(rev)
finally:
release(lock, wlock)
return 0
@command('^commit|ci', @command('^commit|ci',
[('A', 'addremove', None, [('A', 'addremove', None,
_('mark new/missing files as added/removed before committing')), _('mark new/missing files as added/removed before committing')),
('', 'close-branch', None, ('', 'close-branch', None,
_('mark a branch head as closed')), _('mark a branch head as closed')),
('', 'amend', None, _('amend the parent of the working directory')), ('', 'amend', None, _('amend the parent of the working directory')),
('s', 'secret', None, _('use the secret phase for committing')), ('s', 'secret', None, _('use the secret phase for committing')),
('e', 'edit', None, _('invoke editor on commit messages')), ('e', 'edit', None, _('invoke editor on commit messages')),

tests/test-close-branch.t

  • This file was added.
$ hg init
$ hg debugbuilddag '+2*2*3*4'
$ hg log -G --template '{rev}:{node|short}'
o 4:e7bd5218ca15
|
| o 3:6100d3090acf
|/
| o 2:fa942426a6fd
|/
| o 1:66f7d451a68b
|/
o 0:1ea73414a91b
$ hg close-branch -m 'Close old heads' 1 2
$ hg heads
changeset: 4:e7bd5218ca15
parent: 0:1ea73414a91b
user: debugbuilddag
date: Thu Jan 01 00:00:04 1970 +0000
summary: r4
changeset: 3:6100d3090acf
parent: 0:1ea73414a91b
user: debugbuilddag
date: Thu Jan 01 00:00:03 1970 +0000
summary: r3
$ hg close-branch -m 'Close more old heads' 4
$ hg heads
changeset: 3:6100d3090acf
parent: 0:1ea73414a91b
user: debugbuilddag
date: Thu Jan 01 00:00:03 1970 +0000
summary: r3
$ hg close-branch -m 'Not a head' 0
abort: revision is not an open head: 0
[255]
$ hg close-branch -m 'Already closed head' 1
abort: revision is not an open head: 1
[255]

tests/test-completion.t

Show all commands except debug commands Show all commands except debug commands
$ hg debugcomplete $ hg debugcomplete
add add
addremove addremove
annotate annotate
archive archive
backout backout
bisect bisect
bookmarks bookmarks
branch branch
branches branches
bundle bundle
cat cat
clone clone
close-branch
commit commit
config config
copy copy
diff diff
export export
files files
forget forget
graft graft
archive: no-decode, prefix, rev, type, subrepos, include, exclude archive: no-decode, prefix, rev, type, subrepos, include, exclude
backout: merge, commit, no-commit, parent, rev, edit, tool, include, exclude, message, logfile, date, user backout: merge, commit, no-commit, parent, rev, edit, tool, include, exclude, message, logfile, date, user
bisect: reset, good, bad, skip, extend, command, noupdate bisect: reset, good, bad, skip, extend, command, noupdate
bookmarks: force, rev, delete, rename, inactive, template bookmarks: force, rev, delete, rename, inactive, template
branch: force, clean, rev branch: force, clean, rev
branches: active, closed, template branches: active, closed, template
bundle: force, rev, branch, base, all, type, ssh, remotecmd, insecure bundle: force, rev, branch, base, all, type, ssh, remotecmd, insecure
cat: output, rev, decode, include, exclude, template cat: output, rev, decode, include, exclude, template
close-branch: message, logfile, date, user
config: untrusted, edit, local, global, template config: untrusted, edit, local, global, template
copy: after, force, include, exclude, dry-run copy: after, force, include, exclude, dry-run
debugancestor: debugancestor:
debugapplystreamclonebundle: debugapplystreamclonebundle:
debugbuilddag: mergeable-file, overwritten-file, new-file debugbuilddag: mergeable-file, overwritten-file, new-file
debugbundle: all, part-type, spec debugbundle: all, part-type, spec
debugcapabilities: debugcapabilities:
debugcheckstate: debugcheckstate:

tests/test-globalopts.t

backout reverse effect of earlier changeset backout reverse effect of earlier changeset
bisect subdivision search of changesets bisect subdivision search of changesets
bookmarks create a new bookmark or list existing bookmarks bookmarks create a new bookmark or list existing bookmarks
branch set or show the current branch name branch set or show the current branch name
branches list repository named branches branches list repository named branches
bundle create a bundle file bundle create a bundle file
cat output the current or given revision of files cat output the current or given revision of files
clone make a copy of an existing repository clone make a copy of an existing repository
close-branch close the given head revisions
commit commit the specified files or all outstanding changes commit commit the specified files or all outstanding changes
config show combined config settings from all hgrc files config show combined config settings from all hgrc files
copy mark files as copied for the next commit copy mark files as copied for the next commit
diff diff repository (or selected files) diff diff repository (or selected files)
export dump the header and diffs for one or more changesets export dump the header and diffs for one or more changesets
files list tracked files files list tracked files
forget forget the specified files on the next commit forget forget the specified files on the next commit
graft copy changes from other branches onto the current branch graft copy changes from other branches onto the current branch
backout reverse effect of earlier changeset backout reverse effect of earlier changeset
bisect subdivision search of changesets bisect subdivision search of changesets
bookmarks create a new bookmark or list existing bookmarks bookmarks create a new bookmark or list existing bookmarks
branch set or show the current branch name branch set or show the current branch name
branches list repository named branches branches list repository named branches
bundle create a bundle file bundle create a bundle file
cat output the current or given revision of files cat output the current or given revision of files
clone make a copy of an existing repository clone make a copy of an existing repository
close-branch close the given head revisions
commit commit the specified files or all outstanding changes commit commit the specified files or all outstanding changes
config show combined config settings from all hgrc files config show combined config settings from all hgrc files
copy mark files as copied for the next commit copy mark files as copied for the next commit
diff diff repository (or selected files) diff diff repository (or selected files)
export dump the header and diffs for one or more changesets export dump the header and diffs for one or more changesets
files list tracked files files list tracked files
forget forget the specified files on the next commit forget forget the specified files on the next commit
graft copy changes from other branches onto the current branch graft copy changes from other branches onto the current branch

tests/test-help.t

backout reverse effect of earlier changeset backout reverse effect of earlier changeset
bisect subdivision search of changesets bisect subdivision search of changesets
bookmarks create a new bookmark or list existing bookmarks bookmarks create a new bookmark or list existing bookmarks
branch set or show the current branch name branch set or show the current branch name
branches list repository named branches branches list repository named branches
bundle create a bundle file bundle create a bundle file
cat output the current or given revision of files cat output the current or given revision of files
clone make a copy of an existing repository clone make a copy of an existing repository
close-branch close the given head revisions
commit commit the specified files or all outstanding changes commit commit the specified files or all outstanding changes
config show combined config settings from all hgrc files config show combined config settings from all hgrc files
copy mark files as copied for the next commit copy mark files as copied for the next commit
diff diff repository (or selected files) diff diff repository (or selected files)
export dump the header and diffs for one or more changesets export dump the header and diffs for one or more changesets
files list tracked files files list tracked files
forget forget the specified files on the next commit forget forget the specified files on the next commit
graft copy changes from other branches onto the current branch graft copy changes from other branches onto the current branch
backout reverse effect of earlier changeset backout reverse effect of earlier changeset
bisect subdivision search of changesets bisect subdivision search of changesets
bookmarks create a new bookmark or list existing bookmarks bookmarks create a new bookmark or list existing bookmarks
branch set or show the current branch name branch set or show the current branch name
branches list repository named branches branches list repository named branches
bundle create a bundle file bundle create a bundle file
cat output the current or given revision of files cat output the current or given revision of files
clone make a copy of an existing repository clone make a copy of an existing repository
close-branch close the given head revisions
commit commit the specified files or all outstanding changes commit commit the specified files or all outstanding changes
config show combined config settings from all hgrc files config show combined config settings from all hgrc files
copy mark files as copied for the next commit copy mark files as copied for the next commit
diff diff repository (or selected files) diff diff repository (or selected files)
export dump the header and diffs for one or more changesets export dump the header and diffs for one or more changesets
files list tracked files files list tracked files
forget forget the specified files on the next commit forget forget the specified files on the next commit
graft copy changes from other branches onto the current branch graft copy changes from other branches onto the current branch
backout reverse effect of earlier changeset backout reverse effect of earlier changeset
bisect subdivision search of changesets bisect subdivision search of changesets
bookmarks create a new bookmark or list existing bookmarks bookmarks create a new bookmark or list existing bookmarks
branch set or show the current branch name branch set or show the current branch name
branches list repository named branches branches list repository named branches
bundle create a bundle file bundle create a bundle file
cat output the current or given revision of files cat output the current or given revision of files
clone make a copy of an existing repository clone make a copy of an existing repository
close-branch close the given head revisions
commit commit the specified files or all outstanding changes commit commit the specified files or all outstanding changes
config show combined config settings from all hgrc files config show combined config settings from all hgrc files
copy mark files as copied for the next commit copy mark files as copied for the next commit
diff diff repository (or selected files) diff diff repository (or selected files)
export dump the header and diffs for one or more changesets export dump the header and diffs for one or more changesets
files list tracked files files list tracked files
forget forget the specified files on the next commit forget forget the specified files on the next commit
graft copy changes from other branches onto the current branch graft copy changes from other branches onto the current branch
<tr><td> <tr><td>
<a href="/help/cat"> <a href="/help/cat">
cat cat
</a> </a>
</td><td> </td><td>
output the current or given revision of files output the current or given revision of files
</td></tr> </td></tr>
<tr><td> <tr><td>
<a href="/help/close-branch">
close-branch
</a>
</td><td>
close the given head revisions
</td></tr>
<tr><td>
<a href="/help/config"> <a href="/help/config">
config config
</a> </a>
</td><td> </td><td>
show combined config settings from all hgrc files show combined config settings from all hgrc files
</td></tr> </td></tr>
<tr><td> <tr><td>
<a href="/help/copy"> <a href="/help/copy">

tests/test-hgweb-json.t

"summary": "create a bundle file", "summary": "create a bundle file",
"topic": "bundle" "topic": "bundle"
}, },
{ {
"summary": "output the current or given revision of files", "summary": "output the current or given revision of files",
"topic": "cat" "topic": "cat"
}, },
{ {
"summary": "close the given head revisions",
"topic": "close-branch"
},
{
"summary": "show combined config settings from all hgrc files", "summary": "show combined config settings from all hgrc files",
"topic": "config" "topic": "config"
}, },
{ {
"summary": "mark files as copied for the next commit", "summary": "mark files as copied for the next commit",
"topic": "copy" "topic": "copy"
}, },
{ {