D5940: uncommit: add -f/--force when possibly hiding data (issue5977)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
41 messages Options
123
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add -f/--force when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
navaneeth.suresh created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The behaviour of uncommit may confuse a new user. Although it never
  destroys the data, it can hide instead. I added a `-f/--force` flag
  when the working copy is dirty. The data can be visible on `--hidden`
  flag.
 
  Some cases in `test-uncommit.t` changes output. I'll be working on
  this further based on review.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  hgext/uncommit.py
  mercurial/rewriteutil.py
  tests/test-uncommit.t

CHANGE DETAILS

diff --git a/tests/test-uncommit.t b/tests/test-uncommit.t
--- a/tests/test-uncommit.t
+++ b/tests/test-uncommit.t
@@ -35,6 +35,7 @@
   options ([+] can be repeated):
   
       --keep                allow an empty commit after uncommiting
+   -f --force               allow uncommit with outstanding changes
    -I --include PATTERN [+] include names matching the given patterns
    -X --exclude PATTERN [+] exclude names matching the given patterns
   
@@ -158,6 +159,9 @@
   abort: uncommitted changes
   [255]
   $ hg uncommit files
+  abort: uncommitted changes
+  (use -f to force)
+  [255]
   $ cat files
   abcde
   foo
@@ -170,14 +174,48 @@
   abort: uncommitted changes
   [255]
   $ hg uncommit --config experimental.uncommitondirtywdir=True
+  abort: uncommitted changes
+  (use -f to force)
+  [255]
   $ hg commit -m "files abcde + foo"
 
 Uncommit in the middle of a stack, does not move bookmark
 
   $ hg checkout '.^^^'
-  1 files updated, 0 files merged, 2 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
   (leaving bookmark foo)
   $ hg log -r . -p -T '{rev}:{node} {desc}'
+  3:6db330d65db434145c0b59d291853e9a84719b24 added file-abcddiff -r abf2df566fc1 -r 6db330d65db4 file-abcd
+  --- /dev/null Thu Jan 01 00:00:00 1970 +0000
+  +++ b/file-abcd Thu Jan 01 00:00:00 1970 +0000
+  @@ -0,0 +1,1 @@
+  +abcd
+  diff -r abf2df566fc1 -r 6db330d65db4 files
+  --- a/files Thu Jan 01 00:00:00 1970 +0000
+  +++ b/files Thu Jan 01 00:00:00 1970 +0000
+  @@ -1,1 +1,1 @@
+  -abc
+  +abcd
+  
+  $ hg bookmark
+     foo                       9:ad3773de7293
+  $ hg uncommit
+  3 new orphan changesets
+  $ hg status
+  M files
+  A file-abcd
+  $ hg heads -T '{rev}:{node} {desc}'
+  9:ad3773de72930be60f1ebb39fe89115b81630a8a files abcde + foo (no-eol)
+  $ hg bookmark
+     foo                       9:ad3773de7293
+  $ hg commit -m 'new abc'
+  created new head
+
+Partial uncommit in the middle, does not move bookmark
+
+  $ hg checkout '.^'
+  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ hg log -r . -p -T '{rev}:{node} {desc}'
   2:abf2df566fc193b3ac34d946e63c1583e4d4732b added file-abcdiff -r 69a232e754b0 -r abf2df566fc1 file-abc
   --- /dev/null Thu Jan 01 00:00:00 1970 +0000
   +++ b/file-abc Thu Jan 01 00:00:00 1970 +0000
@@ -191,119 +229,85 @@
   +abc
   
   $ hg bookmark
-     foo                       10:48e5bd7cd583
-  $ hg uncommit
-  3 new orphan changesets
+     foo                       9:ad3773de7293
+  $ hg uncommit file-ab
+  nothing to uncommit
+  [1]
   $ hg status
-  M files
-  A file-abc
-  $ hg heads -T '{rev}:{node} {desc}'
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo (no-eol)
-  $ hg bookmark
-     foo                       10:48e5bd7cd583
-  $ hg commit -m 'new abc'
-  created new head
-
-Partial uncommit in the middle, does not move bookmark
-
-  $ hg checkout '.^'
-  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
-  $ hg log -r . -p -T '{rev}:{node} {desc}'
-  1:69a232e754b08d568c4899475faf2eb44b857802 added file-abdiff -r 3004d2d9b508 -r 69a232e754b0 file-ab
-  --- /dev/null Thu Jan 01 00:00:00 1970 +0000
-  +++ b/file-ab Thu Jan 01 00:00:00 1970 +0000
-  @@ -0,0 +1,1 @@
-  +ab
-  diff -r 3004d2d9b508 -r 69a232e754b0 files
-  --- a/files Thu Jan 01 00:00:00 1970 +0000
-  +++ b/files Thu Jan 01 00:00:00 1970 +0000
-  @@ -1,1 +1,1 @@
-  -a
-  +ab
-  
-  $ hg bookmark
-     foo                       10:48e5bd7cd583
-  $ hg uncommit file-ab
-  1 new orphan changesets
-  $ hg status
-  A file-ab
 
   $ hg heads -T '{rev}:{node} {desc}\n'
-  12:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
-  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  10:25798b2f714d0937797be0f9fde55aaf5472c052 new abc
+  9:ad3773de72930be60f1ebb39fe89115b81630a8a files abcde + foo
 
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:ad3773de7293
   $ hg commit -m 'update ab'
+  nothing changed
+  [1]
   $ hg status
   $ hg heads -T '{rev}:{node} {desc}\n'
-  13:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
-  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  10:25798b2f714d0937797be0f9fde55aaf5472c052 new abc
+  9:ad3773de72930be60f1ebb39fe89115b81630a8a files abcde + foo
 
   $ hg log -G -T '{rev}:{node} {desc}' --hidden
-  @  13:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
+  o  10:25798b2f714d0937797be0f9fde55aaf5472c052 new abc
   |
-  o  12:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
-  |
-  | *  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
+  | *  9:ad3773de72930be60f1ebb39fe89115b81630a8a files abcde + foo
+  | |
+  | *  8:84beeba0ac30e19521c036e4d2dd3a5fa02586ff files abcde + foo
+  | |
+  | | x  7:0977fa602c2fd7d8427ed4e7ee15ea13b84c9173 update files for abcde
+  | |/
+  | *  6:3727deee06f72f5ffa8db792ee299cf39e3e190b new change abcde
   | |
