Re: [RFC PATCH v3 0/4] implement fetching of moved submodules

From: Stefan Beller <sbeller@google.com>
Date: Oct 6, 2017, 22:57 UTC
Message-ID: <CAGZ79kZofg3jS+g0weTdco+PGo_p-_Hd-NScZ=q2UfB7tF2GPA@mail.gmail.com>
In-Reply-To: <20171006222544.GA26642@sandbox>

On Fri, Oct 6, 2017 at 3:25 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:

Show 6 quoted lines

> The last iteration can be found here:
>
> https://public-inbox.org/git/20170817105349.GC52233@book.hvoigt.net/
>
> This is mainly a status update and to let people know that I am still
> working on this.

Cool. :)

Show 5 quoted lines

> I struggled quite a bit with reviving my original test for the path
> based recursive fetch (first patch). The behavior seems to haved changed
> and simply setting the submodule configuration in .git/config without
> one in .gitmodules does not work anymore. I did not have time to
> investigate whether this was a deliberate change or a maybe a bug?

I think it is deliberate. (We wrote this man page "gitsubmodules"[1] and there
was so much discussion on "What is a submodule?". Key take away is this:
* a gitlink alone is not a submodule.
* a submodule consists of at least
  -> the gitlink in the superproject
  -> a mapping of path -> name via
      $(git config -f .gitmodules submodule.<name>.path)
  -> Depending on config in .git/config or the existence of its git directory,
      it may be [in]active or [de]initialized.

[1] not to be confused with "gitmodules" or "git-submodule"

Sometimes we accept a plain git-link without the config in .gitmodules, (a) due to historic reasons or (b) because it seems sane even for a repo "that just happens to exist at the gitlinks location" (example git-diff)

> So the solution for now is that I write my fake configuration (to avoid
> skipping submodule handling altogether) into a .gitmodules file.

I'll try to spot what is fake about the config.

Show 10 quoted lines

> The second patch (cleanup of a submodule push testcase) was written
> because that currently is the only test failing. It is not meant for
> inclusion but rather as a demonstration of what might be happening when
> we cleanup testcases: Because of the behavioral change above, on first
> sight, it seemed like there was a shortcut in fetch and so on-demand
> fetch without submodule configuration would not be supported anymore.
>
> IIRC there were a lot more tests failing before when I implemented my
> patch without the fallback on paths. So my guess is that some tests have
> been cleaned up to use proper (.gitmodules) submodule setup.

I don't remember any large recent activity for submodule things lately.

Show 5 quoted lines

> So the thing here is: If we want to make sure that we stay backwards
> compatible by supporting the setup with gitlinks without configuration.
> Then we also should keep tests around that have the plain manual setup
> without .gitmodules files. Just something, I think, we should keep in
> mind.

But do we want this?

Without the name<->path mapping, we can only have the "old style" submodules, that have their git repo inside its tree instead of inside the superprojects git dir. So renaming/moving "old style with no name<->path mapping" will not work. (That may be an acceptable trade off. But then again, just providing the mapping, such that the superproject can absorb the git directory of the submodule for this use case, doesn't seem like a big deal to me. Something you want to have anyway, for ease of use of the superproject w.r.t. cloning for example)

So while I do not try to deliberately break these old behaviors, I'd rather want us to go forward with a saner model than "if we happen to have enough data around, the operation succeeds", i.e. ignore anything that is not following the rather strict definition of a submodule.

FYI: Once upon a time I found "fake submodules"
http://debuggable.com/posts/git-fake-submodules:4b563ee4-f3cc-4061-967e-0e48cbdd56cb
Last time this was discussed on list, this was considered a bug not
worth fixing instead of a feature IIRC. (Personally I think this is
a rather cool hack, which we may want to abuse ourselves for
things like "convert a subtree into a submodule and back again",
but we could also go without this hack)

> Apart from the tests nothing has been added in this iteration. Since I
> finally have a working test now I will continue with reviving the
> fallback to paths.

I'll have a look.

Cheers, Stefan