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

login
register
mail settings
Submitter phabricator
Date Oct. 5, 2019, 8:52 p.m.
Message ID <differential-rev-PHID-DREV-euxbgi75i3lv6oxrq3sj-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/41998/
State Superseded
Headers show

Comments

phabricator - Oct. 5, 2019, 8:52 p.m.
spectral created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6989

AFFECTED FILES
  mercurial/commands.py
  mercurial/configitems.py
  mercurial/help/config.txt
  tests/test-push.t

CHANGE DETAILS




To: spectral, #hg-reviewers
Cc: mercurial-devel
phabricator - Oct. 5, 2019, 10 p.m.
indygreg added a comment.


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

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6989/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6989

To: spectral, #hg-reviewers
Cc: indygreg, mercurial-devel
phabricator - Oct. 6, 2019, 1:50 p.m.
spectral added a comment.


  In D6989#102437 <https://phab.mercurial-scm.org/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`.

INLINE COMMENTS

> commands.py:4648
>                                  'empty set'))
> +    elif ui.configbool('commands', 'push.require-revs'):
> +        raise error.Abort(_('no revisions specified to push'),

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. :)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6989/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6989

To: spectral, #hg-reviewers
Cc: indygreg, mercurial-devel
phabricator - Oct. 7, 2019, 3:18 p.m.
mharbison72 added a comment.


  In D6989#102437 <https://phab.mercurial-scm.org/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.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6989/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6989

To: spectral, #hg-reviewers
Cc: mharbison72, indygreg, mercurial-devel
phabricator - Oct. 8, 2019, 7:40 p.m.
This revision is now accepted and ready to land.
pulkit added a comment.
pulkit accepted this revision.


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

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6989/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6989

To: spectral, #hg-reviewers, pulkit
Cc: pulkit, mharbison72, indygreg, mercurial-devel
phabricator - Oct. 8, 2019, 7:43 p.m.
pulkit added a comment.


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

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6989/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6989

To: spectral, #hg-reviewers, pulkit
Cc: pulkit, mharbison72, indygreg, mercurial-devel

Patch

diff --git a/tests/test-push.t b/tests/test-push.t
--- a/tests/test-push.t
+++ b/tests/test-push.t
@@ -348,3 +348,55 @@ 
   [255]
 
   $ [ ! -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]
diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -442,6 +442,14 @@ 
     Show status of files in the working directory after successful commit.
     (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``
     Confirm before performing action if no filename is passed.
     (default: False)
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -213,6 +213,9 @@ 
     default=False,
     experimental=True,
 )
+coreconfigitem('commands', 'push.require-revs',
+    default=False,
+)
 coreconfigitem('commands', 'resolve.confirm',
     default=False,
 )
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -4645,6 +4645,9 @@ 
         if not revs:
             raise error.Abort(_('default push revset for path evaluates to an '
                                 'empty set'))
+    elif ui.configbool('commands', 'push.require-revs'):
+        raise error.Abort(_('no revisions specified to push'),
+                          hint=_('did you mean "hg push -r ."?'))
 
     repo._subtoppath = dest
     try: