git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Heiko Voigt <hvoigt@hvoigt.net>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Jens Lehmann <Jens.Lehmann@web.de>,
	Brandon Williams <bmwill@google.com>
Subject: Re: [RFC PATCH v3 0/4] implement fetching of moved submodules
Date: Fri, 6 Oct 2017 15:57:19 -0700	[thread overview]
Message-ID: <CAGZ79kZofg3jS+g0weTdco+PGo_p-_Hd-NScZ=q2UfB7tF2GPA@mail.gmail.com> (raw)
In-Reply-To: <20171006222544.GA26642@sandbox>

On Fri, Oct 6, 2017 at 3:25 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> 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. :)

> 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.

> 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.

> 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

  parent reply	other threads:[~2017-10-06 22:57 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-06 22:25 [RFC PATCH v3 0/4] implement fetching of moved submodules Heiko Voigt
2017-10-06 22:30 ` [RFC PATCH 1/4] fetch: add test to make sure we stay backwards compatible Heiko Voigt
2017-10-06 22:32 ` [RFC PATCH 2/4] change submodule push test to use proper repository setup Heiko Voigt
2017-10-09 18:20   ` Stefan Beller
2017-10-10 13:03     ` Heiko Voigt
2017-10-10 18:39       ` Stefan Beller
2017-10-10 23:31         ` Junio C Hamano
2017-10-10 23:41           ` Stefan Beller
2017-10-11  0:19           ` Junio C Hamano
2017-10-11 14:56           ` Heiko Voigt
2017-10-12  0:26             ` Junio C Hamano
2017-10-11 14:52         ` Heiko Voigt
2017-10-11 15:10         ` Josh Triplett
2017-10-12 16:17           ` Brandon Williams
2017-10-06 22:34 ` [RFC PATCH v3 3/4] implement fetching of moved submodules Heiko Voigt
2017-10-06 22:35 ` [RFC PATCH v3 4/4] submodule: simplify decision tree whether to or not to fetch Heiko Voigt
2017-10-06 22:57 ` Stefan Beller [this message]
2017-10-07  1:24 ` [RFC PATCH v3 0/4] implement fetching of moved submodules Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAGZ79kZofg3jS+g0weTdco+PGo_p-_Hd-NScZ=q2UfB7tF2GPA@mail.gmail.com' \
    --to=sbeller@google.com \
    --cc=Jens.Lehmann@web.de \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=hvoigt@hvoigt.net \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).