D6768: split: handle partial commit of renames when doing split or record (issue5723)

[spectral (Kyle Lippincott)]

spectral created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  When using split or record, using either interface (text or curses), selecting
  portions of the file to be committed/recorded did not work; the entire file was
  treated as having been selected. This was because the logic for handling partial
  application of the patches relies on knowing what files are "new with
  modifications" and it doesn't treat "rename destination" as "new".
  
  There was a complicating issue, however. We're relying on the patch header
  specifying the copy from/to information, which works as long as the 'copy from'
  file is there. In the case of renames, however, the 'rename from' file is *not*
  there, so we need to add it back.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/cmdutil.py
  mercurial/patch.py
  tests/test-split.t

CHANGE DETAILS

diff --git a/tests/test-split.t b/tests/test-split.t
--- a/tests/test-split.t
+++ b/tests/test-split.t
@@ -790,6 +790,96 @@
   [255]
 #endif
 
+Test that splitting moves works properly (issue5723)
+----------------------------------------------------
+
+  $ hg init $TESTTMP/issue5723-mv
+  $ cd $TESTTMP/issue5723-mv
+  $ printf '1\n2\n' > file
+  $ hg ci -qAm initial
+  $ hg mv file file2
+  $ printf 'a\nb\n1\n2\n3\n4\n' > file2
+  $ cat > $TESTTMP/messages <<EOF
+  > split1, keeping only the numbered lines
+  > --
+  > split2, keeping the lettered lines
+  > EOF
+  $ hg ci -m 'move and modify'
+  $ printf 'y\nn\na\na\n' | hg split
+  diff --git a/file b/file2
+  rename from file
+  rename to file2
+  2 hunks, 4 lines changed
+  examine changes to 'file' and 'file2'?
+  (enter ? for help) [Ynesfdaq?] y
+  
+  @@ -0,0 +1,2 @@
+  +a
+  +b
+  record change 1/2 to 'file2'?
+  (enter ? for help) [Ynesfdaq?] n
+  
+  @@ -2,0 +5,2 @@ 2
+  +3
+  +4
+  record change 2/2 to 'file2'?
+  (enter ? for help) [Ynesfdaq?] a
+  
+  EDITOR: HG: Splitting 8c42fa635116. Write commit message for the first split changeset.
+  EDITOR: move and modify
+  EDITOR: 
+  EDITOR: 
+  EDITOR: HG: Enter commit message.  Lines beginning with 'HG:' are removed.
+  EDITOR: HG: Leave message empty to abort commit.
+  EDITOR: HG: --
+  EDITOR: HG: user: test
+  EDITOR: HG: branch 'default'
+  EDITOR: HG: added file2
+  EDITOR: HG: removed file
+  created new head
+  diff --git a/file2 b/file2
+  1 hunks, 2 lines changed
+  examine changes to 'file2'?
+  (enter ? for help) [Ynesfdaq?] a
+  
+  EDITOR: HG: Splitting 8c42fa635116. So far it has been split into:
+  EDITOR: HG: - 478be2a70c27: split1, keeping only the numbered lines
+  EDITOR: HG: Write commit message for the next split changeset.
+  EDITOR: move and modify
+  EDITOR: 
+  EDITOR: 
+  EDITOR: HG: Enter commit message.  Lines beginning with 'HG:' are removed.
+  EDITOR: HG: Leave message empty to abort commit.
+  EDITOR: HG: --
+  EDITOR: HG: user: test
+  EDITOR: HG: branch 'default'
+  EDITOR: HG: changed file2
+  saved backup bundle to $TESTTMP/issue5723-mv/.hg/strip-backup/8c42fa635116-a38044d4-split.hg (obsstore-off !)
+  $ hg log -T '{desc}: {files%"{file} "}\n'
+  split2, keeping the lettered lines: file2 
+  split1, keeping only the numbered lines: file file2 
+  initial: file 
+  $ cat file2
+  a
+  b
+  1
+  2
+  3
+  4
+  $ hg cat -r ".^" file2
+  1
+  2
+  3
+  4
+  $ hg cat -r . file2
+  a
+  b
+  1
+  2
+  3
+  4
+
+
 Test that splitting copies works properly (issue5723)
 ----------------------------------------------------
 
diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -864,7 +864,7 @@
     allhunks_re = re.compile('(?:index|deleted file) ')
     pretty_re = re.compile('(?:new file|deleted file) ')
     special_re = re.compile('(?:index|deleted|copy|rename|new mode) ')
-    newfile_re = re.compile('(?:new file|copy to)')
+    newfile_re = re.compile('(?:new file|copy to|rename to)')
 
     def __init__(self, header):
         self.header = header
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -181,11 +181,14 @@
 
 def newandmodified(chunks, originalchunks):
     newlyaddedandmodifiedfiles = set()
+    alsorestore = set()
     for chunk in chunks:
         if (ishunk(chunk) and chunk.header.isnewfile() and chunk not in
             originalchunks):
             newlyaddedandmodifiedfiles.add(chunk.header.filename())
-    return newlyaddedandmodifiedfiles
+            alsorestore.update(set(chunk.header.files()) -
+                               set([chunk.header.filename()]))
+    return newlyaddedandmodifiedfiles, alsorestore
 
 def parsealiases(cmd):
     return cmd.split("|")
@@ -326,8 +329,11 @@
 
         # We need to keep a backup of files that have been newly added and
         # modified during the recording process because there is a previous
-        # version without the edit in the workdir
-        newlyaddedandmodifiedfiles = newandmodified(chunks, originalchunks)
+        # version without the edit in the workdir. We also will need to restore
+        # files that were the sources of renames so that the patch application
+        # works.
+        newlyaddedandmodifiedfiles, alsorestore = newandmodified(chunks,
+                                                                 originalchunks)
         contenders = set()
         for h in chunks:
             try:
@@ -392,7 +398,7 @@
             # 3a. apply filtered patch to clean repo  (clean)
             if backups:
                 # Equivalent to hg.revert
-                m = scmutil.matchfiles(repo, backups.keys())
+                m = scmutil.matchfiles(repo, set(backups.keys()) | alsorestore)
                 mergemod.update(repo, repo.dirstate.p1(), branchmerge=False,
                                 force=True, matcher=m)
 
@@ -3172,7 +3178,13 @@
         except error.PatchError as err:
             raise error.Abort(_('error parsing patch: %s') % err)
 
-        newlyaddedandmodifiedfiles = newandmodified(chunks, originalchunks)
+        # FIXME: when doing an interactive revert of a copy, there's no way of
+        # performing a partial revert of the added file, the only option is
+        # "remove added file <name> (Yn)?", so we don't need to worry about the
+        # alsorestore value. Ideally we'd be able to partially revert
+        # copied/renamed files.
+        newlyaddedandmodifiedfiles, unusedalsorestore = newandmodified(
+                chunks, originalchunks)
         if tobackup is None:
             tobackup = set()
         # Apply changes



To: spectral, #hg-reviewers
Cc: mercurial-devel