Re: [PATCH 2/2] submodule: munge paths to submodule git directories

From: Stefan Beller <sbeller@google.com>
Date: Aug 28, 2018, 21:35 UTC
Message-ID: <CAGZ79kaLXcTeeM9AKvXi7X8WMd+vcyCM5n-Nz2igHkGJdXbSfg@mail.gmail.com>
In-Reply-To: <20180814180406.GA86804@google.com>

Show 14 quoted lines

> > > -           echo "gitdir: ../../../.git/modules/sub3/modules/dirdir/subsub" >./sub3/dirdir/subsub/.git_expect
> > > +           echo "gitdir: ../../../.git/modules/sub3/modules/dirdir%2fsubsub" >./sub3/dirdir/subsub/.git_expect
> >
> > One interesting thing about url-encoding is that it's not one-to-one.
> > This case could also be %2F, which is a different file (on a
> > case-sensitive filesystem). I think "%20" and "+" are similarly
> > interchangeable.
> >
> > If we were decoding the filenames, that's fine. The round-trip is
> > lossless.
> >
> > But that's not quite how the new code behaves. We encode the input and
> > then check to see if it matches an encoding we previously performed. So
> > if our urlencode routines ever change, this will subtly break.

And this is the problem:
a) we have a 'complicated' encoding here, which must never change
b) the "encode and check if it matches", will produce ugly code going forward,
    as it tries to differentiate between submodules named "url_encoded(a)"
    and "a" (e.g. "a%20b" and "a b" would conflict and we have to resolve
    the conflict, although those two names are perfectly fine as they do not
    have the original problem of having slashes)

Hence I would propose a simpler encoding:

1)    / -> _ ( replace a slash by an underscore)
2)    _ -> __ (replace any underscore by 2 underscores, this is just the
          escaping mechanism to differentiate a/b and a_b)

3) (optional) instead of putting it all in modules/, use another
directory gitmodules/
    for example. this will make sure we can tell if a repository has
been converted
    or is stuck with a setup of a current git.

Show 6 quoted lines

> This is exactly the reason why I wanted to get some opinions on what the
> best thing to do here would be.  I _think_ the best thing would probably
> be to write a specific routine to do the conversion, and it wouldn't
> even have to be all that complex.  Basically I'm just interested in
> converting '/' characters so that things no longer behave like
> nested directories.

Yeah, then let's just convert '/' with as little overhead as possible.

Thanks, Stefan