-  | | *  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
-  | | |
-  | | | x  9:8a6b58c173ca6a2e3745d8bd86698718d664bc6c files abcde + foo
-  | | |/
-  | | | x  8:39ad452c7f684a55d161c574340c5766c4569278 update files for abcde
-  | | |/
-  | | | x  7:0977fa602c2fd7d8427ed4e7ee15ea13b84c9173 update files for abcde
-  | | |/
-  | | *  6:3727deee06f72f5ffa8db792ee299cf39e3e190b new change abcde
-  | | |
-  | | | x  5:0c07a3ccda771b25f1cb1edbd02e683723344ef1 new change abcde
-  | | |/
-  | | | x  4:6c4fd43ed714e7fcd8adbaa7b16c953c2e985b60 added file-abcde
-  | | |/
-  | | *  3:6db330d65db434145c0b59d291853e9a84719b24 added file-abcd
-  | | |
-  | | x  2:abf2df566fc193b3ac34d946e63c1583e4d4732b added file-abc
+  | | x  5:0c07a3ccda771b25f1cb1edbd02e683723344ef1 new change abcde
+  | |/
+  | | x  4:6c4fd43ed714e7fcd8adbaa7b16c953c2e985b60 added file-abcde
   | |/
-  | x  1:69a232e754b08d568c4899475faf2eb44b857802 added file-ab
+  | x  3:6db330d65db434145c0b59d291853e9a84719b24 added file-abcd
   |/
+  @  2:abf2df566fc193b3ac34d946e63c1583e4d4732b added file-abc
+  |
+  o  1:69a232e754b08d568c4899475faf2eb44b857802 added file-ab
+  |
   o  0:3004d2d9b50883c1538fc754a3aeb55f1b4084f6 added file-a
   
 Uncommit with draft parent
 
   $ hg uncommit
+  1 new orphan changesets
   $ hg phase -r .
-  12: draft
+  1: draft
   $ hg commit -m 'update ab again'
+  created new head
 
 Phase is preserved
 
   $ hg uncommit --keep --config phases.new-commit=secret
   $ hg phase -r .
-  15: draft
+  12: draft
   $ hg commit --amend -m 'update ab again'
 
 Uncommit with public parent
 
   $ hg phase -p "::.^"
   $ hg uncommit
   $ hg phase -r .
-  12: public
+  1: public
 
 Partial uncommit with public parent
 
   $ echo xyz > xyz
   $ hg add xyz
   $ hg commit -m "update ab and add xyz"
+  created new head
   $ hg uncommit xyz
   $ hg status
   A xyz
   $ hg phase -r .
-  18: draft
+  15: draft
   $ hg phase -r ".^"
-  12: public
+  1: public
 
 Uncommit leaving an empty changeset
 
@@ -379,7 +383,7 @@
   $ hg commit -m 'merge a and b'
 
   $ hg uncommit
-  abort: cannot uncommit merge changeset
+  abort: cannot uncommit merge changesets
   [255]
 
   $ hg status
@@ -398,3 +402,35 @@
   |/
   o  0:ea4e33293d4d274a2ba73150733c2612231f398c a 1
   
+  $ cd ..
+
+Uncommit should require -f/--force when possibly hiding data
+
+  $ hg init issue5977
+  $ cd issue5977
+  $ echo 'super critical info!' > a
+  $ hg ci -Am 'add a'
+  adding a
+  $ echo 'foo' > a
+  $ hg unc a
+  abort: uncommitted changes
+  (use -f to force)
+  [255]
+  $ cat a
+  foo
+  $ hg log
+  changeset:   0:e37b99831f6e
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     add a
+  
+  $ hg unc -f a
+  $ hg log
+  changeset:   1:656ba143d384
+  tag:         tip
+  parent:      -1:000000000000
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     add a
+  
diff --git a/mercurial/rewriteutil.py b/mercurial/rewriteutil.py
--- a/mercurial/rewriteutil.py
+++ b/mercurial/rewriteutil.py
@@ -16,9 +16,10 @@
     revset,
 )
 
-def precheck(repo, revs, action='rewrite'):
+def precheck(repo, revs, action='rewrite', merge=False):
     """check if revs can be rewritten
     action is used to control the error message.
+    action is used to true the reject merges.
 
     Make sure this function is called after taking the lock.
     """
@@ -39,6 +40,9 @@
     newunstable = disallowednewunstable(repo, revs)
     if newunstable:
         raise error.Abort(_("cannot %s changeset with children") % action)
