Patchwork [STABLE,RESEND] convert: detect removal of ".gitmodules" at git source revisions correctly

login
register
mail settings
Submitter Katsunori FUJIWARA
Date July 14, 2014, 3:16 p.m.
Message ID <805d4bd3bc8ec0940a8f.1405350984@feefifofum>
Download mbox | patch
Permalink /patch/5160/
State Accepted
Commit 3420346174b1cff51e6af2cffaa2756c68882dd8
Headers show

Comments

Katsunori FUJIWARA - July 14, 2014, 3:16 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1405348439 -32400
#      Mon Jul 14 23:33:59 2014 +0900
# Branch stable
# Node ID 805d4bd3bc8ec0940a8feda7f4e5b80ade884729
# Parent  92666a869ea4e39d31b836831ef9f7abdf72bc1a
convert: detect removal of ".gitmodules" at git source revisions correctly

Before this patch, all operations applied on ".gitmodules" at git
source revisions are treated as modification, even if they are
actually removal of it.

If removal of ".gitmodules" is treated as modification unexpectedly,
"hg convert" is aborted by the exception raised in
"retrievegitmodules()" for ".gitmodules" at the git source revision
removing it, because that revision doesn't have any information of
".gitmodules".

This patch detects removal of ".gitmodules" at git source revisions
correctly.

If ".gitmodules" is removed at the git source revision, this patch
records "hex(nullid)" as the contents hash value for ".hgsub" and
".hgsubstate" at the destination revision.

This patch makes "getfile()" raise IOError also for ".hgstatus" and
".hgsubstate" if the contents hash value is "hex(nullid)", and this
tells removal of ".hgstatus" and ".hgsubstate" at the destination
revision to "localrepository.commitctx()" correctly.

For files other than ".hgstatus" and ".hgsubstate", checking the
contents hash value in "getfile()" may be redundant, because
"catfile()" for them also does so.

But this patch chooses writing it only once at the beginning of
"getfile()", to avoid writing same code twice both for ".hgsub" and
".hgsubstate" separately.
Katsunori FUJIWARA - July 14, 2014, 3:23 p.m.
At Tue, 15 Jul 2014 00:16:24 +0900,
FUJIWARA Katsunori wrote:
> 
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1405348439 -32400
> #      Mon Jul 14 23:33:59 2014 +0900
> # Branch stable
> # Node ID 805d4bd3bc8ec0940a8feda7f4e5b80ade884729
> # Parent  92666a869ea4e39d31b836831ef9f7abdf72bc1a
> convert: detect removal of ".gitmodules" at git source revisions correctly

This is a just resend of one posted previously.

This series was once queued, but finally rejected, because of breaking
test at merging into default:

  http://selenic.com/pipermail/mercurial-devel/2014-July/059966.html
  http://selenic.com/pipermail/mercurial-devel/2014-July/059967.html

Root cause of problem above was resolved by the change below by Sean
Farley:

  http://selenic.com/repo/hg/rev/7cfd94ec5d30

Now, this series can be merged into default safely.


