Bug with "git submodule update" + subrepo with differing path/name?

Bug with "git submodule update" + subrepo with differing path/name?

[Andreas Urke]

Hi,

I have a repo with two submodules. I regularly use "git submodule
update", which works fine for one subrepo, but not for the other. The
only relevant difference I can think of between these two subrepos is
that the latter one does not have a matching path and name:

$ cat .gitmodules
[submodule "subrepo1"]
    path = subrepo1
    url = git@gitlab.com:subrepo1.git
[submodule "name_subrepo2"]
    path = subrepo2
    url = git@gitlab.com:name_subrepo2.git

And a reproduction looks like this:

$ git --version
git version 2.15.1

$ git status
HEAD detached at 05412d4
Changes not staged for commit:

modified:   subrepo1 (new commits)
modified:   subrepo2 (new commits)

$ git submodule update
Submodule path 'subrepo1': checked out
'e4785498130ec4188bb1745f27e311bff08ed4c8'

$ git status
HEAD detached at 05412d4
Changes not staged for commit:

modified:   subrepo2  (new commits)

Checking out the relevant commit manually in subrepo2 works fine (and
is what I end up doing).

Regards,
Andreas Urke

[PATCH] Re: Bug with "git submodule update" + subrepo with differing path/name?

[Stefan Beller]

I tried reproducing the issue (based on the `next` branch, not 2.15,
but I do not recall any changes in the submodule area lately), and
could not come up with a reproduction recipe, but here is what I got so
far, maybe you can take it from here (i.e. either make the test case
more like your environment such that it fails, or rather bisect git
to tell us the offending commit) ?

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 t/t7406-submodule-update.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 6f083c4d68..d957305c38 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -978,4 +978,20 @@ test_expect_success 'git clone passes the parallel jobs config on to submodules'
 	rm -rf super4
 '
 
+test_expect_success 'git submodule update works with submodules with name/path difference' '
+	test_create_repo super6 &&
+	(
+		cd super6 &&
+		git submodule add ../submodule sub1 &&
+		git submodule add --name testingname ../submodule sub2 &&
+		git commit -m initial &&
+		git -C sub1 checkout HEAD^ &&
+		git -C sub2 checkout HEAD^ &&
+
+		git submodule update >actual &&
+		grep sub1 actual &&
+		grep sub2 actual
+	)
+'
+
 test_done
-- 
2.15.1.620.gb9897f4670-goog

Re: [PATCH] Re: Bug with "git submodule update" + subrepo with differing path/name?

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> I tried reproducing the issue (based on the `next` branch, not 2.15,
> but I do not recall any changes in the submodule area lately), and
> could not come up with a reproduction recipe,...

I do not offhand recall anything; the closest I can think of is the
three-patch series <20171016135623.GA12756@book.hvoigt.net> that
taught the on-demand recursive fetch to pay attention to the location
in the superproject tree a submodule is bound to.

    4b4acedd61 submodule: simplify decision tree whether to or not to fetch
    c68f837576 implement fetching of moved submodules
    01ce12252c fetch: add test to make sure we stay backwards compatible

But IIRC, "submodule update" uses a separate codepath?

Re: [PATCH] Re: Bug with "git submodule update" + subrepo with differing path/name?

[Stefan Beller]

On Tue, Dec 19, 2017 at 2:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> I tried reproducing the issue (based on the `next` branch, not 2.15,
>> but I do not recall any changes in the submodule area lately), and
>> could not come up with a reproduction recipe,...
>
> I do not offhand recall anything; the closest I can think of is the
> three-patch series <20171016135623.GA12756@book.hvoigt.net> that
> taught the on-demand recursive fetch to pay attention to the location
> in the superproject tree a submodule is bound to.

I tried the same test on 2.15 and cannot reproduce there either.

>
>     4b4acedd61 submodule: simplify decision tree whether to or not to fetch
>     c68f837576 implement fetching of moved submodules
>     01ce12252c fetch: add test to make sure we stay backwards compatible
>
> But IIRC, "submodule update" uses a separate codepath?

