Patchwork [2,of,2,STABLE] record: edit patch of newly added files (issue4304)

login
register
mail settings
Submitter Laurent Charignon
Date April 24, 2015, 12:32 a.m.
Message ID <d1db0be0122cfc5ae637.1429835520@lcharignon-mbp.local>
Download mbox | patch
Permalink /patch/8779/
State Superseded
Commit 8133494accf1bd1bd8c6d1fbcd486ac5fa050643
Headers show

Comments

Laurent Charignon - April 24, 2015, 12:32 a.m.
# HG changeset patch
# User Laurent Charignon <lcharignon@fb.com>
# Date 1429824446 25200
#      Thu Apr 23 14:27:26 2015 -0700
# Branch stable
# Node ID d1db0be0122cfc5ae637e4471c63e9d3c471451e
# Parent  b41aabbdd7fa16475ae4a5b5396c3928aa22b883
record: edit patch of newly added files (issue4304)

I tried to fix this issue in the past and had to revert the fix. This is a
second attempt without the regression we found with the first one.

record defines special headers (of file) as headers whose hunk are not shown
to the user for editing, they are used to represent deleted, moved and new
files. Since we want to authorize editing the patch of newly added file we
make the newly added file with some content not special anymore. This entails
that we have to save their content before applying the backup to be able to
revert it if the patch does not apply properly.
We reintroduce the test showing that newly added files can be edited and that
their content is shown to the user.
Laurent Charignon - April 24, 2015, 12:34 a.m.
This is the proper fix of issue4304

On 4/23/15, 5:32 PM, "Laurent Charignon" <lcharignon@fb.com> wrote:

