git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Bug in git submodule update --remote
@ 2021-05-14 18:11 Ben Avison
  2021-05-19 10:49 ` Atharva Raykar
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ben Avison @ 2021-05-14 18:11 UTC (permalink / raw)
  To: git

A recent-ish change in git 2.27.1, introduced in commit f0a96e8d, has 
also got me wondering about whether some related functionality is 
correct. I'm not sure what the best way to fix things is, so can I 
invite opinions?

The scenario: I have a repository with a submodule. The submodule's 
upstream repository uses a fork workflow, so the submodule has two 
remotes, one for pulling in other people's changes, and one for 
uploading my own pull requests.

$ mkdir myproject
$ cd myproject
$ git init
$ git submodule add https://github.com/acme/library
$ cd library
$ git remote add -f myfork https://github.com/user/library
$ cd ..
$ git add .
$ git commit -m "Initial commit"
$ git remote add origin https://github.com/user/myproject
$ git push -u origin master

Furthermore, assume I forked https://github.com/acme/example some time 
ago, so that the master branches between it and my fork have diverged.

Time passes. People hack away on both projects. I want to fix a bug or 
implement a new feature in the library, so I start by ensuring both are 
up-to-date:

$ git pull
$ git submodule update --remote
$ cd library
$ git checkout -b new-feature
$ # hack away
$ git add .
$ git commit -m "New feature"
$ git push -u myfork new-feature
$ cd ..

Some more time passes, and I want to work on it again. Again, I start by 
ensuring I'm up-to-date. Before git 2.27.1, I could do:

$ git submodule update --remote

Now, I get:

fatal: Needed a single revision
Unable to find current myfork/HEAD revision in submodule path 'library'

What's going on is that within "git submodule update --remote", the sha1 
used is formed by looking up the submodule's ref "<remote>/<branch>". 
The change in git 2.27.1 is that if no remote tracking branch is stated 
in the superproject's .gitmodules file, <branch> defaults to "HEAD" 
rather than "master" as previously. That's fine if <remote> is "origin", 
but in practice, it depends on how the submodule is currently checked out:

* if it's in detached HEAD state, <remote> is set to "origin"
* if its branch is not a tracking branch, "origin" is also used
* but if it's a tracking branch (as I used in my workflow above - not 
uncommon in pull request workflows because you might grant other people 
access to the branch during the review process) then it looks up the 
name of the remote which is being tracked

First observation: a ref called "myfork/HEAD" presumably *could* have 
been created by the "git remote add" command, reflecting that remote's 
default branch. Should it?

In practice, that's not actually what I'd want, though. If I do "git 
submodule update --remote", I personally normally do so as a shortcut 
compared to cloning everything again. I don't expect the behaviour to 
change depending on whether I happen to have left one of the submodules 
checked out on a tracking branch or not: myfork/master is potentially 
quite out of date compared to origin/master.

It also makes very little sense to me that issuing "git submodule update 
--remote" twice in quick succession would leave us in a different state, 
because the first call puts all the submodules into detached HEAD state, 
meaning that the second call always looks up the remote tracking 
branches from the submodule's "origin" remote.

One way this could be fixed is that if <branch> turns out to be "HEAD", 
we could force <remote> to be "origin". However, this doesn't address 
the equivalent problem that arises if the remote tracking branch *is* 
named in .gitmodules.

I'd therefore like to propose that "git submodule update --remote" 
*always* uses the remote named "origin" to form the ref used to check 
out the submodule. However, I'm not sure whether everyone would agree 
that this constitutes a bugfix, or whether I'd need to introduce a new 
switch to enforce this behaviour.

So, what do you think?

Ben

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-05-22 19:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-14 18:11 Bug in git submodule update --remote Ben Avison
2021-05-19 10:49 ` Atharva Raykar
2021-05-19 14:41   ` Ben Avison
2021-05-21 11:47     ` Atharva Raykar
2021-05-21 18:36       ` Christian Couder
2021-05-22 19:15     ` Philippe Blain
2021-05-19 11:19 ` Atharva Raykar
2021-05-22 18:02 ` Philippe Blain

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 NNTP newsgroup(s).