Yes, any portion of git-submodule.sh that calls out to C is going
through the submodule--helper. I want to revive the port of that
shell script to C again.

The "submodule update" uses the submodule helper to obtain
the list of submodules and then does a "git -C $sub fetch" in there.

Stefan

Re: [PATCH] Re: Bug with "git submodule update" + subrepo with differing path/name?

[Andreas Urke]

Thanks for looking into this.

I was able to reproduce it from scratch, but I followed my earlier
workflow where I first created the subrepos, and then later renamed
it. At the time I was not able to find any command to rename without
changing the path (and I was not able find one now either, is there
any?), so I edited name and path in .gitmodules and ran git submodule
sync. Am I asking for trouble doing it that way?

Let me know if you need the exact steps I followed.

Andreas


On 19 December 2017 at 23:33, Stefan Beller <sbeller@google.com> wrote:
> On Tue, Dec 19, 2017 at 2:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> I tried reproducing the issue (based on the `next` branch, not 2.15,
>>> but I do not recall any changes in the submodule area lately), and
>>> could not come up with a reproduction recipe,...
>>
>> I do not offhand recall anything; the closest I can think of is the
>> three-patch series <20171016135623.GA12756@book.hvoigt.net> that
>> taught the on-demand recursive fetch to pay attention to the location
>> in the superproject tree a submodule is bound to.
>
> I tried the same test on 2.15 and cannot reproduce there either.
>
>>
>>     4b4acedd61 submodule: simplify decision tree whether to or not to fetch
>>     c68f837576 implement fetching of moved submodules
>>     01ce12252c fetch: add test to make sure we stay backwards compatible
>>
>> But IIRC, "submodule update" uses a separate codepath?
>
> Yes, any portion of git-submodule.sh that calls out to C is going
> through the submodule--helper. I want to revive the port of that
> shell script to C again.
>
> The "submodule update" uses the submodule helper to obtain
> the list of submodules and then does a "git -C $sub fetch" in there.
>
> Stefan

Re: [PATCH] Re: Bug with "git submodule update" + subrepo with differing path/name?

[Stefan Beller]

On Wed, Dec 20, 2017 at 12:22 AM, Andreas Urke <arurke@gmail.com> wrote:
> Thanks for looking into this.
>
> I was able to reproduce it from scratch, but I followed my earlier
> workflow where I first created the subrepos, and then later renamed
> it. At the time I was not able to find any command to rename without
> changing the path (and I was not able find one now either, is there
> any?), so I edited name and path in .gitmodules and ran git submodule
> sync. Am I asking for trouble doing it that way?

"rename without changing the path" sounds like a red flag to me,
as the submodule name was introduced specifically to be a constant
as the path may change, whereas the name ought to never change.




>
> Let me know if you need the exact steps I followed.

Well yes, ideally as a shell script (or even embedded into our test suite).

Re: [PATCH] Re: Bug with "git submodule update" + subrepo with differing path/name?

[Andreas Urke]

If I am entering into undefined behavior territory with the renaming
then there might not be any point to pursue this further, but in any
case, script below:

Apologies up-front for the verbosity, there are probably a lot of
unnecessary steps and git shortcuts missed - I just wanted it to
exactly match my scenario. Note that it is divided into two parts as
it requires you to edit .gitmodules.

Part 1:

cd ~/
# Make sub repos and add a commit to each
mkdir super && cd super
mkdir sub1 && mkdir sub2
cd sub1
git init && touch first && git add . && git commit -m "first"
cd ../sub2
git init && touch first && git add . && git commit -m "first"
cd ..

# Make super repo, add subrepos, and commit
git init
git submodule add ./sub1
git submodule add ./sub2
git add .
git commit -m "first"

# Edit .gitmodules, change sub2 name and path to sub2_newname:
# $ cat .gitmodules
# [submodule "sub1"]
# path = sub1
# url = ./sub1
# [submodule "sub2_newname"]
# path = sub2
# url = ./sub2_newname