+    if merge and any(repo[r].p2() for r in revs):
+        msg = _("cannot %s merge changesets") % (action,)
+        raise error.Abort(msg)
 
 def disallowednewunstable(repo, revs):
     """Checks whether editing the revs will create new unstable changesets and
diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -137,6 +137,7 @@
 
 @command('uncommit',
     [('', 'keep', False, _('allow an empty commit after uncommiting')),
+     ('f', 'force', False, _('allow uncommit with outstanding changes')),
     ] + commands.walkopts,
     _('[OPTION]... [FILE]...'),
     helpcategory=command.CATEGORY_CHANGE_MANAGEMENT)
@@ -159,12 +160,16 @@
                                                'uncommitondirtywdir'):
             cmdutil.bailifchanged(repo)
         old = repo['.']
-        rewriteutil.precheck(repo, [old.rev()], 'uncommit')
-        if len(old.parents()) > 1:
-            raise error.Abort(_("cannot uncommit merge changeset"))
+        rewriteutil.precheck(repo, [old.rev()], 'uncommit', merge=True)
+
+        match = scmutil.match(old, pats, opts)
+        # explicitly check for a merge, so it cannot be overridden
 
+        if old.p2():
+            raise error.Abort(_("outstanding uncommitted merge"))
+        if not opts.get('force'):
+            cmdutil.bailifchanged(repo, hint=_('use -f to force'))
         with repo.transaction('uncommit'):
-            match = scmutil.match(old, pats, opts)
             keepcommit = opts.get('keep') or pats
             newid = _commitfiltered(repo, old, match, keepcommit)
             if newid is None:



To: navaneeth.suresh, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add -f/--force when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
pulkit added a comment.


  Hi, it will be good if you specify that the patch is authored by someone else, mention their name and also provide the link to the original PR of this patch.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add -f/--force when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
navaneeth.suresh added a comment.


  In https://phab.mercurial-scm.org/D5940#87026, @pulkit wrote:
 
  > Hi, it will be good if you specify that the patch is authored by someone else, mention their name and also provide the link to the original PR of this patch.
 
 
  @pulkit the link of WIP patch to evolve extension's uncommit is given in the bugzilla page itself.  Link to the patch is https://bitbucket.org/octobus/evolve-devel/pull-requests/8/bug-5977-uncommit-dirtiness/diff which is authored by Dan Villiom Podlaski Christiansen.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add -f/--force when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
pulkit added a comment.


  In https://phab.mercurial-scm.org/D5940#87087, @navaneeth.suresh wrote:
 
  > In https://phab.mercurial-scm.org/D5940#87026, @pulkit wrote:
  >
  > > Hi, it will be good if you specify that the patch is authored by someone else, mention their name and also provide the link to the original PR of this patch.
  >
  >
  > @pulkit the link of WIP patch to evolve extension's uncommit is given in the bugzilla page itself.  Link to the patch is https://bitbucket.org/octobus/evolve-devel/pull-requests/8/bug-5977-uncommit-dirtiness/diff which is authored by Dan Villiom Podlaski Christiansen.
 
 
  Yep, that's how I figured it out. ;)
 
  It's always good to include that information in commit message. Commit messages should try to explain all the things related to patch and should contain all the related links.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add -f/--force when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
navaneeth.suresh updated this revision to Diff 14096.
navaneeth.suresh edited the summary of this revision.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5940?vs=14036&id=14096

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

AFFECTED FILES
  hgext/uncommit.py
  mercurial/rewriteutil.py
  tests/test-uncommit.t

CHANGE DETAILS

diff --git a/tests/test-uncommit.t b/tests/test-uncommit.t
--- a/tests/test-uncommit.t
+++ b/tests/test-uncommit.t
@@ -35,6 +35,7 @@
   options ([+] can be repeated):
   
       --keep                allow an empty commit after uncommiting
+   -f --force               allow uncommit with outstanding changes
    -I --include PATTERN [+] include names matching the given patterns
    -X --exclude PATTERN [+] exclude names matching the given patterns
   
@@ -158,6 +159,9 @@
   abort: uncommitted changes
   [255]
   $ hg uncommit files
+  abort: uncommitted changes
+  (use -f to force)
+  [255]
   $ cat files
   abcde
   foo
@@ -170,14 +174,48 @@
   abort: uncommitted changes
   [255]
   $ hg uncommit --config experimental.uncommitondirtywdir=True
+  abort: uncommitted changes
+  (use -f to force)
+  [255]
   $ hg commit -m "files abcde + foo"
 
 Uncommit in the middle of a stack, does not move bookmark
 
   $ hg checkout '.^^^'
-  1 files updated, 0 files merged, 2 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
   (leaving bookmark foo)
   $ hg log -r . -p -T '{rev}:{node} {desc}'
+  3:6db330d65db434145c0b59d291853e9a84719b24 added file-abcddiff -r abf2df566fc1 -r 6db330d65db4 file-abcd
+  --- /dev/null Thu Jan 01 00:00:00 1970 +0000
+  +++ b/file-abcd Thu Jan 01 00:00:00 1970 +0000
+  @@ -0,0 +1,1 @@
+  +abcd
+  diff -r abf2df566fc1 -r 6db330d65db4 files
+  --- a/files Thu Jan 01 00:00:00 1970 +0000
+  +++ b/files Thu Jan 01 00:00:00 1970 +0000
+  @@ -1,1 +1,1 @@
+  -abc
+  +abcd
+  
+  $ hg bookmark
+     foo                       9:ad3773de7293
+  $ hg uncommit
+  3 new orphan changesets
+  $ hg status
+  M files
+  A file-abcd
+  $ hg heads -T '{rev}:{node} {desc}'
+  9:ad3773de72930be60f1ebb39fe89115b81630a8a files abcde + foo (no-eol)
+  $ hg bookmark
+     foo                       9:ad3773de7293
+  $ hg commit -m 'new abc'
+  created new head
+
+Partial uncommit in the middle, does not move bookmark
+
+  $ hg checkout '.^'
+  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ hg log -r . -p -T '{rev}:{node} {desc}'
   2:abf2df566fc193b3ac34d946e63c1583e4d4732b added file-abcdiff -r 69a232e754b0 -r abf2df566fc1 file-abc
   --- /dev/null Thu Jan 01 00:00:00 1970 +0000
   +++ b/file-abc Thu Jan 01 00:00:00 1970 +0000
@@ -191,119 +229,85 @@
   +abc
   
   $ hg bookmark
-     foo                       10:48e5bd7cd583
-  $ hg uncommit
-  3 new orphan changesets
+     foo                       9:ad3773de7293
+  $ hg uncommit file-ab
+  nothing to uncommit
+  [1]
   $ hg status
-  M files
-  A file-abc
-  $ hg heads -T '{rev}:{node} {desc}'
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo (no-eol)
-  $ hg bookmark
-     foo                       10:48e5bd7cd583
-  $ hg commit -m 'new abc'
-  created new head
-
-Partial uncommit in the middle, does not move bookmark
-
-  $ hg checkout '.^'
-  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
-  $ hg log -r . -p -T '{rev}:{node} {desc}'
-  1:69a232e754b08d568c4899475faf2eb44b857802 added file-abdiff -r 3004d2d9b508 -r 69a232e754b0 file-ab
-  --- /dev/null Thu Jan 01 00:00:00 1970 +0000
-  +++ b/file-ab Thu Jan 01 00:00:00 1970 +0000
-  @@ -0,0 +1,1 @@
-  +ab
-  diff -r 3004d2d9b508 -r 69a232e754b0 files
-  --- a/files Thu Jan 01 00:00:00 1970 +0000
-  +++ b/files Thu Jan 01 00:00:00 1970 +0000
-  @@ -1,1 +1,1 @@
-  -a
-  +ab
-  
-  $ hg bookmark
-     foo                       10:48e5bd7cd583
-  $ hg uncommit file-ab
-  1 new orphan changesets
-  $ hg status
-  A file-ab
 
   $ hg heads -T '{rev}:{node} {desc}\n'
-  12:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
-  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  10:25798b2f714d0937797be0f9fde55aaf5472c052 new abc
+  9:ad3773de72930be60f1ebb39fe89115b81630a8a files abcde + foo
 
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:ad3773de7293
   $ hg commit -m 'update ab'
+  nothing changed
+  [1]
   $ hg status
   $ hg heads -T '{rev}:{node} {desc}\n'
-  13:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
-  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  10:25798b2f714d0937797be0f9fde55aaf5472c052 new abc
+  9:ad3773de72930be60f1ebb39fe89115b81630a8a files abcde + foo
 
   $ hg log -G -T '{rev}:{node} {desc}' --hidden
-  @  13:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
+  o  10:25798b2f714d0937797be0f9fde55aaf5472c052 new abc
   |
-  o  12:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
-  |
-  | *  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
+  | *  9:ad3773de72930be60f1ebb39fe89115b81630a8a files abcde + foo
+  | |
+  | *  8:84beeba0ac30e19521c036e4d2dd3a5fa02586ff files abcde + foo
+  | |
+  | | x  7:0977fa602c2fd7d8427ed4e7ee15ea13b84c9173 update files for abcde
+  | |/
+  | *  6:3727deee06f72f5ffa8db792ee299cf39e3e190b new change abcde
   | |
-  | | *  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
-  | | |
-  | | | x  9:8a6b58c173ca6a2e3745d8bd86698718d664bc6c files abcde + foo
-  | | |/
-  | | | x  8:39ad452c7f684a55d161c574340c5766c4569278 update files for abcde
-  | | |/
-  | | | x  7:0977fa602c2fd7d8427ed4e7ee15ea13b84c9173 update files for abcde
-  | | |/
-  | | *  6:3727deee06f72f5ffa8db792ee299cf39e3e190b new change abcde
-  | | |
-  | | | x  5:0c07a3ccda771b25f1cb1edbd02e683723344ef1 new change abcde
-  | | |/
-  | | | x  4:6c4fd43ed714e7fcd8adbaa7b16c953c2e985b60 added file-abcde
-  | | |/
-  | | *  3:6db330d65db434145c0b59d291853e9a84719b24 added file-abcd
-  | | |
-  | | x  2:abf2df566fc193b3ac34d946e63c1583e4d4732b added file-abc
+  | | x  5:0c07a3ccda771b25f1cb1edbd02e683723344ef1 new change abcde
+  | |/
+  | | x  4:6c4fd43ed714e7fcd8adbaa7b16c953c2e985b60 added file-abcde
   | |/
-  | x  1:69a232e754b08d568c4899475faf2eb44b857802 added file-ab
+  | x  3:6db330d65db434145c0b59d291853e9a84719b24 added file-abcd
   |/
+  @  2:abf2df566fc193b3ac34d946e63c1583e4d4732b added file-abc
+  |
+  o  1:69a232e754b08d568c4899475faf2eb44b857802 added file-ab
+  |
   o  0:3004d2d9b50883c1538fc754a3aeb55f1b4084f6 added file-a
   
 Uncommit with draft parent
 
   $ hg uncommit
+  1 new orphan changesets
   $ hg phase -r .
-  12: draft
+  1: draft
   $ hg commit -m 'update ab again'
+  created new head
 
 Phase is preserved
 
   $ hg uncommit --keep --config phases.new-commit=secret
   $ hg phase -r .
-  15: draft
+  12: draft
   $ hg commit --amend -m 'update ab again'
 
 Uncommit with public parent
 
   $ hg phase -p "::.^"
   $ hg uncommit
   $ hg phase -r .
-  12: public
+  1: public
 
 Partial uncommit with public parent
 
   $ echo xyz > xyz
   $ hg add xyz
   $ hg commit -m "update ab and add xyz"
+  created new head
   $ hg uncommit xyz
   $ hg status
   A xyz
   $ hg phase -r .
-  18: draft
+  15: draft
   $ hg phase -r ".^"
-  12: public
+  1: public
 
 Uncommit leaving an empty changeset
 
@@ -379,7 +383,7 @@
   $ hg commit -m 'merge a and b'
 
   $ hg uncommit
-  abort: cannot uncommit merge changeset
+  abort: cannot uncommit merge changesets
   [255]
 
   $ hg status
@@ -398,3 +402,35 @@
   |/
   o  0:ea4e33293d4d274a2ba73150733c2612231f398c a 1
   
+  $ cd ..
+
+Uncommit should require -f/--force when possibly hiding data
+
+  $ hg init issue5977
+  $ cd issue5977
+  $ echo 'super critical info!' > a
+  $ hg ci -Am 'add a'
+  adding a
+  $ echo 'foo' > a
+  $ hg unc a
+  abort: uncommitted changes
+  (use -f to force)
+  [255]
+  $ cat a
+  foo
+  $ hg log
+  changeset:   0:e37b99831f6e
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     add a
+  
+  $ hg unc -f a
+  $ hg log
+  changeset:   1:656ba143d384
+  tag:         tip
+  parent:      -1:000000000000
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     add a
+  
diff --git a/mercurial/rewriteutil.py b/mercurial/rewriteutil.py
--- a/mercurial/rewriteutil.py
+++ b/mercurial/rewriteutil.py
@@ -16,9 +16,10 @@
     revset,
 )
 
-def precheck(repo, revs, action='rewrite'):
+def precheck(repo, revs, action='rewrite', merge=False):
     """check if revs can be rewritten
     action is used to control the error message.
