Patchwork [4,of,4,V2] revert: properly backup added files with local modification

login
register
mail settings
Submitter Pierre-Yves David
Date Sept. 30, 2014, 6:05 p.m.
Message ID <d4b4b1260fbc7abb9288.1412100340@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/6050/
State Accepted
Headers show

Comments

Pierre-Yves David - Sept. 30, 2014, 6:05 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1409482860 -7200
#      Sun Aug 31 13:01:00 2014 +0200
# Node ID d4b4b1260fbc7abb92889450e1bbddf984939bb7
# Parent  a58e6d9115fca974a01c41ee68fe4619c072792c
revert: properly backup added files with local modification

This files were previously not backed up because the backup mechanism was not
smart enough. This lead to data lost from user since uncommitted content were
lost.

We now properly move the modified version to <filename>.orig before deleting it.

We have to use a small hack to do a different action if "--no-backup" is
specified. This is needed because the backup process is actually a move (not a
copy) so the file is already missing when we backup. The internet kitten is a
bit disapointed about that, but such is live.

This patch concludes the "lets refactor revert" phases. We can now open the
"Lets find stupid bug with renames and merge" phases.

I'm sure that now that the code is clearer we could do it in another simpler
way, but I consider the current improvement good enough for now.
Pierre-Yves David - Oct. 2, 2014, 2 a.m.
On 09/30/2014 01:05 PM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1409482860 -7200
> #      Sun Aug 31 13:01:00 2014 +0200
> # Node ID d4b4b1260fbc7abb92889450e1bbddf984939bb7
> # Parent  a58e6d9115fca974a01c41ee68fe4619c072792c
> revert: properly backup added files with local modification

This four patch have been pushed to the clowncopter after Matt reviewed 
them in real life. They go along multiple bookmark related patches.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2628,10 +2628,15 @@  def revert(ui, repo, ctx, parents, *pats
         check = 1   # check if existing file differ from target
         discard = 0 # never do backup
         if opts.get('no_backup'):
             backup = check = discard
 
+
+        backupanddel = actions['remove']
+        if not opts.get('no_backup'):
+            backupanddel = actions['drop']
+
         disptable = (
             # dispatch table:
             #   file state
             #   action
             #   make backup
@@ -2646,11 +2651,11 @@  def revert(ui, repo, ctx, parents, *pats
             # Added since target
             (added,         actions['remove'],   discard),
             # Added in working directory
             (dsadded,       actions['forget'],   discard),
             # Added since target, have local modification
-            (modadded,      actions['remove'],   discard),
+            (modadded,      backupanddel,        backup),
             # Added since target but file is missing in working directory
             (deladded,      actions['drop'],   discard),
             # Removed since  target, before working copy parent
             (removed,       actions['add'],      discard),
             # Same as `removed` but an unknown file exists at the same path
diff --git a/tests/test-revert.t b/tests/test-revert.t
--- a/tests/test-revert.t
+++ b/tests/test-revert.t
@@ -948,10 +948,11 @@  Misbehavior:
   $ python ../dircontent.py > ../content-base-all.txt
   $ cd ..
   $ diff -U 0 -- content-base.txt content-base-all.txt | grep _
   +parent added_untracked-clean
   +wc     added_untracked-wc
+  +wc     added_wc.orig
   +wc     clean_untracked-wc.orig
   +wc     clean_wc.orig
   +wc     missing_untracked-wc
   +wc     missing_wc
   +parent modified_untracked-clean.orig