( )⚙ D6989 push: support config option to require revs be specified when running push

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6989

push: support config option to require revs be specified when running push
ClosedPublic

Authored by spectral on Oct 5 2019, 4:52 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG5617b748aad8: push: support config option to require revs be specified when running push

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

spectral created this revision.Oct 5 2019, 4:52 PM
indygreg added a subscriber: indygreg.Oct 5 2019, 6:00 PM

Question: should this checking be performed in exchange.push or at the command layer?

(I'm not sure of the answer.)

spectral added a comment.Oct 6 2019, 9:50 AM
In D6989#102437, @indygreg wrote:

Question: should this checking be performed in exchange.push or at the command layer?
(I'm not sure of the answer.)

I think where it is makes sense - I'd be wary of pushing it down any further having a much larger area of effect (or, conversely, no effect - it's possible that further down we always have the revs specified). This is also where it is for commands.update.requiredest.

mercurial/commands.py
5616

Do we want to have this have the old behavior if plain? commands.update.requiredest doesn't respect plain. I don't think that it makes sense (or at least - it doesn't make sense to have a category for it: HGPLAINEXCEPT=make-me-specify-revs-to-push seems like it won't really be useful. :)

mharbison72 added a subscriber: mharbison72.Oct 7 2019, 11:18 AM
In D6989#102437, @indygreg wrote:

Question: should this checking be performed in exchange.push or at the command layer?
(I'm not sure of the answer.)

A problem with moving it to exchange is that subrepos implicitly (AFAICT) push all revisions with exchange.push, even when a subset of revisions in the parent is pushed.

pulkit accepted this revision.Oct 8 2019, 3:40 PM
pulkit added a subscriber: pulkit.

The current place for the check looks good. Can you follow-up by adding it to ui.tweakdefaults?

This revision is now accepted and ready to land.Oct 8 2019, 3:40 PM
pulkit added a comment.Oct 8 2019, 3:43 PM

Hm, looks like this needs to be rebased on tip of hg-committed because of recent blackening of codebase.

spectral updated this revision to Diff 17011.Oct 9 2019, 5:39 PM
pulkit accepted this revision.Oct 10 2019, 9:22 AM

Revision Contents
Changeset List

Diff 17011

mercurial/commands.py

elif path.pushrev: elif path.pushrev:
# It doesn't make any sense to specify ancestor revisions. So limit # It doesn't make any sense to specify ancestor revisions. So limit
# to DAG heads to make discovery simpler. # to DAG heads to make discovery simpler.
expr = revsetlang.formatspec(b'heads(%r)', path.pushrev) expr = revsetlang.formatspec(b'heads(%r)', path.pushrev)
revs = scmutil.revrange(repo, [expr]) revs = scmutil.revrange(repo, [expr])
revs = [repo[rev].node() for rev in revs] revs = [repo[rev].node() for rev in revs]
if not revs: if not revs:
raise error.Abort( raise error.Abort(
_(b'default push revset for path evaluates to an empty set') _(b'default push revset for path evaluates to an empty set')
spectralAuthorUnsubmitted
Done

Do we want to have this have the old behavior if plain? commands.update.requiredest doesn't respect plain. I don't think that it makes sense (or at least - it doesn't make sense to have a category for it: HGPLAINEXCEPT=make-me-specify-revs-to-push seems like it won't really be useful. :)

spectral: Do we want to have this have the old behavior if plain? commands.update.requiredest doesn't…
) )
elif ui.configbool(b'commands', b'push.require-revs'):
raise error.Abort(_(b'no revisions specified to push'),
hint=_(b'did you mean "hg push -r ."?'))
repo._subtoppath = dest repo._subtoppath = dest
try: try:
# push subrepos depth-first for coherent ordering # push subrepos depth-first for coherent ordering
c = repo[b'.'] c = repo[b'.']
subs = c.substate # only repos that are committed subs = c.substate # only repos that are committed
for s in sorted(subs): for s in sorted(subs):
result = c.sub(s).push(opts) result = c.sub(s).push(opts)

mercurial/configitems.py

_registerdiffopts(section=b'commands', configprefix=b'commit.interactive.') _registerdiffopts(section=b'commands', configprefix=b'commit.interactive.')
coreconfigitem( coreconfigitem(
b'commands', b'commit.post-status', default=False, b'commands', b'commit.post-status', default=False,
) )
coreconfigitem( coreconfigitem(
b'commands', b'grep.all-files', default=False, experimental=True, b'commands', b'grep.all-files', default=False, experimental=True,
) )
coreconfigitem( coreconfigitem(
b'commands', b'push.require-revs', default=False,
)
coreconfigitem(
b'commands', b'resolve.confirm', default=False, b'commands', b'resolve.confirm', default=False,
) )
coreconfigitem( coreconfigitem(
b'commands', b'resolve.explicit-re-merge', default=False, b'commands', b'resolve.explicit-re-merge', default=False,
) )
coreconfigitem( coreconfigitem(
b'commands', b'resolve.mark-check', default=b'none', b'commands', b'resolve.mark-check', default=b'none',
) )

mercurial/help/config.txt

``commands`` ``commands``
------------ ------------
``commit.post-status`` ``commit.post-status``
Show status of files in the working directory after successful commit. Show status of files in the working directory after successful commit.
(default: False) (default: False)
``push.require-revs``
Require revisions to push be specified using one or more mechanisms such as
specifying them positionally on the command line, using ``-r``, ``-b``,
and/or ``-B`` on the command line, or using ``paths.<path>:pushrev`` in the
configuration. If this is enabled and revisions are not specified, the
command aborts.
(default: False)
``resolve.confirm`` ``resolve.confirm``
Confirm before performing action if no filename is passed. Confirm before performing action if no filename is passed.
(default: False) (default: False)
``resolve.explicit-re-merge`` ``resolve.explicit-re-merge``
Require uses of ``hg resolve`` to specify which action it should perform, Require uses of ``hg resolve`` to specify which action it should perform,
instead of re-merging files by default. instead of re-merging files by default.
(default: False) (default: False)

tests/test-push.t

abort: no suitable response from remote hg! abort: no suitable response from remote hg!
[255] [255]
$ hg -R test-revflag push 'ssh://fakehost%7Ctouch%20owned/path' $ hg -R test-revflag push 'ssh://fakehost%7Ctouch%20owned/path'
pushing to ssh://fakehost%7Ctouch%20owned/path pushing to ssh://fakehost%7Ctouch%20owned/path
abort: no suitable response from remote hg! abort: no suitable response from remote hg!
[255] [255]
$ [ ! -f owned ] || echo 'you got owned' $ [ ! -f owned ] || echo 'you got owned'
Test `commands.push.require-revs`
---------------------------------
$ hg clone -q test-revflag test-require-revs-source
$ hg init test-require-revs-dest
$ cd test-require-revs-source
$ cat >> .hg/hgrc << EOF
> [paths]
> default = ../test-require-revs-dest
> [commands]
> push.require-revs=1
> EOF
$ hg push
pushing to $TESTTMP/test-require-revs-dest
abort: no revisions specified to push
(did you mean "hg push -r ."?)
[255]
$ hg push -r 0
pushing to $TESTTMP/test-require-revs-dest
searching for changes
adding changesets
adding manifests
adding file changes
added 1 changesets with 1 changes to 1 files
$ hg bookmark -r 0 push-this-bookmark
(test that -B (bookmark) works for specifying "revs")
$ hg push -B push-this-bookmark
pushing to $TESTTMP/test-require-revs-dest
searching for changes
no changes found
exporting bookmark push-this-bookmark
[1]
(test that -b (branch) works for specifying "revs")
$ hg push -b default
pushing to $TESTTMP/test-require-revs-dest
searching for changes
abort: push creates new remote head [0-9a-f]+! (re)
(merge or see 'hg help push' for details about pushing new heads)
[255]
(demonstrate that even though we don't have anything to exchange, we're still
showing the error)
$ hg push
pushing to $TESTTMP/test-require-revs-dest
abort: no revisions specified to push
(did you mean "hg push -r ."?)
[255]
$ hg push --config paths.default:pushrev=0
pushing to $TESTTMP/test-require-revs-dest
searching for changes
no changes found
[1]