Patchwork D8200: pull: add `--confirm` flag to confirm before writing changes

login
register
mail settings
Submitter phabricator
Date Feb. 29, 2020, 7:32 a.m.
Message ID <differential-rev-PHID-DREV-jx3h5jcjf7cintgqhqrt-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/45412/
State Superseded
Headers show

Comments

phabricator - Feb. 29, 2020, 7:32 a.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This introduces a new flag to pull command `--confirm` and also a config option
  named `pull.confirm` which if used will prompt user describing changes which are
  pulled and asking whether to accept them or not.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/commands.py
  mercurial/configitems.py
  mercurial/exchange.py
  relnotes/next
  tests/test-completion.t
  tests/test-obsolete-distributed.t
  tests/test-obsolete.t
  tests/test-phases-exchange.t
  tests/test-pull-r.t

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - March 3, 2020, 6:26 p.m.
durin42 added inline comments.

INLINE COMMENTS

> exchange.py:1807
>      with wlock, repo.lock(), pullop.trmanager:
> +        if repo.ui.configbool(b"pull", b"confirm"):
> +            add_confirm_callback(repo, pullop)

I think this should probably respect HGPLAIN. Thoughts?

(Note that doing so will require some reworking, because you probably want `HGPLAIN=1 hg pull --confirm` to confirm, and it wouldn't if you just respect HGPLAIN on this line.)

REPOSITORY
  rHG Mercurial

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

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

To: pulkit, #hg-reviewers
Cc: durin42, mercurial-devel
phabricator - March 4, 2020, 7:48 p.m.
marmoute added inline comments.

INLINE COMMENTS

> durin42 wrote in exchange.py:1807
> I think this should probably respect HGPLAIN. Thoughts?
> 
> (Note that doing so will require some reworking, because you probably want `HGPLAIN=1 hg pull --confirm` to confirm, and it wouldn't if you just respect HGPLAIN on this line.)

From IRC discussion, HGPLAIN should result in the config option to be ignored, but the command line flag to be enforced.

REPOSITORY
  rHG Mercurial

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

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

To: pulkit, #hg-reviewers
Cc: marmoute, durin42, mercurial-devel
phabricator - March 4, 2020, 10:07 p.m.
This revision now requires changes to proceed.
durin42 added a comment.
durin42 requested changes to this revision.


  (marking as needs-changes since we've got consensus)

REPOSITORY
  rHG Mercurial

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

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

To: pulkit, #hg-reviewers, durin42
Cc: marmoute, durin42, mercurial-devel

Patch

diff --git a/tests/test-pull-r.t b/tests/test-pull-r.t
--- a/tests/test-pull-r.t
+++ b/tests/test-pull-r.t
@@ -1,3 +1,9 @@ 
+  $ cat <<EOF >> $HGRCPATH
+  > [ui]
+  > interactive = true
+  > EOF
+
+
   $ hg init repo
   $ cd repo
   $ echo foo > foo
@@ -42,12 +48,47 @@ 
   $ hg heads -q --closed
   2:effea6de0384
   1:ed1b79f46b9a
-  $ hg pull
+  $ hg pull --confirm << EOF
+  > n
+  > EOF
   pulling from $TESTTMP/repo2
   searching for changes
   adding changesets
   adding manifests
   adding file changes
+  adding 2 changesets with 1 changes to 1 files
+  new changesets 8c900227dd5d:00cfe9073916
+  accept incoming changes (yn)? n
+  transaction abort!
+  rollback completed
+  abort: user aborted
+  [255]
+  $ hg pull --config pull.confirm=true << EOF
+  > n
+  > EOF
+  pulling from $TESTTMP/repo2
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  adding 2 changesets with 1 changes to 1 files
+  new changesets 8c900227dd5d:00cfe9073916
+  accept incoming changes (yn)? n
+  transaction abort!
+  rollback completed
+  abort: user aborted
+  [255]
+  $ hg pull --confirm << EOF
+  > y
+  > EOF
+  pulling from $TESTTMP/repo2
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  adding 2 changesets with 1 changes to 1 files
+  new changesets 8c900227dd5d:00cfe9073916
+  accept incoming changes (yn)? y
   added 2 changesets with 1 changes to 1 files
   new changesets 8c900227dd5d:00cfe9073916
   (run 'hg update' to get a working copy)
diff --git a/tests/test-phases-exchange.t b/tests/test-phases-exchange.t
--- a/tests/test-phases-exchange.t
+++ b/tests/test-phases-exchange.t
@@ -326,12 +326,18 @@ 
   o  0 public a-A - 054250a37db4
   
   $ cd ../mu
-  $ hg pull ../nu
+  $ hg pull ../nu --confirm --config ui.interactive=True<<EOF
+  > y
+  > EOF
   pulling from ../nu
   searching for changes
   adding changesets
   adding manifests
   adding file changes
+  adding 2 changesets with 2 changes to 2 files
+  new changesets d6bcb4f74035:145e75495359 (2 drafts)
+  4 local changesets will be published
+  accept incoming changes (yn)? y
   added 2 changesets with 2 changes to 2 files
   new changesets d6bcb4f74035:145e75495359 (2 drafts)
   4 local changesets published
diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
--- a/tests/test-obsolete.t
+++ b/tests/test-obsolete.t
@@ -380,12 +380,34 @@ 
 Try to pull markers
 (extinct changeset are excluded but marker are pushed)
 
-  $ hg pull ../tmpb
+  $ hg pull ../tmpb --confirm --config ui.interactive=true <<EOF
+  > n
+  > EOF
   pulling from ../tmpb
   requesting all changes
   adding changesets
   adding manifests
   adding file changes
+  adding 4 changesets with 4 changes to 4 files (+1 heads)
+  5 new obsolescence markers
+  new changesets 1f0dee641bb7:6f9641995072 (1 drafts)
+  accept incoming changes (yn)? n
+  transaction abort!
+  rollback completed
+  abort: user aborted
+  [255]
+  $ hg pull ../tmpb --confirm --config ui.interactive=true <<EOF
+  > y
+  > EOF
+  pulling from ../tmpb
+  requesting all changes
+  adding changesets
+  adding manifests
+  adding file changes
+  adding 4 changesets with 4 changes to 4 files (+1 heads)
+  5 new obsolescence markers
+  new changesets 1f0dee641bb7:6f9641995072 (1 drafts)
+  accept incoming changes (yn)? y
   added 4 changesets with 4 changes to 4 files (+1 heads)
   5 new obsolescence markers
   new changesets 1f0dee641bb7:6f9641995072 (1 drafts)
diff --git a/tests/test-obsolete-distributed.t b/tests/test-obsolete-distributed.t
--- a/tests/test-obsolete-distributed.t
+++ b/tests/test-obsolete-distributed.t
@@ -138,12 +138,37 @@ 
 
   $ hg up 'desc("ROOT")'
   0 files updated, 0 files merged, 1 files removed, 0 files unresolved
-  $ hg pull
+  $ hg pull --confirm --config ui.interactive=True << EOF
+  > n
+  > EOF
   pulling from $TESTTMP/distributed-chain-building/server
   searching for changes
   adding changesets
   adding manifests
   adding file changes
+  adding 1 changesets with 1 changes to 1 files (+1 heads)
+  1 new obsolescence markers
+  obsoleting 1 changesets
+  new changesets 391a2bf12b1b (1 drafts)
+  accept incoming changes (yn)? n
+  transaction abort!
+  rollback completed
+  abort: user aborted
+  [255]
+
+  $ hg pull --confirm --config ui.interactive=True << EOF
+  > y
+  > EOF
+  pulling from $TESTTMP/distributed-chain-building/server
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  adding 1 changesets with 1 changes to 1 files (+1 heads)
+  1 new obsolescence markers
+  obsoleting 1 changesets
+  new changesets 391a2bf12b1b (1 drafts)
+  accept incoming changes (yn)? y
   added 1 changesets with 1 changes to 1 files (+1 heads)
   1 new obsolescence markers
   obsoleted 1 changesets
diff --git a/tests/test-completion.t b/tests/test-completion.t
--- a/tests/test-completion.t
+++ b/tests/test-completion.t
@@ -343,7 +343,7 @@ 
   parents: rev, style, template
   paths: template
   phase: public, draft, secret, force, rev
-  pull: update, force, rev, bookmark, branch, ssh, remotecmd, insecure
+  pull: update, force, confirm, rev, bookmark, branch, ssh, remotecmd, insecure
   push: force, rev, bookmark, branch, new-branch, pushvars, publish, ssh, remotecmd, insecure
   recover: verify
   remove: after, force, subrepos, include, exclude, dry-run
diff --git a/relnotes/next b/relnotes/next
--- a/relnotes/next
+++ b/relnotes/next
@@ -3,6 +3,9 @@ 
  * `hg purge`/`hg clean` can now delete ignored files instead of
    untracked files, with the new -i flag.
 
+ * `hg pull` now has a `--confirm` flag to prompt before applying changes.
+   Config option `pull.confirm` is also added for that.
+
  * `hg log` now defaults to using an '%' symbol for commits involved
     in unresolved merge conflicts. That includes unresolved conflicts
     caused by e.g. `hg update --merge` and `hg graft`. '@' still takes
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -8,6 +8,7 @@ 
 from __future__ import absolute_import
 
 import collections
+import weakref
 
 from .i18n import _
 from .node import (
@@ -1705,6 +1706,23 @@ 
         pullop.rheads = set(pullop.rheads) - pullop.common
 
 
+def add_confirm_callback(repo, pullop):
+    """ adds a finalize callback to transaction which can be used to show stats
+    to user and confirm the pull before committing transaction """
+
+    tr = pullop.trmanager.transaction()
+    scmutil.registersummarycallback(repo, tr, txnname=b'pull', as_validator=True)
+    reporef = weakref.ref(repo.unfiltered())
+
+    def prompt(tr):
+        repo = reporef()
+        cm = _(b'accept incoming changes (yn)?$$ &Yes $$ &No')
+        if repo.ui.promptchoice(cm):
+            raise error.Abort("user aborted")
+
+    tr.addvalidator(b'900-pull-prompt', prompt)
+
+
 def pull(
     repo,
     remote,
@@ -1786,6 +1804,9 @@ 
     if not bookmod.bookmarksinstore(repo):
         wlock = repo.wlock()
     with wlock, repo.lock(), pullop.trmanager:
+        if repo.ui.configbool(b"pull", b"confirm"):
+            add_confirm_callback(repo, pullop)
+
         # Use the modern wire protocol, if available.
         if remote.capable(b'command-changesetdata'):
             exchangev2.pull(pullop)
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -1053,6 +1053,9 @@ 
     b'progress', b'width', default=dynamicdefault,
 )
 coreconfigitem(
+    b'pull', b'confirm', default=False,
+)
+coreconfigitem(
     b'push', b'pushvars.server', default=False,
 )
 coreconfigitem(
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5344,6 +5344,7 @@ 
             None,
             _(b'run even when remote repository is unrelated'),
         ),
+        (b'', b'confirm', None, _(b'confirm pull before applying changes'),),
         (
             b'r',
             b'rev',
@@ -5451,7 +5452,10 @@ 
         wlock = util.nullcontextmanager()
         if opts.get(b'update'):
             wlock = repo.wlock()
-        with wlock:
+        overrides = {}
+        if opts.get(b'confirm'):
+            overrides = {(b'pull', b'confirm'): True}
+        with wlock, ui.configoverride(overrides, b'pull'):
             pullopargs.update(opts.get(b'opargs', {}))
             modheads = exchange.pull(
                 repo,