Part 2:

cd ~/
# Sync submodule after we edited .gitmodules
cd super
git submodule sync

# Commit in subrepos and commit this to superrepo
cd sub1
touch second && git add . && git commit -m "second"
cd ../sub2
touch second && git add . && git commit -m "second"
cd ..
git add .
git commit -m "second"

# Commit in subrepos and commit this to superrepo
cd sub1
touch third && git add . && git commit -m "third"
cd ../sub2
touch third && git add . && git commit -m "third"
cd ..
git add .
git commit -m "third"

git checkout HEAD^
git status
git submodule update
git status # Observe only sub1 repo was updated

On 20 December 2017 at 18:54, Stefan Beller <sbeller@google.com> wrote:
> On Wed, Dec 20, 2017 at 12:22 AM, Andreas Urke <arurke@gmail.com> wrote:
>> Thanks for looking into this.
>>
>> I was able to reproduce it from scratch, but I followed my earlier
>> workflow where I first created the subrepos, and then later renamed
>> it. At the time I was not able to find any command to rename without
>> changing the path (and I was not able find one now either, is there
>> any?), so I edited name and path in .gitmodules and ran git submodule
>> sync. Am I asking for trouble doing it that way?
>
> "rename without changing the path" sounds like a red flag to me,
> as the submodule name was introduced specifically to be a constant
> as the path may change, whereas the name ought to never change.
>
>
>
>
>>
>> Let me know if you need the exact steps I followed.
>
> Well yes, ideally as a shell script (or even embedded into our test suite).

Re: [PATCH] Re: Bug with "git submodule update" + subrepo with differing path/name?

[Stefan Beller]

On Thu, Dec 21, 2017 at 8:21 AM, Andreas Urke <arurke@gmail.com> wrote:
> If I am entering into undefined behavior territory with the renaming
> then there might not be any point to pursue this further, but in any
> case, script below:
>
> Apologies up-front for the verbosity, there are probably a lot of
> unnecessary steps and git shortcuts missed - I just wanted it to
> exactly match my scenario. Note that it is divided into two parts as
> it requires you to edit .gitmodules.
>
> Part 1:
>
> cd ~/
> # Make sub repos and add a commit to each
> mkdir super && cd super
> mkdir sub1 && mkdir sub2
> cd sub1
> git init && touch first && git add . && git commit -m "first"
> cd ../sub2
> git init && touch first && git add . && git commit -m "first"
> cd ..
>
> # Make super repo, add subrepos, and commit
> git init
> git submodule add ./sub1
> git submodule add ./sub2
> git add .
> git commit -m "first"
>
> # Edit .gitmodules, change sub2 name and path to sub2_newname:
> # $ cat .gitmodules
> # [submodule "sub1"]
> # path = sub1
> # url = ./sub1
> # [submodule "sub2_newname"]
> # path = sub2
> # url = ./sub2_newname
>

A couple of things here:
(a) In a script you could do this via
    git config -f .gitmodules --rename-section <old> <new>
(b) This is not just undefined, but rather Git explicitly assumes
    the user does not rename the section themselves.

The reason for this is found in gits history:
1. submodules were invented
2. submodule configuration such as where to obtain
    the submodule from needs some place, and at the time
    submodule.<path>.url inside the .gitmodules file seemed
    like a good idea.
3. "What if we want git-mv, git-rm support for submodules?"
    Or for example when bisecting and jumping between revisions
    where the submodule exists or does not exists. To solve
    this problem, the concept of a "submodule name" was invented,
    such that the submodules git dir can be put into the
    superprojects .git/modules/<name> dir, and the working tree
    only needs to have a ".git" file pointing at that gitdir. The the
    working tree of the submodule can be removed while keeping
    the submodules git dir (with potentially important information
    such as local-only commits) around! Success!

    The transition path is also easy:
    These newly "named" submodules have an additional entry of
    'submodule.<name>.path =<path>'. Note how the subsection
    part of the config changed its meaning, but that is ok, as the
    name is allowed to be equal to the <path> value, and keeps
    all config related to one submodule in one section. Easy but
    confusing as now we have to adhere to a couple assumptions:
    The <path> value must match where it is in the working tree,
    the <name> however must not be changed because that is
    where we'll find its repository inside the superproject.
    And confusingly these two keys have often the same value,
    for this historic reason.