+    action is used to true the reject merges.
 
     Make sure this function is called after taking the lock.
     """
@@ -39,6 +40,9 @@
     newunstable = disallowednewunstable(repo, revs)
     if newunstable:
         raise error.Abort(_("cannot %s changeset with children") % action)
+    if merge and any(repo[r].p2() for r in revs):
+        msg = _("cannot %s merge changesets") % (action,)
+        raise error.Abort(msg)
 
 def disallowednewunstable(repo, revs):
     """Checks whether editing the revs will create new unstable changesets and
diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -137,6 +137,7 @@
 
 @command('uncommit',
     [('', 'keep', False, _('allow an empty commit after uncommiting')),
+     ('f', 'force', False, _('allow uncommit with outstanding changes')),
     ] + commands.walkopts,
     _('[OPTION]... [FILE]...'),
     helpcategory=command.CATEGORY_CHANGE_MANAGEMENT)
@@ -159,12 +160,16 @@
                                                'uncommitondirtywdir'):
             cmdutil.bailifchanged(repo)
         old = repo['.']
-        rewriteutil.precheck(repo, [old.rev()], 'uncommit')
-        if len(old.parents()) > 1:
-            raise error.Abort(_("cannot uncommit merge changeset"))
+        rewriteutil.precheck(repo, [old.rev()], 'uncommit', merge=True)
+
+        match = scmutil.match(old, pats, opts)
+        # explicitly check for a merge, so it cannot be overridden
 
+        if old.p2():
+            raise error.Abort(_("outstanding uncommitted merge"))
+        if not opts.get('force'):
+            cmdutil.bailifchanged(repo, hint=_('use -f to force'))
         with repo.transaction('uncommit'):
-            match = scmutil.match(old, pats, opts)
             keepcommit = opts.get('keep') or pats
             newid = _commitfiltered(repo, old, match, keepcommit)
             if newid is None:



To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add -f/--force when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
martinvonz added inline comments.

INLINE COMMENTS

> test-uncommit.t:176-179
>    $ hg uncommit --config experimental.uncommitondirtywdir=True
> +  abort: uncommitted changes
> +  (use -f to force)
> +  [255]

Why did this change?

I don't understand what the new flag is for since we already how the config option. They seem to me to do the same thing.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add -f/--force when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
navaneeth.suresh added inline comments.

INLINE COMMENTS

> martinvonz wrote in test-uncommit.t:176-179
> Why did this change?
>
> I don't understand what the new flag is for since we already how the config option. They seem to me to do the same thing.

