Patchwork D10936: cmdutil: fix newandmodified file accounting for --interactive commits

login
register
mail settings
Submitter phabricator
Date July 2, 2021, 8:18 p.m.
Message ID <differential-rev-PHID-DREV-y4eotedtkeoqxm3uxsuj-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49250/
State Superseded
Headers show

Comments

phabricator - July 2, 2021, 8:18 p.m.
dploch created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  `originalchunks` is a misleading name, because it only contains header objects, which are flattened to selected hunks by the filter function. As such, `chunks not in originalchunks` is always True and misleading, because hunk objects never compare equal to header objects. This change fixes the internal naming and removes the useless parameter from the method.
  
  This change also fixes https://bz.mercurial-scm.org/show_bug.cgi?id=6533, by considering the filtered headers, rather than the hunks, when determining new and modified files. If a file is renamed + edited, and the edited hunks are deselected (but the file is not), the filtered chunks will contain a header for the file (because it's .special()) but but no hunks.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/cmdutil.py
  tests/test-commit-interactive.t

CHANGE DETAILS




To: dploch, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-commit-interactive.t b/tests/test-commit-interactive.t
--- a/tests/test-commit-interactive.t
+++ b/tests/test-commit-interactive.t
@@ -1713,20 +1713,58 @@ 
   record this change to 'plain3'?
   (enter ? for help) [Ynesfdaq?] y
   
+
+Rename file but discard edits
+
+  $ echo content > new-file
+  $ hg add -q new-file
+  $ hg commit -qm 'new file'
+  $ hg mv new-file renamed-file
+  $ echo new-content >> renamed-file
+  $ hg commit -i -d '24 0' -m content-rename<<EOF
+  > y
+  > n
+  > EOF
+  diff --git a/new-file b/renamed-file
+  rename from new-file
+  rename to renamed-file
+  1 hunks, 1 lines changed
+  examine changes to 'new-file' and 'renamed-file'?
+  (enter ? for help) [Ynesfdaq?] y
+  
+  @@ -1,1 +1,2 @@
+   content
+  +new-content
+  record this change to 'renamed-file'?
+  (enter ? for help) [Ynesfdaq?] n
+  
+  $ hg status
+  M renamed-file
+  ? editedfile.orig
+  ? editedfile.rej
+  ? editor.sh
+  $ hg diff
+  diff -r a006b0d78ce9 renamed-file
+  --- a/renamed-file	Thu Jan 01 00:00:24 1970 +0000
+  +++ b/renamed-file	Thu Jan 01 00:00:00 1970 +0000
+  @@ -1,1 +1,2 @@
+   content
+  +new-content
+
 The #if execbit block above changes the hash here on some systems
   $ hg status -A plain3
   C plain3
   $ hg tip
-  changeset:   32:* (glob)
+  changeset:   34:a006b0d78ce9
   tag:         tip
   user:        test
-  date:        Thu Jan 01 00:00:23 1970 +0000
-  summary:     moving_files
+  date:        Thu Jan 01 00:00:24 1970 +0000
+  summary:     content-rename
   
 Editing patch of newly added file
 
   $ hg update -C .
-  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ cat > editor.sh << '__EOF__'
   > cat "$1"  | sed "s/first/very/g"  > tt
   > mv tt  "$1"
@@ -1737,7 +1775,7 @@ 
   > This is the third line
   > __EOF__
   $ hg add newfile
-  $ HGEDITOR="\"sh\" \"`pwd`/editor.sh\"" hg commit -i -d '23 0' -medit-patch-new <<EOF
+  $ HGEDITOR="\"sh\" \"`pwd`/editor.sh\"" hg commit -i -d '25 0' -medit-patch-new <<EOF
   > y
   > e
   > EOF
@@ -1770,7 +1808,7 @@ 
   $ cd folder
   $ echo "foo" > bar
   $ hg add bar
-  $ hg commit -i -d '23 0' -mnewfilesubdir  <<EOF
+  $ hg commit -i -d '26 0' -mnewfilesubdir  <<EOF
   > y
   > y
   > EOF
@@ -1786,15 +1824,15 @@ 
   
 The #if execbit block above changes the hashes here on some systems
   $ hg tip -p
-  changeset:   34:* (glob)
+  changeset:   36:6d2ed8a25e2a
   tag:         tip
   user:        test
-  date:        Thu Jan 01 00:00:23 1970 +0000
+  date:        Thu Jan 01 00:00:26 1970 +0000
   summary:     newfilesubdir
   
   diff -r * -r * folder/bar (glob)
   --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
-  +++ b/folder/bar	Thu Jan 01 00:00:23 1970 +0000
+  +++ b/folder/bar	Thu Jan 01 00:00:26 1970 +0000
   @@ -0,0 +1,1 @@
   +foo
   
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -346,18 +346,19 @@ 
     return isinstance(x, hunkclasses)
 
 
-def newandmodified(chunks, originalchunks):
+def isheader(x):
+    headerclasses = (crecordmod.uiheader, patch.header)
+    return isinstance(x, headerclasses)
+
+
+def newandmodified(chunks):
     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())
+        if isheader(chunk) and chunk.isnewfile():
+            newlyaddedandmodifiedfiles.add(chunk.filename())
             alsorestore.update(
-                set(chunk.header.files()) - {chunk.header.filename()}
+                set(chunk.files()) - {chunk.filename()}
             )
     return newlyaddedandmodifiedfiles, alsorestore
 
@@ -517,12 +518,12 @@ 
         diffopts.git = True
         diffopts.showfunc = True
         originaldiff = patch.diff(repo, changes=status, opts=diffopts)
-        originalchunks = patch.parsepatch(originaldiff)
+        originalhdrs = patch.parsepatch(originaldiff)
         match = scmutil.match(repo[None], pats)
 
         # 1. filter patch, since we are intending to apply subset of it
         try:
-            chunks, newopts = filterfn(ui, originalchunks, match)
+            chunks, newopts = filterfn(ui, originalhdrs, match)
         except error.PatchError as err:
             raise error.InputError(_(b'error parsing patch: %s') % err)
         opts.update(newopts)
@@ -532,15 +533,11 @@ 
         # 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
-        )
+        newlyaddedandmodifiedfiles, alsorestore = newandmodified(chunks)
         contenders = set()
         for h in chunks:
-            try:
+            if isheader(h):
                 contenders.update(set(h.files()))
-            except AttributeError:
-                pass
 
         changed = status.modified + status.added + status.removed
         newfiles = [f for f in changed if f in contenders]
@@ -3630,12 +3627,12 @@ 
             diff = patch.diff(repo, None, ctx.node(), m, opts=diffopts)
         else:
             diff = patch.diff(repo, ctx.node(), None, m, opts=diffopts)
-        originalchunks = patch.parsepatch(diff)
+        originalhdrs = patch.parsepatch(diff)
 
         try:
 
             chunks, opts = recordfilter(
-                repo.ui, originalchunks, match, operation=operation
+                repo.ui, originalhdrs, match, operation=operation
             )
             if operation == b'discard':
                 chunks = patch.reversehunks(chunks)
@@ -3648,9 +3645,7 @@ 
         # "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
-        )
+        newlyaddedandmodifiedfiles, unusedalsorestore = newandmodified(chunks)
         if tobackup is None:
             tobackup = set()
         # Apply changes