4. (rather lately): We need to fix the submodule mess.
    While in (3) we were unclear about name/path issues,
    and tried to be super backwards compatible, now we'd
    rather want the submodules to be well-defined. So we'll
    try to write some docs, such as gitsubmodules [1] in addition
    to the usage manual of "git submodule"[2]. Of course there is
    still the description of the .gitmodules file[3].

[1] https://www.kernel.org/pub/software/scm/git/docs/gitsubmodules.html
[2] https://www.kernel.org/pub/software/scm/git/docs/git-submodule.html
[3] https://www.kernel.org/pub/software/scm/git/docs/gitmodules.html

I guess in [1] we'd want to add a discussion on what the <name> and
what the <path> is for and why one can change but not the other.

I think you ran into trouble here because you and Git had
differing assumptions on some part of the submodule model.
Is there any part of the documentation, configuration or some
commands that would have helped explaining some of these things?
(i.e. What do we best patch to help others from running into the
same issue?)

Thanks,
Stefan

Re: [PATCH] Re: Bug with "git submodule update" + subrepo with differing path/name?

[Andreas Urke]

Thanks for the detailed explanation. Although I am not too git savvy,
I believe I got the gist of it.

Regarding your question,
I would say the term "name" in an IT context makes me primarily think
of something that is specified by a user (as opposed to e.g. an "id"),
and can be altered by a user. Also, the manual mention it as a
"logical name", which would further strengthen my belief that it is an
abstraction which can be changed (as opposed to something "physical").
I would be much more reluctant to change the id of something than its
name.

In terms of the commands...In an ideal world I would obviously ask for
a --rename or mv command which would achieve what I wanted to do.
Other than that, maybe a word about this in the man for "git mv"? Or
perhaps "git submodule sync" could give me some kind of warning that I
did something strange/illegal.

Can I ask you, now that I have made this mess, do you have any
suggestion on how to rectify it or some other way to achieve my goal?
The only side-effect I have seen is this update problem (been running
this for a few months), but after your explanations I would like to
get back to solid ground. My use-case is that I need to follow a
specific folder-naming (i.e. subrepo path) convention, but I do not
want to use that naming as the repo name in our gitlab.

Regards,
Andreas