I'm also surprised at the change in that output. Do you recommend removing that config option if that doesn't do anything new?

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add -f/--force when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
martinvonz added inline comments.

INLINE COMMENTS

> uncommit.py:140
>      [('', 'keep', False, _('allow an empty commit after uncommiting')),
> +     ('f', 'force', False, _('allow uncommit with outstanding changes')),
>      ] + commands.walkopts,

Could you rename --force to e.g. --allow-dirty-working-copy so it's clearer what's being allowed?

> navaneeth.suresh wrote in test-uncommit.t:176-179
> I'm also surprised at the change in that output. Do you recommend removing that config option if that doesn't do anything new?

I like that config option and I don't want to lose it. I was even hoping it would be on by default some day.

IIUC, the new behavior is to require --force if there are uncommitted changes, whether you're uncommitting one file or all. I was actually surprised that it used to be allowed to uncommit specific files on a dirty working copy. I like the new consistency more.

I think the --force option should be equivalent to the existing config. How about we just add `and not opts.get('force')` to the existing condition on line 159? I'd also prefer to drop the `not pats` from that condition, but that can (and should) be in a separate patch.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add --allowdirtywcopy when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
navaneeth.suresh updated this revision to Diff 14100.
navaneeth.suresh edited the summary of this revision.
navaneeth.suresh retitled this revision from "uncommit: add -f/--force when possibly hiding data (issue5977)" to "uncommit: add --allowdirtywcopy when possibly hiding data (issue5977)".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5940?vs=14096&id=14100

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

AFFECTED FILES
  hgext/uncommit.py
  mercurial/rewriteutil.py
  tests/test-uncommit.t

CHANGE DETAILS

diff --git a/tests/test-uncommit.t b/tests/test-uncommit.t
--- a/tests/test-uncommit.t
+++ b/tests/test-uncommit.t
@@ -35,6 +35,7 @@
   options ([+] can be repeated):
   
       --keep                allow an empty commit after uncommiting
+      --allowdirtywcopy     allow uncommit with outstanding changes
    -I --include PATTERN [+] include names matching the given patterns
    -X --exclude PATTERN [+] exclude names matching the given patterns
   
@@ -156,8 +157,12 @@
   M files
   $ hg uncommit
   abort: uncommitted changes
+  (use --allowdirtywcopy to uncommit)
   [255]
   $ hg uncommit files
+  abort: uncommitted changes
+  (use --allowdirtywcopy to uncommit)
+  [255]
   $ cat files
   abcde
   foo
@@ -168,6 +173,7 @@
   $ echo "bar" >> files
   $ hg uncommit
   abort: uncommitted changes
+  (use --allowdirtywcopy to uncommit)
   [255]
   $ hg uncommit --config experimental.uncommitondirtywdir=True
   $ hg commit -m "files abcde + foo"
@@ -191,16 +197,16 @@
   +abc
   
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:48e5bd7cd583
   $ hg uncommit
   3 new orphan changesets
   $ hg status
   M files
   A file-abc
   $ hg heads -T '{rev}:{node} {desc}'
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo (no-eol)
+  9:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo (no-eol)
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:48e5bd7cd583
   $ hg commit -m 'new abc'
   created new head
 
@@ -222,38 +228,36 @@
   +ab
   
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:48e5bd7cd583
   $ hg uncommit file-ab
   1 new orphan changesets
   $ hg status
   A file-ab
 
   $ hg heads -T '{rev}:{node} {desc}\n'
-  12:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
-  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  11:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
+  10:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
+  9:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
 
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:48e5bd7cd583
   $ hg commit -m 'update ab'
   $ hg status
   $ hg heads -T '{rev}:{node} {desc}\n'
-  13:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
-  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  12:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
+  10:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
+  9:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
 
   $ hg log -G -T '{rev}:{node} {desc}' --hidden
-  @  13:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
+  @  12:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
   |
-  o  12:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
+  o  11:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
   |
-  | *  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
+  | *  10:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
   | |
-  | | *  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  | | *  9:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
   | | |
-  | | | x  9:8a6b58c173ca6a2e3745d8bd86698718d664bc6c files abcde + foo
-  | | |/
-  | | | x  8:39ad452c7f684a55d161c574340c5766c4569278 update files for abcde
+  | | | x  8:84beeba0ac30e19521c036e4d2dd3a5fa02586ff files abcde + foo
   | | |/
   | | | x  7:0977fa602c2fd7d8427ed4e7ee15ea13b84c9173 update files for abcde
   | | |/
@@ -275,22 +279,22 @@
 
   $ hg uncommit
   $ hg phase -r .
-  12: draft
+  11: draft
   $ hg commit -m 'update ab again'
 
 Phase is preserved
 
   $ hg uncommit --keep --config phases.new-commit=secret
   $ hg phase -r .
-  15: draft
+  14: draft
   $ hg commit --amend -m 'update ab again'
 
 Uncommit with public parent
 
   $ hg phase -p "::.^"
   $ hg uncommit
   $ hg phase -r .
-  12: public
+  11: public
 
 Partial uncommit with public parent
 
@@ -301,9 +305,9 @@
   $ hg status
   A xyz
   $ hg phase -r .
-  18: draft
+  17: draft
   $ hg phase -r ".^"
-  12: public
+  11: public
 
 Uncommit leaving an empty changeset
 
@@ -368,6 +372,7 @@
 
   $ hg uncommit
   abort: outstanding uncommitted merge
+  (use --allowdirtywcopy to uncommit)
   [255]
 
   $ hg uncommit --config experimental.uncommitondirtywdir=True
@@ -379,7 +384,7 @@
   $ hg commit -m 'merge a and b'
 
   $ hg uncommit
-  abort: cannot uncommit merge changeset
+  abort: cannot uncommit merge changesets
   [255]
 
   $ hg status
@@ -398,3 +403,35 @@
   |/
   o  0:ea4e33293d4d274a2ba73150733c2612231f398c a 1
   
+  $ cd ..
+
+Uncommit should require -f/--force when possibly hiding data
+
+  $ hg init issue5977
+  $ cd issue5977
+  $ echo 'super critical info!' > a
+  $ hg ci -Am 'add a'
+  adding a
+  $ echo 'foo' > a
+  $ hg unc a
+  abort: uncommitted changes
+  (use --allowdirtywcopy to uncommit)
+  [255]
+  $ cat a
+  foo
+  $ hg log
+  changeset:   0:e37b99831f6e
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     add a
+  
+  $ hg unc --allowdirtywcopy a
+  $ hg log
+  changeset:   1:656ba143d384
+  tag:         tip
+  parent:      -1:000000000000
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     add a
+  
diff --git a/mercurial/rewriteutil.py b/mercurial/rewriteutil.py
--- a/mercurial/rewriteutil.py
+++ b/mercurial/rewriteutil.py
@@ -16,9 +16,10 @@
     revset,
 )
 
-def precheck(repo, revs, action='rewrite'):
+def precheck(repo, revs, action='rewrite', merge=False):
     """check if revs can be rewritten
     action is used to control the error message.
+    action is used to true the reject merges.
 
     Make sure this function is called after taking the lock.
     """
@@ -39,6 +40,9 @@
     newunstable = disallowednewunstable(repo, revs)
     if newunstable:
         raise error.Abort(_("cannot %s changeset with children") % action)
+    if merge and any(repo[r].p2() for r in revs):
+        msg = _("cannot %s merge changesets") % (action,)
+        raise error.Abort(msg)
 
 def disallowednewunstable(repo, revs):
     """Checks whether editing the revs will create new unstable changesets and
diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -137,6 +137,8 @@
 
 @command('uncommit',
     [('', 'keep', False, _('allow an empty commit after uncommiting')),
+     ('', 'allowdirtywcopy', False, _('allow uncommit with '
+                                               'outstanding changes')),
     ] + commands.walkopts,
     _('[OPTION]... [FILE]...'),
     helpcategory=command.CATEGORY_CHANGE_MANAGEMENT)
@@ -155,16 +157,19 @@
 
     with repo.wlock(), repo.lock():
 
-        if not pats and not repo.ui.configbool('experimental',
-                                               'uncommitondirtywdir'):
-            cmdutil.bailifchanged(repo)
+        if (not repo.ui.configbool('experimental', 'uncommitondirtywdir')
+            and not opts.get('allowdirtywcopy')):
+            cmdutil.bailifchanged(repo, hint=_('use '
+                                  '--allowdirtywcopy to uncommit'))
         old = repo['.']
-        rewriteutil.precheck(repo, [old.rev()], 'uncommit')
-        if len(old.parents()) > 1:
-            raise error.Abort(_("cannot uncommit merge changeset"))
+        rewriteutil.precheck(repo, [old.rev()], 'uncommit', merge=True)
 
+        match = scmutil.match(old, pats, opts)
+        # explicitly check for a merge, so it cannot be overridden
+
+        if old.p2():
+            raise error.Abort(_("outstanding uncommitted merge"))
         with repo.transaction('uncommit'):
-            match = scmutil.match(old, pats, opts)
             keepcommit = opts.get('keep') or pats
             newid = _commitfiltered(repo, old, match, keepcommit)
             if newid is None:



To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add --allowdirtywcopy when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
pulkit added a comment.


  Your current patch is doing more than one thing. Let's split this up and make one patch do one thing.

INLINE COMMENTS

> uncommit.py:168
>  
> +        if old.p2():
> +            raise error.Abort(_("outstanding uncommitted merge"))

this merits a different patch and tests of it's own.

> uncommit.py:170
> +            raise error.Abort(_("outstanding uncommitted merge"))
> +        if not opts.get('force'):
> +            cmdutil.bailifchanged(repo, hint=_('use -f to force'))

this should also consider checking the config `experimental.uncommitondirtywdir`.

> rewriteutil.py:43
>          raise error.Abort(_("cannot %s changeset with children") % action)
> +    if merge and any(repo[r].p2() for r in revs):
> +        msg = _("cannot %s merge changesets") % (action,)

I am not sure why this is a part of patch. Can you explain?

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add --allowdirtywcopy when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
pulkit added inline comments.

INLINE COMMENTS

> martinvonz wrote in uncommit.py:140
> Could you rename --force to e.g. --allow-dirty-working-copy so it's clearer what's being allowed?

we have a config for that already, let's prevent to introduce flags for config options and vice versa here.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add --allowdirtywcopy when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
martinvonz added inline comments.

INLINE COMMENTS

> pulkit wrote in uncommit.py:140
> we have a config for that already, let's prevent to introduce flags for config options and vice versa here.

Doesn't that mean the whole patch is unnecessary? It pretty much just provides an easier way of saying `--config experimental.uncommitondirtywdir=yes`, doesn't it?

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add --allowdirtywcopy when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
navaneeth.suresh added inline comments.

INLINE COMMENTS

> martinvonz wrote in uncommit.py:140
> Doesn't that mean the whole patch is unnecessary? It pretty much just provides an easier way of saying `--config experimental.uncommitondirtywdir=yes`, doesn't it?

I tried with `--allow-dirty-working-copy` but, it failed to work. I'm also not convinced by the patch now. How do you want me to proceed?

> pulkit wrote in uncommit.py:170
> this should also consider checking the config `experimental.uncommitondirtywdir`.

will correct that.

> pulkit wrote in rewriteutil.py:43
> I am not sure why this is a part of patch. Can you explain?

I imported those from the original PR to the evolve extension version of uncommit. IIUC, this will remove the check in `uncommit.py` and let it do here.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: add --allowdirtywcopy when possibly hiding data (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
pulkit added inline comments.

INLINE COMMENTS

> martinvonz wrote in uncommit.py:140
> Doesn't that mean the whole patch is unnecessary? It pretty much just provides an easier way of saying `--config experimental.uncommitondirtywdir=yes`, doesn't it?

Nice point. I had to revisit the bug to understand more now. IMO the bug says as following:

`uncommit` should require a flag or config when it is trying to do the following:

1. uncommit some files which are dirty in wdir, and
2. the files are passed as `hg uncommit PATH`

(refer Yuya's comment: https://bz.mercurial-scm.org/show_bug.cgi?id=5977#c1)

However the bug was filed for evolve uncommit, it's applicable to core one also.

The core uncommit allows you to do `hg uncommit PATH` on a dirty wdir even if `experimental.uncommitondirtywdir` is set. We should change that and let `hg uncommit PATH` work only if `PATH` are not dirty in wdir. If they are dirty, use `experimental.uncommitondirtywdir`.

The end result on a dirty wdir should look like:

`hg uncommit` : abort and require `experimental.uncommitondirtywdir`
`hg uncommit PATH`: works if PATH is clean in wdir, aborts if PATH is dirty and require `experimental.uncommitondirtywdir`

How does that sound?

(Ignore my earlier comments as I didn't figured this out completely then)

(P.S. I hate to say that we have two uncommits which are kind of diverging)

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: make experimental.uncommitondirtydir to work on PATH (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
navaneeth.suresh updated this revision to Diff 14130.
navaneeth.suresh edited the summary of this revision.
navaneeth.suresh retitled this revision from "uncommit: add --allowdirtywcopy when possibly hiding data (issue5977)" to "uncommit: make experimental.uncommitondirtydir to work on PATH (issue5977)".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5940?vs=14100&id=14130

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

AFFECTED FILES
  hgext/uncommit.py
  tests/test-uncommit.t

CHANGE DETAILS

diff --git a/tests/test-uncommit.t b/tests/test-uncommit.t
--- a/tests/test-uncommit.t
+++ b/tests/test-uncommit.t
@@ -156,8 +156,12 @@
   M files
   $ hg uncommit
   abort: uncommitted changes
+  (requires experimental.uncommitondirtywdir to uncommit)
   [255]
   $ hg uncommit files
+  abort: uncommitted changes
+  (requires experimental.uncommitondirtywdir to uncommit)
+  [255]
   $ cat files
   abcde
   foo
@@ -168,6 +172,7 @@
   $ echo "bar" >> files
   $ hg uncommit
   abort: uncommitted changes
+  (requires experimental.uncommitondirtywdir to uncommit)
   [255]
   $ hg uncommit --config experimental.uncommitondirtywdir=True
   $ hg commit -m "files abcde + foo"
@@ -191,16 +196,16 @@
   +abc
   
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:48e5bd7cd583
   $ hg uncommit
   3 new orphan changesets
   $ hg status
   M files
   A file-abc
   $ hg heads -T '{rev}:{node} {desc}'
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo (no-eol)
+  9:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo (no-eol)
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:48e5bd7cd583
   $ hg commit -m 'new abc'
   created new head
 
@@ -222,38 +227,36 @@
   +ab
   
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:48e5bd7cd583
   $ hg uncommit file-ab
   1 new orphan changesets
   $ hg status
   A file-ab
 
   $ hg heads -T '{rev}:{node} {desc}\n'
-  12:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
-  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  11:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
+  10:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
+  9:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
 
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:48e5bd7cd583
   $ hg commit -m 'update ab'
   $ hg status
   $ hg heads -T '{rev}:{node} {desc}\n'
-  13:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
-  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  12:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
+  10:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
+  9:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
 
   $ hg log -G -T '{rev}:{node} {desc}' --hidden
-  @  13:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
+  @  12:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
   |
-  o  12:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
+  o  11:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
   |
-  | *  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
+  | *  10:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
   | |
-  | | *  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  | | *  9:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
   | | |
-  | | | x  9:8a6b58c173ca6a2e3745d8bd86698718d664bc6c files abcde + foo
-  | | |/
-  | | | x  8:39ad452c7f684a55d161c574340c5766c4569278 update files for abcde
+  | | | x  8:84beeba0ac30e19521c036e4d2dd3a5fa02586ff files abcde + foo
   | | |/
   | | | x  7:0977fa602c2fd7d8427ed4e7ee15ea13b84c9173 update files for abcde
   | | |/
@@ -275,22 +278,22 @@
 
   $ hg uncommit
   $ hg phase -r .
-  12: draft
+  11: draft
   $ hg commit -m 'update ab again'
 
 Phase is preserved
 
   $ hg uncommit --keep --config phases.new-commit=secret
   $ hg phase -r .
-  15: draft
+  14: draft
   $ hg commit --amend -m 'update ab again'
 
 Uncommit with public parent
 
   $ hg phase -p "::.^"
   $ hg uncommit
   $ hg phase -r .
-  12: public
+  11: public
 
 Partial uncommit with public parent
 
@@ -301,9 +304,9 @@
   $ hg status
   A xyz
   $ hg phase -r .
-  18: draft
+  17: draft
   $ hg phase -r ".^"
-  12: public
+  11: public
 
 Uncommit leaving an empty changeset
 
@@ -368,6 +371,7 @@
 
   $ hg uncommit
   abort: outstanding uncommitted merge
+  (requires experimental.uncommitondirtywdir to uncommit)
   [255]
 
   $ hg uncommit --config experimental.uncommitondirtywdir=True
@@ -398,3 +402,35 @@
   |/
   o  0:ea4e33293d4d274a2ba73150733c2612231f398c a 1
   
+  $ cd ..
+
+experimental.uncommitondirtywdir should also work for uncommit PATH
+
+  $ hg init issue5977
+  $ cd issue5977
+  $ echo 'super critical info!' > a
+  $ hg ci -Am 'add a'
+  adding a
+  $ echo 'foo' > a
+  $ hg unc a
+  abort: uncommitted changes
+  (requires experimental.uncommitondirtywdir to uncommit)
+  [255]
+  $ cat a
+  foo
+  $ hg log
+  changeset:   0:e37b99831f6e
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     add a
+  
+  $ hg unc --config experimental.uncommitondirtywdir=true a
+  $ hg log
+  changeset:   1:656ba143d384
+  tag:         tip
+  parent:      -1:000000000000
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     add a
+  
diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -155,9 +155,9 @@
 
     with repo.wlock(), repo.lock():
 
-        if not pats and not repo.ui.configbool('experimental',
-                                               'uncommitondirtywdir'):
-            cmdutil.bailifchanged(repo)
+        if not repo.ui.configbool('experimental', 'uncommitondirtywdir'):
+            cmdutil.bailifchanged(repo, hint=_('requires '
+                                  'experimental.uncommitondirtywdir to uncommit'))
         old = repo['.']
         rewriteutil.precheck(repo, [old.rev()], 'uncommit')
         if len(old.parents()) > 1:



To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: make experimental.uncommitondirtydir to work on PATH (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
navaneeth.suresh marked 4 inline comments as done.
navaneeth.suresh added inline comments.

INLINE COMMENTS

> pulkit wrote in uncommit.py:140
> Nice point. I had to revisit the bug to understand more now. IMO the bug says as following:
>
> `uncommit` should require a flag or config when it is trying to do the following:
>
> 1. uncommit some files which are dirty in wdir, and
> 2. the files are passed as `hg uncommit PATH`
>
> (refer Yuya's comment: https://bz.mercurial-scm.org/show_bug.cgi?id=5977#c1)
>
> However the bug was filed for evolve uncommit, it's applicable to core one also.
>
> The core uncommit allows you to do `hg uncommit PATH` on a dirty wdir even if `experimental.uncommitondirtywdir` is set. We should change that and let `hg uncommit PATH` work only if `PATH` are not dirty in wdir. If they are dirty, use `experimental.uncommitondirtywdir`.
>
> The end result on a dirty wdir should look like:
>
> `hg uncommit` : abort and require `experimental.uncommitondirtywdir`
> `hg uncommit PATH`: works if PATH is clean in wdir, aborts if PATH is dirty and require `experimental.uncommitondirtywdir`
>
> How does that sound?
>
> (Ignore my earlier comments as I didn't figured this out completely then)
>
> (P.S. I hate to say that we have two uncommits which are kind of diverging)

I quite like the idea. This should be a quick fix. I've updated the revision and edited the summary. Please see if it works as expected. In this revision, I'm doing only one thing. For the case of an outstanding uncommitted merge, I will be sending another patch as a follow-up.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: make experimental.uncommitondirtydir to work on PATH (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
navaneeth.suresh updated this revision to Diff 14131.
navaneeth.suresh marked an inline comment as done.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5940?vs=14130&id=14131

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

AFFECTED FILES
  hgext/uncommit.py
  tests/test-uncommit.t

CHANGE DETAILS

diff --git a/tests/test-uncommit.t b/tests/test-uncommit.t
--- a/tests/test-uncommit.t
+++ b/tests/test-uncommit.t
@@ -156,8 +156,12 @@
   M files
   $ hg uncommit
   abort: uncommitted changes
+  (requires experimental.uncommitondirtywdir to uncommit)
   [255]
   $ hg uncommit files
+  abort: uncommitted changes
+  (requires experimental.uncommitondirtywdir to uncommit)
+  [255]
   $ cat files
   abcde
   foo
@@ -168,6 +172,7 @@
   $ echo "bar" >> files
   $ hg uncommit
   abort: uncommitted changes
+  (requires experimental.uncommitondirtywdir to uncommit)
   [255]
   $ hg uncommit --config experimental.uncommitondirtywdir=True
   $ hg commit -m "files abcde + foo"
@@ -191,16 +196,16 @@
   +abc
   
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:48e5bd7cd583
   $ hg uncommit
   3 new orphan changesets
   $ hg status
   M files
   A file-abc
   $ hg heads -T '{rev}:{node} {desc}'
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo (no-eol)
+  9:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo (no-eol)
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:48e5bd7cd583
   $ hg commit -m 'new abc'
   created new head
 
@@ -222,38 +227,36 @@
   +ab
   
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:48e5bd7cd583
   $ hg uncommit file-ab
   1 new orphan changesets
   $ hg status
   A file-ab
 
   $ hg heads -T '{rev}:{node} {desc}\n'
-  12:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
-  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  11:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
+  10:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
+  9:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
 
   $ hg bookmark
-     foo                       10:48e5bd7cd583
+     foo                       9:48e5bd7cd583
   $ hg commit -m 'update ab'
   $ hg status
   $ hg heads -T '{rev}:{node} {desc}\n'
-  13:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
-  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
-  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  12:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
+  10:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
+  9:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
 
   $ hg log -G -T '{rev}:{node} {desc}' --hidden
-  @  13:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
+  @  12:f21039c59242b085491bb58f591afc4ed1c04c09 update ab
   |
-  o  12:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
+  o  11:8eb87968f2edb7f27f27fe676316e179de65fff6 added file-ab
   |
-  | *  11:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
+  | *  10:5dc89ca4486f8a88716c5797fa9f498d13d7c2e1 new abc
   | |
-  | | *  10:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
+  | | *  9:48e5bd7cd583eb24164ef8b89185819c84c96ed7 files abcde + foo
   | | |
-  | | | x  9:8a6b58c173ca6a2e3745d8bd86698718d664bc6c files abcde + foo
-  | | |/
-  | | | x  8:39ad452c7f684a55d161c574340c5766c4569278 update files for abcde
+  | | | x  8:84beeba0ac30e19521c036e4d2dd3a5fa02586ff files abcde + foo
   | | |/
   | | | x  7:0977fa602c2fd7d8427ed4e7ee15ea13b84c9173 update files for abcde
   | | |/
@@ -275,22 +278,22 @@
 
   $ hg uncommit
   $ hg phase -r .
-  12: draft
+  11: draft
   $ hg commit -m 'update ab again'
 
 Phase is preserved
 
   $ hg uncommit --keep --config phases.new-commit=secret
   $ hg phase -r .
-  15: draft
+  14: draft
   $ hg commit --amend -m 'update ab again'
 
 Uncommit with public parent
 
   $ hg phase -p "::.^"
   $ hg uncommit
   $ hg phase -r .
-  12: public
+  11: public
 
 Partial uncommit with public parent
 
@@ -301,9 +304,9 @@
   $ hg status
   A xyz
   $ hg phase -r .
-  18: draft
+  17: draft
   $ hg phase -r ".^"
-  12: public
+  11: public
 
 Uncommit leaving an empty changeset
 
@@ -368,6 +371,7 @@
 
   $ hg uncommit
   abort: outstanding uncommitted merge
+  (requires experimental.uncommitondirtywdir to uncommit)
   [255]
 
   $ hg uncommit --config experimental.uncommitondirtywdir=True
@@ -398,3 +402,35 @@
   |/
   o  0:ea4e33293d4d274a2ba73150733c2612231f398c a 1
   
+  $ cd ..
+
+experimental.uncommitondirtywdir should also work for uncommit PATH
+
+  $ hg init issue5977
+  $ cd issue5977
+  $ echo 'super critical info!' > a
+  $ hg ci -Am 'add a'
+  adding a
+  $ echo 'foo' > a
+  $ hg unc a
+  abort: uncommitted changes
+  (requires experimental.uncommitondirtywdir to uncommit)
+  [255]
+  $ cat a
+  foo
+  $ hg log
+  changeset:   0:e37b99831f6e
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     add a
+  
+  $ hg unc --config experimental.uncommitondirtywdir=true a
+  $ hg log
+  changeset:   1:656ba143d384
+  tag:         tip
+  parent:      -1:000000000000
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     add a
+  
diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -155,9 +155,9 @@
 
     with repo.wlock(), repo.lock():
 
-        if not pats and not repo.ui.configbool('experimental',
-                                               'uncommitondirtywdir'):
-            cmdutil.bailifchanged(repo)
+        if not repo.ui.configbool('experimental', 'uncommitondirtywdir'):
+            cmdutil.bailifchanged(repo, hint=_('requires '
+                              'experimental.uncommitondirtywdir to uncommit'))
         old = repo['.']
         rewriteutil.precheck(repo, [old.rev()], 'uncommit')
         if len(old.parents()) > 1:



To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: make experimental.uncommitondirtydir to work on PATH (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
navaneeth.suresh added a comment.


  Ping for review.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: make experimental.uncommitondirtydir to work on PATH (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
pulkit added a comment.


  This patch changes uncommit behavior to always require `experimental.uncommitondirtywdir` option if wdir is dirty. However what we want when wdir is dirty is as follows:
 
  `hg uncommit` : abort and require `experimental.uncommitondirtywdir`
  `hg uncommit PATH`: works if `PATH` is clean in wdir, aborts if PATH is dirty and require `experimental.uncommitondirtywdir`

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5940: uncommit: make experimental.uncommitondirtydir to work on PATH (issue5977)

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
martinvonz added a comment.


  Do we really want to tell the user to set an experimental option? I prefer exposing it as --allow-dirty-working-copy.

REPOSITORY
  rHG Mercurial

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
123