># HG changeset patch
># User Laurent Charignon <lcharignon@fb.com>
># Date 1429824446 25200
>#      Thu Apr 23 14:27:26 2015 -0700
># Branch stable
># Node ID d1db0be0122cfc5ae637e4471c63e9d3c471451e
># Parent  b41aabbdd7fa16475ae4a5b5396c3928aa22b883
>record: edit patch of newly added files (issue4304)
>
>I tried to fix this issue in the past and had to revert the fix. This is a
>second attempt without the regression we found with the first one.
>
>record defines special headers (of file) as headers whose hunk are not
>shown
>to the user for editing, they are used to represent deleted, moved and new
>files. Since we want to authorize editing the patch of newly added file we
>make the newly added file with some content not special anymore. This
>entails
>that we have to save their content before applying the backup to be able
>to
>revert it if the patch does not apply properly.
>We reintroduce the test showing that newly added files can be edited and
>that
>their content is shown to the user.
>
>diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>--- a/mercurial/cmdutil.py
>+++ b/mercurial/cmdutil.py
>@@ -59,6 +59,8 @@
> def dorecord(ui, repo, commitfunc, cmdsuggest, backupall,
>             filterfn, *pats, **opts):
>     import merge as mergemod
>+    hunkclasses = (crecordmod.uihunk, patch.recordhunk)
>+    ishunk = lambda x: isinstance(x, hunkclasses)
> 
>     if not ui.interactive():
>         raise util.Abort(_('running non-interactively, use %s instead') %
>@@ -102,6 +104,14 @@
>         except patch.PatchError, err:
>             raise util.Abort(_('error parsing patch: %s') % err)
> 
>+        # 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 = set()
>+        for chunk in chunks:
>+            if ishunk(chunk) and chunk.header.isnewfile() and chunk not
>in \
>+                originalchunks:
>+                newlyaddedandmodifiedfiles.add(chunk.header.filename())
>         contenders = set()
>         for h in chunks:
>             try:
>@@ -122,8 +132,8 @@
>         if backupall:
>             tobackup = changed
>         else:
>-            tobackup = [f for f in newfiles if f in modified]
>-
>+            tobackup = [f for f in newfiles if f in modified or f in \
>+                    newlyaddedandmodifiedfiles]
>         backups = {}
>         if tobackup:
>             backupdir = repo.join('record-backups')
>@@ -151,6 +161,7 @@
>             dopatch = fp.tell()
>             fp.seek(0)
> 
>+            [os.unlink(c) for c in newlyaddedandmodifiedfiles]
>             # 3a. apply filtered patch to clean repo  (clean)
>             if backups:
>                 # Equivalent to hg.revert
>diff --git a/mercurial/patch.py b/mercurial/patch.py
>--- a/mercurial/patch.py
>+++ b/mercurial/patch.py
>@@ -820,9 +820,10 @@
>     """
>     diffgit_re = re.compile('diff --git a/(.*) b/(.*)$')
>     diff_re = re.compile('diff -r .* (.*)$')
>-    allhunks_re = re.compile('(?:index|new file|deleted file) ')
>+    allhunks_re = re.compile('(?:index|deleted file) ')
>     pretty_re = re.compile('(?:new file|deleted file) ')
>-    special_re = re.compile('(?:index|new|deleted|copy|rename) ')
>+    special_re = re.compile('(?:index|deleted|copy|rename) ')
>+    newfile_re = re.compile('(?:new file)')
> 
>     def __init__(self, header):
>         self.header = header
>@@ -870,8 +871,21 @@
>     def __repr__(self):
>         return '<header %s>' % (' '.join(map(repr, self.files())))
> 
>+    def isnewfile(self):
>+        return util.any(self.newfile_re.match(h) for h in self.header)
>+
>     def special(self):
>-        return util.any(self.special_re.match(h) for h in self.header)
>+        # Special files are shown only at the header level and not at
>the hunk
>+        # level for example a file that has been deleted is a special
>file.
>+        # The user cannot change the content of the operation, in the
>case of
>+        # the deleted file he has to take the deletion or not take it, he
>+        # cannot take some of it.
>+        # Newly added files are special if they are empty, they are not
>special
>+        # if they have some content as we want to be able to change it
>+        nocontent = len(self.header) == 2
>+        emptynewfile = self.isnewfile() and nocontent
>+        return emptynewfile or \
>+                util.any(self.special_re.match(h) for h in self.header)
> 
> class recordhunk(object):
>     """patch hunk
>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
>@@ -229,11 +229,25 @@
>   $ hg add plain
>   $ hg commit -i -d '7 0' -m plain plain<<EOF
>   > y
>+  > y
>   > EOF
>   diff --git a/plain b/plain
>   new file mode 100644
>   examine changes to 'plain'? [Ynesfdaq?] y
>   
>+  @@ -0,0 +1,10 @@
>+  +1
>+  +2
>+  +3
>+  +4
>+  +5
>+  +6
>+  +7
>+  +8
>+  +9
>+  +10
>+  record this change to 'plain'? [Ynesfdaq?] y
>+  
>   $ hg tip -p
>   changeset:   7:11fb457c1be4
>   tag:         tip
>@@ -315,6 +329,7 @@
>   > y
>   > y
>   > y
>+  > y
>   > EOF
>   diff --git a/plain b/plain
>   1 hunks, 1 lines changed
>@@ -333,6 +348,10 @@
>   new file mode 100644
>   examine changes to 'plain2'? [Ynesfdaq?] y
>   
>+  @@ -0,0 +1,1 @@
>+  +1
>+  record change 2/2 to 'plain2'? [Ynesfdaq?] y
>+  
> Modify beginning, trim end, record both, add another file to test
> changes numbering
> 
>@@ -1406,4 +1425,41 @@
>   date:        Thu Jan 01 00:00:23 1970 +0000
>   summary:     moving_files
>   
>+Editing patch of newly added file
>+
>+  $ hg update -C .
>+  0 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"
>+  > __EOF__
>+  $ cat > newfile << '__EOF__'
>+  > This is the first line
>+  > This is the second line
>+  > This is the third line
>+  > __EOF__
>+  $ hg add newfile
>+  $ HGEDITOR="\"sh\" \"`pwd`/editor.sh\"" hg commit -i -d '23 0'
>-medit-patch-new <<EOF
>+  > y
>+  > e
>+  > EOF
>+  diff --git a/newfile b/newfile
>+  new file mode 100644
>+  examine changes to 'newfile'? [Ynesfdaq?] y
>+  
>+  @@ -0,0 +1,3 @@
>+  +This is the first line
>+  +This is the second line
>+  +This is the third line
>+  record this change to 'newfile'? [Ynesfdaq?] e
>+  
>+  $ hg cat -r tip newfile
>+  This is the very line
>+  This is the second line
>+  This is the third line
>+
>+  $ cat newfile
>+  This is the first line
>+  This is the second line
>+  This is the third line
>   $ cd ..
>diff --git a/tests/test-keyword.t b/tests/test-keyword.t
>--- a/tests/test-keyword.t
>+++ b/tests/test-keyword.t
>@@ -479,6 +479,12 @@
>   new file mode 100644
>   examine changes to 'r'? [Ynesfdaq?] y
>   
>+  @@ -0,0 +1,1 @@
>+  +$Id$
>+  record this change to 'r'? [Ynesfdaq?] y
>+  
>+  resolving manifests
>+  patching file r
>   committing files:
>   r
>   committing manifest
>@@ -507,6 +513,12 @@
>   new file mode 100644
>   examine changes to 'i'? [Ynesfdaq?] y
>   
>+  @@ -0,0 +1,1 @@
>+  +$Id$
>+  record this change to 'i'? [Ynesfdaq?] y
>+  
>+  resolving manifests
>+  patching file i
>   committing files:
>   i
>   committing manifest
>diff --git a/tests/test-mq-subrepo.t b/tests/test-mq-subrepo.t
>--- a/tests/test-mq-subrepo.t
>+++ b/tests/test-mq-subrepo.t
>@@ -295,6 +295,11 @@
>   new file mode 100644
>   examine changes to '.hgsub'? [Ynesfdaq?] y
>   
>+  @@ -0,0 +1,1 @@
>+  +sub = sub
>+  record this change to '.hgsub'? [Ynesfdaq?] y
>+  
>+  warning: subrepo spec file '.hgsub' not found
>   abort: uncommitted changes in subrepository 'sub'
>   [255]
>   % update substate when adding .hgsub w/clean updated subrepo
>@@ -304,6 +309,11 @@
>   new file mode 100644
>   examine changes to '.hgsub'? [Ynesfdaq?] y
>   
>+  @@ -0,0 +1,1 @@
>+  +sub = sub
>+  record this change to '.hgsub'? [Ynesfdaq?] y
>+  
>+  warning: subrepo spec file '.hgsub' not found
>   path sub
>    source   sub
>    revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31
>_______________________________________________
>Mercurial-devel mailing list
>Mercurial-devel@selenic.com
>http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -59,6 +59,8 @@ 
 def dorecord(ui, repo, commitfunc, cmdsuggest, backupall,
             filterfn, *pats, **opts):
     import merge as mergemod
+    hunkclasses = (crecordmod.uihunk, patch.recordhunk)
+    ishunk = lambda x: isinstance(x, hunkclasses)
 
     if not ui.interactive():
         raise util.Abort(_('running non-interactively, use %s instead') %
@@ -102,6 +104,14 @@ 
         except patch.PatchError, err:
             raise util.Abort(_('error parsing patch: %s') % err)
 
+        # 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 = set()
+        for chunk in chunks:
+            if ishunk(chunk) and chunk.header.isnewfile() and chunk not in \
+                originalchunks:
+                newlyaddedandmodifiedfiles.add(chunk.header.filename())
         contenders = set()
         for h in chunks:
             try:
@@ -122,8 +132,8 @@ 
         if backupall:
             tobackup = changed
         else:
-            tobackup = [f for f in newfiles if f in modified]
-
+            tobackup = [f for f in newfiles if f in modified or f in \
+                    newlyaddedandmodifiedfiles]
         backups = {}
         if tobackup:
             backupdir = repo.join('record-backups')
@@ -151,6 +161,7 @@ 
             dopatch = fp.tell()
             fp.seek(0)
 
+            [os.unlink(c) for c in newlyaddedandmodifiedfiles]
             # 3a. apply filtered patch to clean repo  (clean)
             if backups:
                 # Equivalent to hg.revert
diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -820,9 +820,10 @@ 
     """
     diffgit_re = re.compile('diff --git a/(.*) b/(.*)$')
     diff_re = re.compile('diff -r .* (.*)$')
-    allhunks_re = re.compile('(?:index|new file|deleted file) ')
+    allhunks_re = re.compile('(?:index|deleted file) ')
     pretty_re = re.compile('(?:new file|deleted file) ')
-    special_re = re.compile('(?:index|new|deleted|copy|rename) ')
+    special_re = re.compile('(?:index|deleted|copy|rename) ')
+    newfile_re = re.compile('(?:new file)')
 
     def __init__(self, header):
         self.header = header
@@ -870,8 +871,21 @@ 
     def __repr__(self):
         return '<header %s>' % (' '.join(map(repr, self.files())))
 
+    def isnewfile(self):
+        return util.any(self.newfile_re.match(h) for h in self.header)
+
     def special(self):
-        return util.any(self.special_re.match(h) for h in self.header)
+        # Special files are shown only at the header level and not at the hunk
+        # level for example a file that has been deleted is a special file.
+        # The user cannot change the content of the operation, in the case of
+        # the deleted file he has to take the deletion or not take it, he
+        # cannot take some of it.
+        # Newly added files are special if they are empty, they are not special
+        # if they have some content as we want to be able to change it
+        nocontent = len(self.header) == 2
+        emptynewfile = self.isnewfile() and nocontent
+        return emptynewfile or \
+                util.any(self.special_re.match(h) for h in self.header)
 
 class recordhunk(object):
     """patch hunk
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
@@ -229,11 +229,25 @@ 
   $ hg add plain
   $ hg commit -i -d '7 0' -m plain plain<<EOF
   > y
+  > y
   > EOF
   diff --git a/plain b/plain
   new file mode 100644
   examine changes to 'plain'? [Ynesfdaq?] y
   
+  @@ -0,0 +1,10 @@
+  +1
+  +2
+  +3
+  +4
+  +5
+  +6
+  +7
+  +8
+  +9
+  +10
+  record this change to 'plain'? [Ynesfdaq?] y
+  
   $ hg tip -p
   changeset:   7:11fb457c1be4
   tag:         tip
@@ -315,6 +329,7 @@ 
   > y
   > y
   > y
+  > y
   > EOF
   diff --git a/plain b/plain
   1 hunks, 1 lines changed
@@ -333,6 +348,10 @@ 
   new file mode 100644
   examine changes to 'plain2'? [Ynesfdaq?] y
   
+  @@ -0,0 +1,1 @@
+  +1
+  record change 2/2 to 'plain2'? [Ynesfdaq?] y
+  
 Modify beginning, trim end, record both, add another file to test
 changes numbering
 
@@ -1406,4 +1425,41 @@ 
   date:        Thu Jan 01 00:00:23 1970 +0000
   summary:     moving_files
   
+Editing patch of newly added file
+
+  $ hg update -C .
+  0 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"
+  > __EOF__
+  $ cat > newfile << '__EOF__'
+  > This is the first line
+  > This is the second line
+  > This is the third line
+  > __EOF__
+  $ hg add newfile
+  $ HGEDITOR="\"sh\" \"`pwd`/editor.sh\"" hg commit -i -d '23 0' -medit-patch-new <<EOF
+  > y
+  > e
+  > EOF
+  diff --git a/newfile b/newfile
+  new file mode 100644
+  examine changes to 'newfile'? [Ynesfdaq?] y
+  
+  @@ -0,0 +1,3 @@
+  +This is the first line
+  +This is the second line
+  +This is the third line
+  record this change to 'newfile'? [Ynesfdaq?] e
+  
+  $ hg cat -r tip newfile
+  This is the very line
+  This is the second line
+  This is the third line
+
+  $ cat newfile
+  This is the first line
+  This is the second line
+  This is the third line
   $ cd ..
diff --git a/tests/test-keyword.t b/tests/test-keyword.t
--- a/tests/test-keyword.t
+++ b/tests/test-keyword.t
@@ -479,6 +479,12 @@ 
   new file mode 100644
   examine changes to 'r'? [Ynesfdaq?] y
   
+  @@ -0,0 +1,1 @@
+  +$Id$
+  record this change to 'r'? [Ynesfdaq?] y
+  
+  resolving manifests
+  patching file r
   committing files:
   r
   committing manifest
@@ -507,6 +513,12 @@ 
   new file mode 100644
   examine changes to 'i'? [Ynesfdaq?] y
   
+  @@ -0,0 +1,1 @@
+  +$Id$
+  record this change to 'i'? [Ynesfdaq?] y
+  
+  resolving manifests
+  patching file i
   committing files:
   i
   committing manifest
diff --git a/tests/test-mq-subrepo.t b/tests/test-mq-subrepo.t
--- a/tests/test-mq-subrepo.t
+++ b/tests/test-mq-subrepo.t
@@ -295,6 +295,11 @@ 
   new file mode 100644
   examine changes to '.hgsub'? [Ynesfdaq?] y
   
+  @@ -0,0 +1,1 @@
+  +sub = sub
+  record this change to '.hgsub'? [Ynesfdaq?] y
+  
+  warning: subrepo spec file '.hgsub' not found
   abort: uncommitted changes in subrepository 'sub'
   [255]
   % update substate when adding .hgsub w/clean updated subrepo
@@ -304,6 +309,11 @@ 
   new file mode 100644
   examine changes to '.hgsub'? [Ynesfdaq?] y
   
+  @@ -0,0 +1,1 @@
+  +sub = sub
+  record this change to '.hgsub'? [Ynesfdaq?] y
+  
+  warning: subrepo spec file '.hgsub' not found
   path sub
    source   sub
    revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31