On 21 December 2017 at 19:55, Stefan Beller <sbeller@google.com> wrote:
> On Thu, Dec 21, 2017 at 8:21 AM, Andreas Urke <arurke@gmail.com> wrote:
>> If I am entering into undefined behavior territory with the renaming
>> then there might not be any point to pursue this further, but in any
>> case, script below:
>>
>> Apologies up-front for the verbosity, there are probably a lot of
>> unnecessary steps and git shortcuts missed - I just wanted it to
>> exactly match my scenario. Note that it is divided into two parts as
>> it requires you to edit .gitmodules.
>>
>> Part 1:
>>
>> cd ~/
>> # Make sub repos and add a commit to each
>> mkdir super && cd super
>> mkdir sub1 && mkdir sub2
>> cd sub1
>> git init && touch first && git add . && git commit -m "first"
>> cd ../sub2
>> git init && touch first && git add . && git commit -m "first"
>> cd ..
>>
>> # Make super repo, add subrepos, and commit
>> git init
>> git submodule add ./sub1
>> git submodule add ./sub2
>> git add .
>> git commit -m "first"
>>
>> # Edit .gitmodules, change sub2 name and path to sub2_newname:
>> # $ cat .gitmodules
>> # [submodule "sub1"]
>> # path = sub1
>> # url = ./sub1
>> # [submodule "sub2_newname"]
>> # path = sub2
>> # url = ./sub2_newname
>>
>
> A couple of things here:
> (a) In a script you could do this via
>     git config -f .gitmodules --rename-section <old> <new>
> (b) This is not just undefined, but rather Git explicitly assumes
>     the user does not rename the section themselves.
>
> The reason for this is found in gits history:
> 1. submodules were invented
> 2. submodule configuration such as where to obtain
>     the submodule from needs some place, and at the time
>     submodule.<path>.url inside the .gitmodules file seemed
>     like a good idea.
> 3. "What if we want git-mv, git-rm support for submodules?"
>     Or for example when bisecting and jumping between revisions
>     where the submodule exists or does not exists. To solve
>     this problem, the concept of a "submodule name" was invented,
>     such that the submodules git dir can be put into the
>     superprojects .git/modules/<name> dir, and the working tree
>     only needs to have a ".git" file pointing at that gitdir. The the
>     working tree of the submodule can be removed while keeping
>     the submodules git dir (with potentially important information
>     such as local-only commits) around! Success!
>
>     The transition path is also easy:
>     These newly "named" submodules have an additional entry of
>     'submodule.<name>.path =<path>'. Note how the subsection
>     part of the config changed its meaning, but that is ok, as the
>     name is allowed to be equal to the <path> value, and keeps
>     all config related to one submodule in one section. Easy but
>     confusing as now we have to adhere to a couple assumptions:
>     The <path> value must match where it is in the working tree,
>     the <name> however must not be changed because that is
>     where we'll find its repository inside the superproject.
>     And confusingly these two keys have often the same value,
>     for this historic reason.
>
> 4. (rather lately): We need to fix the submodule mess.
>     While in (3) we were unclear about name/path issues,
>     and tried to be super backwards compatible, now we'd
>     rather want the submodules to be well-defined. So we'll
>     try to write some docs, such as gitsubmodules [1] in addition
>     to the usage manual of "git submodule"[2]. Of course there is
>     still the description of the .gitmodules file[3].
>
> [1] https://www.kernel.org/pub/software/scm/git/docs/gitsubmodules.html
> [2] https://www.kernel.org/pub/software/scm/git/docs/git-submodule.html
> [3] https://www.kernel.org/pub/software/scm/git/docs/gitmodules.html
>
> I guess in [1] we'd want to add a discussion on what the <name> and
> what the <path> is for and why one can change but not the other.
>
> I think you ran into trouble here because you and Git had
> differing assumptions on some part of the submodule model.
> Is there any part of the documentation, configuration or some
> commands that would have helped explaining some of these things?
> (i.e. What do we best patch to help others from running into the
> same issue?)
>
> Thanks,
> Stefan

Re: [PATCH] Re: Bug with "git submodule update" + subrepo with differing path/name?

[Stefan Beller]

On Thu, Dec 21, 2017 at 2:17 PM, Andreas Urke <arurke@gmail.com> wrote:
> Thanks for the detailed explanation. Although I am not too git savvy,
> I believe I got the gist of it.
>
> Regarding your question,
> I would say the term "name" in an IT context makes me primarily think
> of something that is specified by a user (as opposed to e.g. an "id"),

ID sounds great.

> and can be altered by a user. Also, the manual mention it as a
> "logical name", which would further strengthen my belief that it is an
> abstraction which can be changed (as opposed to something "physical").
> I would be much more reluctant to change the id of something than its
> name.

If you changed the name in all places, it would have been no problem. ;)
So once you did the "git config -f .gitmodules --rename-section ...",
you'd also have to "mv .git/modules/<old-name> .git/modules/<new-name>"
and then it would work fine for you. (You would also need to instruct
any collaborators to run the move command when they pull your changes.
So it is more of a "in theory it works", rather than practical advice here)

> In terms of the commands...In an ideal world I would obviously ask for
> a --rename or mv command which would achieve what I wanted to do.
> Other than that, maybe a word about this in the man for "git mv"? Or

git-mv moves the path, not the name of the submodule, though.

> perhaps "git submodule sync" could give me some kind of warning that I
> did something strange/illegal.

