Patchwork [2,of,2] subrepo: calculate _relpath for hgsubrepo based on self instead of parent

login
register
mail settings
Submitter Matt Harbison
Date April 15, 2015, 6:26 p.m.
Message ID <5b69b0a606afbc71bc83.1429122380@waste.org>
Download mbox | patch
Permalink /patch/8683/
State Accepted
Headers show

Comments

Matt Harbison - April 15, 2015, 6:26 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1429112984 14400
#      Wed Apr 15 11:49:44 2015 -0400
# Node ID 5b69b0a606afbc71bc83cbba8e956f52585b8f38
# Parent  7f70e8f8239f813f5041210ff6395fae53e01971
subrepo: calculate _relpath for hgsubrepo based on self instead of parent

Prior to 105758d1b37b, the subrelpath() (now _relpath) for hgsubrepo was
calculated by removing the root path of the outermost repo from the root path of
the subrepo.  Since the root paths use platform specific separators, and the
relative path is printed by various commands, the output of these commands
require a glob (and check-code.py enforces this).

In an effort to be generic to all subrepos, 105758d1b37b started calculating
this path based on the parent repo, and then joining the subrepo path in .hgsub.
One of the tests in test-subrepo.t creates a subrepo inside a directory, so the
path being joined contained '/' instead of '\'.  This made the test fail with a
'~' status, because the glob is unnecessary[1].  Removing them made the test
work, but then check-code complains.  We can't just drop the check-code rule,
because sub-subrepos are still joined with '\'.  Presumably the other subrepo
types have this issue as well, but there likely isn't a test with git or svn
repos inside a subdirectory.

This simply restores the exact _relpath value (and output) for hgsubrepos prior
to 105758d1b37b.


[1] http://www.selenic.com/pipermail/mercurial-devel/2015-April/068720.html
Katsunori FUJIWARA - April 16, 2015, 4:49 p.m.
At Wed, 15 Apr 2015 13:26:20 -0500,
Matt Harbison wrote:
> 
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1429112984 14400
> #      Wed Apr 15 11:49:44 2015 -0400
> # Node ID 5b69b0a606afbc71bc83cbba8e956f52585b8f38
> # Parent  7f70e8f8239f813f5041210ff6395fae53e01971
> subrepo: calculate _relpath for hgsubrepo based on self instead of parent
> 
> Prior to 105758d1b37b, the subrelpath() (now _relpath) for hgsubrepo was
> calculated by removing the root path of the outermost repo from the root path of
> the subrepo.  Since the root paths use platform specific separators, and the
> relative path is printed by various commands, the output of these commands
> require a glob (and check-code.py enforces this).
> 
> In an effort to be generic to all subrepos, 105758d1b37b started calculating
> this path based on the parent repo, and then joining the subrepo path in .hgsub.
> One of the tests in test-subrepo.t creates a subrepo inside a directory, so the
> path being joined contained '/' instead of '\'.  This made the test fail with a
> '~' status, because the glob is unnecessary[1].  Removing them made the test
> work, but then check-code complains.  We can't just drop the check-code rule,
> because sub-subrepos are still joined with '\'.  Presumably the other subrepo
> types have this issue as well, but there likely isn't a test with git or svn
> repos inside a subdirectory.
> 
> This simply restores the exact _relpath value (and output) for hgsubrepos prior
> to 105758d1b37b.

Oops, I forgot about os.sep portability while working around
"reporelpath()". Thank you for catching !

BTW, we still have to work for eliminating inconsistent usages of path
separators in subrepo.py :-)

  - gitsubrepo and svnsubrepo in a subdirectory cause mixing path
    separators, as you described above

  - some warn/status/debug outputs use "self._path" diretly

  - svn command may be invoked with absolute path using both separators

        if filename is not None:
            path = self.wvfs.reljoin(self._ctx.repo().origroot,
                                     self._path, filename)

> 
> [1] http://www.selenic.com/pipermail/mercurial-devel/2015-April/068720.html
> 
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -959,6 +959,13 @@
>          """
>          return self._repo.wvfs
>  
> +    @propertycache
> +    def _relpath(self):
> +        """return path to this subrepository as seen from outermost repository
> +        """
> +        # Keep consistent dir separators by avoiding vfs.join(self._path)
> +        return reporelpath(self._repo)
> +
>  class svnsubrepo(abstractsubrepo):
>      def __init__(self, ctx, path, state):
>          super(svnsubrepo, self).__init__(ctx, path)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Matt Mackall - April 17, 2015, 12:07 a.m.
On Wed, 2015-04-15 at 13:26 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1429112984 14400
> #      Wed Apr 15 11:49:44 2015 -0400
> # Node ID 5b69b0a606afbc71bc83cbba8e956f52585b8f38
> # Parent  7f70e8f8239f813f5041210ff6395fae53e01971
> subrepo: calculate _relpath for hgsubrepo based on self instead of parent

These are queued for default, thanks.

Patch

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -959,6 +959,13 @@ 
         """
         return self._repo.wvfs
 
+    @propertycache
+    def _relpath(self):
+        """return path to this subrepository as seen from outermost repository
+        """
+        # Keep consistent dir separators by avoiding vfs.join(self._path)
+        return reporelpath(self._repo)
+
 class svnsubrepo(abstractsubrepo):
     def __init__(self, ctx, path, state):
         super(svnsubrepo, self).__init__(ctx, path)