changeset 29498:1b38cfde9530

revert: don't backup if no files reverted in interactive mode (issue4793)

When reverting interactively, we always backup files before prompting the user
to find out if they actually want to revert them. This can create spurious
*.orig files if a user enters an interactive revert session and then doesn't
revert any files. Instead, we should only backup files that are actually being
touched.

--- a/mercurial/cmdutil.py	Tue Jul 05 07:25:51 2016 +0900
+++ b/mercurial/cmdutil.py	Thu Jun 30 08:38:19 2016 -0700
@@ -3143,11 +3143,17 @@
         # All set to `discard` if `no-backup` is set do avoid checking
         # no_backup lower in the code.
         # These values are ordered for comparison purposes
+        backupinteractive = 3 # do backup if interactively modified
         backup = 2  # unconditionally do backup
         check = 1   # check if the existing file differs from target
         discard = 0 # never do backup
         if opts.get('no_backup'):
-            backup = check = discard
+            backupinteractive = backup = check = discard
+        if interactive:
+            dsmodifiedbackup = backupinteractive
+        else:
+            dsmodifiedbackup = backup
+        tobackup = set()
 
         backupanddel = actions['remove']
         if not opts.get('no_backup'):
@@ -3165,7 +3171,7 @@
             # Modified compared to target, but local file is deleted
             (deleted,       actions['revert'],   discard),
             # Modified compared to target, local change
-            (dsmodified,    actions['revert'],   backup),
+            (dsmodified,    actions['revert'],   dsmodifiedbackup),
             # Added since target
             (added,         actions['remove'],   discard),
             # Added in working directory
@@ -3200,8 +3206,12 @@
                     continue
                 if xlist is not None:
                     xlist.append(abs)
-                    if dobackup and (backup <= dobackup
-                                     or wctx[abs].cmp(ctx[abs])):
+                    if dobackup:
+                        # If in interactive mode, don't automatically create
+                        # .orig files (issue4793)
+                        if dobackup == backupinteractive:
+                            tobackup.add(abs)
+                        elif (backup <= dobackup or wctx[abs].cmp(ctx[abs])):
                             bakname = scmutil.origpath(ui, repo, rel)
                             ui.note(_('saving current version of %s as %s\n') %
                                     (rel, bakname))
@@ -3221,7 +3231,7 @@
         if not opts.get('dry_run'):
             needdata = ('revert', 'add', 'undelete')
             _revertprefetch(repo, ctx, *[actions[name][0] for name in needdata])
-            _performrevert(repo, parents, ctx, actions, interactive)
+            _performrevert(repo, parents, ctx, actions, interactive, tobackup)
 
         if targetsubs:
             # Revert the subrepos on the revert list
@@ -3236,7 +3246,8 @@
     """Let extension changing the storage layer prefetch content"""
     pass
 
-def _performrevert(repo, parents, ctx, actions, interactive=False):
+def _performrevert(repo, parents, ctx, actions, interactive=False,
+                   tobackup=None):
     """function that actually perform all the actions computed for revert
 
     This is an independent function to let extension to plug in and react to
@@ -3316,9 +3327,18 @@
             raise error.Abort(_('error parsing patch: %s') % err)
 
         newlyaddedandmodifiedfiles = newandmodified(chunks, originalchunks)
+        if tobackup is None:
+            tobackup = set()
         # Apply changes
         fp = stringio()
         for c in chunks:
+            # Create a backup file only if this hunk should be backed up
+            if ishunk(c) and c.header.filename() in tobackup:
+                abs = c.header.filename()
+                target = repo.wjoin(abs)
+                bakname = scmutil.origpath(repo.ui, repo, m.rel(abs))
+                util.copyfile(target, bakname)
+                tobackup.remove(abs)
             c.write(fp)
         dopatch = fp.tell()
         fp.seek(0)

--- a/tests/test-revert-interactive.t	Tue Jul 05 07:25:51 2016 +0900
+++ b/tests/test-revert-interactive.t	Thu Jun 30 08:38:19 2016 -0700
@@ -134,7 +134,41 @@
   
   abort: user quit
   [255]
-  $ rm folder1/g.orig
+  $ ls folder1/
+  g
+
+Test that a noop revert doesn't do an unecessary backup
+  $ (echo y; echo n) | hg revert -i -r 2 folder1/g
+  diff --git a/folder1/g b/folder1/g
+  1 hunks, 1 lines changed
+  examine changes to 'folder1/g'? [Ynesfdaq?] y
+  
+  @@ -3,3 +3,4 @@
+   3
+   4
+   5
+  +d
+  revert this change to 'folder1/g'? [Ynesfdaq?] n
+  
+  $ ls folder1/
+  g
+
+Test --no-backup
+  $ (echo y; echo y) | hg revert -i -C -r 2 folder1/g
+  diff --git a/folder1/g b/folder1/g
+  1 hunks, 1 lines changed
+  examine changes to 'folder1/g'? [Ynesfdaq?] y
+  
+  @@ -3,3 +3,4 @@
+   3
+   4
+   5
+  +d
+  revert this change to 'folder1/g'? [Ynesfdaq?] y
+  
+  $ ls folder1/
+  g
+  >>> open('folder1/g', 'wb').write("1\n2\n3\n4\n5\nd\n")
 
 
   $ hg update -C 6