That is harder than thought, because it is normal that a submodule
worktree doesn't exist at the given path (not initialized).
Also it is totally valid to have outdated entries in the .gitmodules file. :/

> Can I ask you, now that I have made this mess, do you have any
> suggestion on how to rectify it or some other way to achieve my goal?

Looking at the script you had above, I would think you need to init
the renamed submodule again, not sure if anything else needs
to be done, as now you'd have essentially two repositories inside
the superprojects .git/modules/ for just one logical submodule.

So I would assume if you were to re-clone the superproject
everything works out fine again (except for bisection/checkout
before that rename. ;) But for that you can symlink the
oldname to the newname inside the .git/modules/ of the superproject
I would assume.

Stefan

Re: [PATCH] Re: Bug with "git submodule update" + subrepo with differing path/name?

[Stefan Beller]

On Thu, Dec 21, 2017 at 2:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> That sounds like a bit of revisionist history, but you weren't around
> back then, so...
>
> https://public-inbox.org/git/11793556371774-git-send-email-junkio@cox.net/#r
>
> is my summarization of discussions before that time. There is a
> mention of "three-level"
> thing by Steven Grimm in
> https://public-inbox.org/git/464E4C94.5070408@midwinter.com/
> in the thread, and together with messages like
> https://public-inbox.org/git/7vejle6p96.fsf@assigned-by-dhcp.cox.net/
> you can see that we already knew that submodule identity (name), path
> in the superproject (path) and where it
> comes from (url) need to be separate things that need to be tied
> together by .gitmodules.

Taking this private reply back to the list, I hope you don't mind.
I think linking into the archives (hence making the huge unstructured
archive a bit more discoverable by these links) is a good idea.

Yes, I was not around when these discussions happened, hence I came
up with a narrative that I can rationalize best.

This message https://public-inbox.org/git/464CF435.1010405@midwinter.com/
helps (me) most as that states the problem that need to be solved and I agree
with the issues and how to solve it. The "symbolic names", however, are the
crux there. They must not be changed, so as Andreas says, maybe "ID" is
a better notion than "submodule name".

I am of the opinion that we'd even want to go as far as to not expose
this symbolic to the user.

So if we redesign from scratch, I would not have "gitlink entries"
but rather "special blobs"[1], that contain the submodule sha1,
as well as their ID.

A special blob could look like:

  git cat-file "kernel/"
  version <sha1>
  id <another hash value, determined at creation time>

Additionally you'd have slight guidance of id -> URL in
an extra ref, versioned independently of the main history:

  <other hash> -> git://kernel.org/...

(A)
So if linux moves away from kernel.org, you can change that ref
independently of your history at that time, such that even old
versions of your main history can obtain the kernel from the correct
URL. Of course a porcelain command could do the double lookup
such that the user can say "kernel/ -> newURL" without needing
to have knowledge of the internal ID.

(B)
Now if you want to move the submodule locally, you can do so by
having that special blob at a different path in your tree.
git-mv "just works" even for nested submodules. No need to
rewrite the .gitmodules file.

(C)
What if you want to drop linux and use some BSD?
Then you change the ID such that a new submodule repo
is created inside .git/modules/. Of course it is also pinned
to a different sha1. A porcelain command such as
"git submodule replace" would sound like a good porcelain-ish.

I think this so far is a clean design, but it is incompatible with history.

The really big difference is that the ID is the core thing that everything
revolves around (similar to the submodule name), but is tied to the path
and is hidden as much as possible from the user, who can use given
commands for each of the scenarios.

The big flaw of this that the ID is sort of random and not based
on content, which is counter to Gits philosophy as a content
addressable FS. Maybe the ID can be set to
HASH(<first version> +<path>), such that when two people
independently of each other make a submodule pointing at the
same commit at the same path, they'd have the same tree-id.

Thanks,
Stefan

[1] An in office discussion hinted at that "special blobs" are not
that special. git-LFS uses this mechanism, but of course their
design flaw is to keep it out of main-Git. But one could posit
"submodules can be implemented using smudge filters alone".