> Before this patch, all operations applied on ".gitmodules" at git
> source revisions are treated as modification, even if they are
> actually removal of it.
> 
> If removal of ".gitmodules" is treated as modification unexpectedly,
> "hg convert" is aborted by the exception raised in
> "retrievegitmodules()" for ".gitmodules" at the git source revision
> removing it, because that revision doesn't have any information of
> ".gitmodules".
> 
> This patch detects removal of ".gitmodules" at git source revisions
> correctly.
> 
> If ".gitmodules" is removed at the git source revision, this patch
> records "hex(nullid)" as the contents hash value for ".hgsub" and
> ".hgsubstate" at the destination revision.
> 
> This patch makes "getfile()" raise IOError also for ".hgstatus" and
> ".hgsubstate" if the contents hash value is "hex(nullid)", and this
> tells removal of ".hgstatus" and ".hgsubstate" at the destination
> revision to "localrepository.commitctx()" correctly.
> 
> For files other than ".hgstatus" and ".hgsubstate", checking the
> contents hash value in "getfile()" may be redundant, because
> "catfile()" for them also does so.
> 
> But this patch chooses writing it only once at the beginning of
> "getfile()", to avoid writing same code twice both for ".hgsub" and
> ".hgsubstate" separately.
> 
> diff --git a/hgext/convert/git.py b/hgext/convert/git.py
> --- a/hgext/convert/git.py
> +++ b/hgext/convert/git.py
> @@ -104,6 +104,8 @@
>          return data
>  
>      def getfile(self, name, rev):
> +        if rev == hex(nullid):
> +            raise IOError
>          if name == '.hgsub':
>              data = '\n'.join([m.hgsub() for m in self.submoditer()])
>              mode = ''
> @@ -155,6 +157,7 @@
>          seen = set()
>          entry = None
>          subexists = False
> +        subdeleted = False
>          for l in fh.read().split('\x00'):
>              if not entry:
>                  if not l.startswith(':'):
> @@ -171,7 +174,11 @@
>  
>                  if f == '.gitmodules':
>                      subexists = True
> -                    changes.append(('.hgsub', ''))
> +                    if entry[4] == 'D':
> +                        subdeleted = True
> +                        changes.append(('.hgsub', hex(nullid)))
> +                    else:
> +                        changes.append(('.hgsub', ''))
>                  elif entry[1] == '160000' or entry[0] == ':160000':
>                      subexists = True
>                  else:
> @@ -182,8 +189,11 @@
>              raise util.Abort(_('cannot read changes in %s') % version)
>  
>          if subexists:
> -            self.retrievegitmodules(version)
> -            changes.append(('.hgsubstate', ''))
> +            if subdeleted:
> +                changes.append(('.hgsubstate', hex(nullid)))
> +            else:
> +                self.retrievegitmodules(version)
> +                changes.append(('.hgsubstate', ''))
>          return (changes, {})
>  
>      def getcommit(self, version):
> diff --git a/tests/test-convert-git.t b/tests/test-convert-git.t
> --- a/tests/test-convert-git.t
> +++ b/tests/test-convert-git.t
> @@ -363,6 +363,23 @@
>  
>    $ cd ../..
>  
> +convert the revision removing '.gitmodules' itself (and related
> +submodules)
> +
> +  $ cd git-repo6
> +  $ git rm .gitmodules
> +  rm '.gitmodules'
> +  $ git rm --cached git-repo5
> +  rm 'git-repo5'
> +  $ commit -a -m 'remove .gitmodules and submodule git-repo5'
> +  $ cd ..
> +
> +  $ hg convert -q git-repo6 git-repo6-hg
> +  $ hg -R git-repo6-hg tip -T "{desc|firstline}\n"
> +  remove .gitmodules and submodule git-repo5
> +  $ hg -R git-repo6-hg tip -T "{file_dels}\n"
> +  .hgsub .hgsubstate
> +
>  damaged git repository tests:
>  In case the hard-coded hashes change, the following commands can be used to
>  list the hashes and their corresponding types in the repository:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Matt Mackall - July 14, 2014, 10:33 p.m.
On Tue, 2014-07-15 at 00:16 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1405348439 -32400
> #      Mon Jul 14 23:33:59 2014 +0900
> # Branch stable
> # Node ID 805d4bd3bc8ec0940a8feda7f4e5b80ade884729
> # Parent  92666a869ea4e39d31b836831ef9f7abdf72bc1a
> convert: detect removal of ".gitmodules" at git source revisions correctly

Queued for stable, thanks.

Patch

diff --git a/hgext/convert/git.py b/hgext/convert/git.py
--- a/hgext/convert/git.py
+++ b/hgext/convert/git.py
@@ -104,6 +104,8 @@ 
         return data
 
     def getfile(self, name, rev):
+        if rev == hex(nullid):
+            raise IOError
         if name == '.hgsub':
             data = '\n'.join([m.hgsub() for m in self.submoditer()])
             mode = ''
@@ -155,6 +157,7 @@ 
         seen = set()
         entry = None
         subexists = False
+        subdeleted = False
         for l in fh.read().split('\x00'):
             if not entry:
                 if not l.startswith(':'):
@@ -171,7 +174,11 @@ 
 
                 if f == '.gitmodules':
                     subexists = True
-                    changes.append(('.hgsub', ''))
+                    if entry[4] == 'D':
+                        subdeleted = True
+                        changes.append(('.hgsub', hex(nullid)))
+                    else:
+                        changes.append(('.hgsub', ''))
                 elif entry[1] == '160000' or entry[0] == ':160000':
                     subexists = True
                 else:
@@ -182,8 +189,11 @@ 
             raise util.Abort(_('cannot read changes in %s') % version)
 
         if subexists:
-            self.retrievegitmodules(version)
-            changes.append(('.hgsubstate', ''))
+            if subdeleted:
+                changes.append(('.hgsubstate', hex(nullid)))
+            else:
+                self.retrievegitmodules(version)
+                changes.append(('.hgsubstate', ''))
         return (changes, {})
 
     def getcommit(self, version):
diff --git a/tests/test-convert-git.t b/tests/test-convert-git.t
--- a/tests/test-convert-git.t
+++ b/tests/test-convert-git.t
@@ -363,6 +363,23 @@ 
 
   $ cd ../..
 
+convert the revision removing '.gitmodules' itself (and related
+submodules)
+
+  $ cd git-repo6
+  $ git rm .gitmodules
+  rm '.gitmodules'
+  $ git rm --cached git-repo5
+  rm 'git-repo5'
+  $ commit -a -m 'remove .gitmodules and submodule git-repo5'
+  $ cd ..
+
+  $ hg convert -q git-repo6 git-repo6-hg
+  $ hg -R git-repo6-hg tip -T "{desc|firstline}\n"
+  remove .gitmodules and submodule git-repo5
+  $ hg -R git-repo6-hg tip -T "{file_dels}\n"
+  .hgsub .hgsubstate
+
 damaged git repository tests:
 In case the hard-coded hashes change, the following commands can be used to
 list the hashes and their corresponding types in the repository: