submodule update --force

submodule update --force

[Stefan Zager]

I have a situation where I have a full source tree -- top-level
repository and all submodules -- generated via `git clone -n`.  So,
the directory structure and .git directories are intact, but no actual
source files have been checked out.

If I run `git submodule update` from the top level, some submodules
get checked out, but not others.  Weird.

Root cause is in the cmd_update function in git-submodule.sh, which
does essentially this:

sha1 = submodule revision as registered in top-level module
subsha1 = HEAD revision in submodule checkout

if test "$subsha1" != "$sha1"
then
    git checkout $sha1
fi

In the submodule checkouts, HEAD is refs/heads/master.  In *some* of
the submodules, the revision registered in the top-level repository is
the same as HEAD.  So, for those submodules, `git submodule update`
run from the top level is a no-op, because $sha1 = $subsha1.  That's
true even though there are no actual source files checked out in the
submodule.

For other submodules, $sha1 != $subsha1, and `git submodule update`
checks out the source code as expected.

Confusing!

According to the docs for git-submodule:

       -f, --force
           This option is only valid for add and update commands. When
running add, allow adding an otherwise ignored
           submodule path. When running update, throw away local
changes in submodules when switching to a different
           commit.

I'd like to propose amending the documentation thusly:

According to the docs:

       -f, --force
           This option is only valid for add and update commands. When
running add, allow adding an otherwise ignored
           submodule path. When running update, throw away local
changes in submodules when switching to a different
           commit; if not switching to a different commit, a checkout
to HEAD will still be run.

... and here's the patch to implement it:

diff --git a/git-submodule.sh b/git-submodule.sh
index 64a70d6..8b045d9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -536,7 +536,7 @@ Maybe you want to use 'update --init'?")"
                        die "$(eval_gettext "Unable to find current
revision in submodule path '\$sm_path'")"
                fi

-               if test "$subsha1" != "$sha1"
+               if test "$subsha1" != "$sha1" -o -n "$force"
                then
                        subforce=$force
                        # If we don't already have a -f flag and the
submodule has never been checked out



Thoughts?

Thanks,

Stefan

Re: submodule update --force

[Junio C Hamano]

Stefan Zager <szager@google.com> writes:

> I'd like to propose amending the documentation thusly:
>
> According to the docs:
>
>        -f, --force
>            This option is only valid for add and update commands. When
> running add, allow adding an otherwise ignored
>            submodule path. When running update, throw away local
> changes in submodules when switching to a different
>            commit; if not switching to a different commit, a checkout
> to HEAD will still be run.
>
> ... and here's the patch to implement it:
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 64a70d6..8b045d9 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -536,7 +536,7 @@ Maybe you want to use 'update --init'?")"
>                         die "$(eval_gettext "Unable to find current
> revision in submodule path '\$sm_path'")"
>                 fi
>
> -               if test "$subsha1" != "$sha1"
> +               if test "$subsha1" != "$sha1" -o -n "$force"
>                 then
>                         subforce=$force
>                         # If we don't already have a -f flag and the
> submodule has never been checked out
>
> Thoughts?

Even though I admit that I do not use submodule heavily myself, I think
this is a sane thing to do.  After all, the user explicitly said "I want
to force update it", and it is a strong sign that what is in the working
tree is suspect and the user wants to make sure everything is in sync.

This is a tangent, but what strikes me odd with the code before this patch
is that the decision to recurse into the submodule repository is made
solely on the status of the submodule, and there is no way for the user to
say "I do not want it to recurse" (in other words, "--recursive" option
from the command line does not have any effect on this part of the code).

Perhaps that is because we consider submodules that have been "init"ed
always interesting, and if that is the case that may not be a big deal,
but it might not be a bad idea to allow "--no-recursive" option to mean
something stronger than not giving --recursive option, i.e. not recursing
in a situation where it normally would even when run without --recursive.

Re: submodule update --force

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Stefan Zager <szager@google.com> writes:
>
>> I'd like to propose amending the documentation thusly:
>>
>> According to the docs:
>>
>>        -f, --force
>>            This option is only valid for add and update commands. When
>> running add, allow adding an otherwise ignored
>>            submodule path. When running update, throw away local
>> changes in submodules when switching to a different
>>            commit; if not switching to a different commit, a checkout
>> to HEAD will still be run.
>>
>> ... and here's the patch to implement it:
>>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 64a70d6..8b045d9 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -536,7 +536,7 @@ Maybe you want to use 'update --init'?")"
>>                         die "$(eval_gettext "Unable to find current
>> revision in submodule path '\$sm_path'")"
>>                 fi
>>
>> -               if test "$subsha1" != "$sha1"
>> +               if test "$subsha1" != "$sha1" -o -n "$force"
>>                 then
>>                         subforce=$force
>>                         # If we don't already have a -f flag and the
>> submodule has never been checked out
>>
>> Thoughts?
>
> Even though I admit that I do not use submodule heavily myself, I think
> this is a sane thing to do.  After all, the user explicitly said "I want
> to force update it", and it is a strong sign that what is in the working
> tree is suspect and the user wants to make sure everything is in sync.
>
> This is a tangent, but what strikes me odd with the code before this patch
> is that the decision to recurse into the submodule repository is made
> solely on the status of the submodule, and there is no way for the user to
> say "I do not want it to recurse" (in other words, "--recursive" option
> from the command line does not have any effect on this part of the code).
>
> Perhaps that is because we consider submodules that have been "init"ed
> always interesting, and if that is the case that may not be a big deal,
> but it might not be a bad idea to allow "--no-recursive" option to mean
> something stronger than not giving --recursive option, i.e. not recursing
> in a situation where it normally would even when run without --recursive.

And similarly, it might be a good idea to make the presense of "--recursive"
a bit stronger than the command line that did not say either "--recursive"
nor "--no-recursive", so that a subcommand that normally inspects the
state of the submodule and decide to recurse (or not) can be told to
always recurse into it.

If we follow that line of thought, it may make more sense not to implement
your feature like the above patch, but instead make it so

	if the user told us never to recurse
        then
		nothing
	elif the user told us to always recurse ||
             subsha1 != sha1
        then
		do the "recurse" thing
	fi

so that you can still force it recurse into the submodule, even when you
do not necessarily want the "force checkout" thing to happen to clobber
the working tree.

Re: submodule update --force

[Stefan Zager]

On Wed, May 9, 2012 at 11:18 PM, Junio C Hamano <gitster@pobox.com> wrote:

> If we follow that line of thought, it may make more sense not to implement
> your feature like the above patch, but instead make it so
>
>        if the user told us never to recurse
>        then
>                nothing
>        elif the user told us to always recurse ||
>             subsha1 != sha1
>        then
>                do the "recurse" thing
>        fi
>
> so that you can still force it recurse into the submodule, even when you
> do not necessarily want the "force checkout" thing to happen to clobber
> the working tree.

I'm a bit confused by your use of the term recursion here.  It sounds
like you consider any operation run on a submodule to be a form of
recursion, which is not the way I think about it.  To my mind, any
`git submodule` command should *always* run on the first level of
submodules.  If you're going to specify --no-recurse, then why are you
running `git submodule` at all?  I think 'recursion' only applies to
moving beyond the first level of submodules.

I think your solution would work for our project, because we only use
one level of submodules.  But IMO the functionality, as you have
written it, is less generally useful.

Stefan

Re: submodule update --force

[Junio C Hamano]

Stefan Zager <szager@google.com> writes:

> ...  To my mind, any
> `git submodule` command should *always* run on the first level of
> submodules.  If you're going to specify --no-recurse, then why are you
> running `git submodule` at all?  I think 'recursion' only applies to
> moving beyond the first level of submodules.

Very true.

Submodule folks, any opinion on the Stefan's approach?

Re: submodule update --force

[Heiko Voigt]

Hallo all,

On Thu, May 10, 2012 at 07:58:09AM -0700, Junio C Hamano wrote:
> Stefan Zager <szager@google.com> writes:
> 
> > ...  To my mind, any
> > `git submodule` command should *always* run on the first level of
> > submodules.  If you're going to specify --no-recurse, then why are you
> > running `git submodule` at all?  I think 'recursion' only applies to
> > moving beyond the first level of submodules.
> 
> Very true.
> 
> Submodule folks, any opinion on the Stefan's approach?

The distinction between first level of submodules and deeper is only
present in the "git submodule" subcommand and I think mainly for
historical reasons. I do not see a use case where this would be helpful.
To skip uninteresting submodules one can always use the
submodule.$name.update option set to 'none'. (I just found that its
documentation is in the wrong place but I will send a seperate patch
about that).

In the non submodule commands we usually name this option
--recurse-submodules=always and have another
--recurse-submodules=on-demand option for the current behavior. Those
options would either recurse or do nothing with the submodule. Such a
behavior, as pointed out, does not make sense for 'submodule update'.
Similar options names for 'submodule update' would probably be
--recurse=always and --recurse=on-demand.

Nonetheless is force a term where the user probably wants to skip all
optimizations which the sha1 equality provides. So to make the current
behavior more consistent I would be fine with adding this change.

One thing which might make force even more useful would be to also skip
the "is the sha1 available"-check for fetch that is possibly run before
the checkout and just always run the fetch.

In the long term, once checkout has learned things 'submodule update' is
currently doing, it probably makes sense to let 'submodule update'
always recurse into all checked out submodules. Since then it does not
make sense to run 'submodule update' for much more than resetting
things or changing the currently registered commits anymore. So in the
bright new future the 'on-demand' part will probably move away from
'submodule update' and as such it does not make sense to implement
the seperate recurse options I described above.

What do others think?

Cheers Heiko

Re: submodule update --force

[Phil Hord]

On Thu, May 10, 2012 at 2:57 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>
> Hallo all,
>
> On Thu, May 10, 2012 at 07:58:09AM -0700, Junio C Hamano wrote:
> > Stefan Zager <szager@google.com> writes:
> >
> > > ...  To my mind, any
> > > `git submodule` command should *always* run on the first level of
> > > submodules.  If you're going to specify --no-recurse, then why are you
> > > running `git submodule` at all?  I think 'recursion' only applies to
> > > moving beyond the first level of submodules.
> >
> > Very true.
> >
> > Submodule folks, any opinion on the Stefan's approach?
>
> The distinction between first level of submodules and deeper is only
> present in the "git submodule" subcommand and I think mainly for
> historical reasons. I do not see a use case where this would be helpful.

Do I understand you to mean that you think the git-submodule ...
--recursive option is archaic?  I would agree that one might expect it
to be the default option, but I do not think it should be deprecated
in any way.

> To skip uninteresting submodules one can always use the
> submodule.$name.update option set to 'none'. (I just found that its
> documentation is in the wrong place but I will send a seperate patch
> about that).
>
> In the non submodule commands we usually name this option
> --recurse-submodules=always and have another
> --recurse-submodules=on-demand option for the current behavior. Those
> options would either recurse or do nothing with the submodule.  Such a
> behavior, as pointed out, does not make sense for 'submodule update'.
> Similar options names for 'submodule update' would probably be
> --recurse=always and --recurse=on-demand.
>
> Nonetheless is force a term where the user probably wants to skip all
> optimizations which the sha1 equality provides. So to make the current
> behavior more consistent I would be fine with adding this change.
>
> One thing which might make force even more useful would be to also skip
> the "is the sha1 available"-check for fetch that is possibly run before
> the checkout and just always run the fetch.
>
> In the long term, once checkout has learned things 'submodule update' is
> currently doing, it probably makes sense to let 'submodule update'
> always recurse into all checked out submodules. Since then it does not
> make sense to run 'submodule update' for much more than resetting
> things or changing the currently registered commits anymore. So in the
> bright new future the 'on-demand' part will probably move away from
> 'submodule update' and as such it does not make sense to implement
> the seperate recurse options I described above.
>
> What do others think?

I think there are three cases:

1. I want to update any sha1-mismatching submodules so
    their HEAD matches the superproject gitlink.

    git submodule update

2. Same as (1) above, but also check out files for all
    submodules which are not already checked out.

    git submodule update &&
    git submodule foreach 'git checkout HEAD || :'

3. I want to update exactly to the gitlinks in the superproject
   and discard any local or staged changes.

    git submodule update -f

(2) above is the case Junio was trying to cover.  I cannot think of an
elegant name for the switch for such an option, but I would be
surprised it to find it is not the default behavior if I also
encountered it like Stefan did.  We should try to eliminate surprises
to help dispel the notion that submodules are unwieldy.

(3) is too heavy when I really only wanted (2).

I do not understand that use case that led Stefan to the predicament
he was in where he had submodules with HEADs but with no checked out
files.  But I do not begrudge his being there.


Regards,
Phil

Re: submodule update --force

[Heiko Voigt]

Hi,

On Fri, May 11, 2012 at 04:56:07PM -0400, Phil Hord wrote:
> On Thu, May 10, 2012 at 2:57 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > On Thu, May 10, 2012 at 07:58:09AM -0700, Junio C Hamano wrote:
> > > Stefan Zager <szager@google.com> writes:
> > >
> > > > ... ?To my mind, any
> > > > `git submodule` command should *always* run on the first level of
> > > > submodules. ?If you're going to specify --no-recurse, then why are you
> > > > running `git submodule` at all? ?I think 'recursion' only applies to
> > > > moving beyond the first level of submodules.
> > >
> > > Very true.
> > >
> > > Submodule folks, any opinion on the Stefan's approach?
> >
> > The distinction between first level of submodules and deeper is only
> > present in the "git submodule" subcommand and I think mainly for
> > historical reasons. I do not see a use case where this would be helpful.
> 
> Do I understand you to mean that you think the git-submodule ...
> --recursive option is archaic?  I would agree that one might expect it
> to be the default option, but I do not think it should be deprecated
> in any way.

In a way yes its archaic since I do not know why one would distinguish
between the first level of populated submodules and below. For example
if you have nested submodules and want them all be populated you need to
use

	git submodule update --init --recursive

The sequence

	git submodule init --recursive
	git submodule update --recursive

does not do the same thing but would have to be called multiple times
until you have reached the deepest level. IMO that is confusing but not
only a problem of this option.

> > To skip uninteresting submodules one can always use the
> > submodule.$name.update option set to 'none'. (I just found that its
> > documentation is in the wrong place but I will send a seperate patch
> > about that).
> >
> > In the non submodule commands we usually name this option
> > --recurse-submodules=always and have another
> > --recurse-submodules=on-demand option for the current behavior. Those
> > options would either recurse or do nothing with the submodule.  Such a
> > behavior, as pointed out, does not make sense for 'submodule update'.
> > Similar options names for 'submodule update' would probably be
> > --recurse=always and --recurse=on-demand.
> >
> > Nonetheless is force a term where the user probably wants to skip all
> > optimizations which the sha1 equality provides. So to make the current
> > behavior more consistent I would be fine with adding this change.
> >
> > One thing which might make force even more useful would be to also skip
> > the "is the sha1 available"-check for fetch that is possibly run before
> > the checkout and just always run the fetch.
> >
> > In the long term, once checkout has learned things 'submodule update' is
> > currently doing, it probably makes sense to let 'submodule update'
> > always recurse into all checked out submodules. Since then it does not
> > make sense to run 'submodule update' for much more than resetting
> > things or changing the currently registered commits anymore. So in the
> > bright new future the 'on-demand' part will probably move away from
> > 'submodule update' and as such it does not make sense to implement
> > the seperate recurse options I described above.
> >
> > What do others think?
> 
> I think there are three cases:
> 
> 1. I want to update any sha1-mismatching submodules so
>     their HEAD matches the superproject gitlink.
> 
>     git submodule update
> 
> 2. Same as (1) above, but also check out files for all
>     submodules which are not already checked out.
> 
>     git submodule update &&
>     git submodule foreach 'git checkout HEAD || :'
> 
> 3. I want to update exactly to the gitlinks in the superproject
>    and discard any local or staged changes.
> 
>     git submodule update -f
> 
> (2) above is the case Junio was trying to cover.  I cannot think of an
> elegant name for the switch for such an option, but I would be
> surprised it to find it is not the default behavior if I also
> encountered it like Stefan did.  We should try to eliminate surprises
> to help dispel the notion that submodules are unwieldy.

Yes we should eliminate surprises thats true. On the other hand there is
no way to setup submodules in the way Stefan had them by using the git
submodule command or is there? So for his use case the command sequence
you described seems to be more appropriate but I am not sure whether
that justifies a separate option for it.

> (3) is too heavy when I really only wanted (2).
> 
> I do not understand that use case that led Stefan to the predicament
> he was in where he had submodules with HEADs but with no checked out
> files.  But I do not begrudge his being there.

Yes, but currently -f is wrong in the way that when the submodules HEAD
sha1 is the same as registered in the superproject it will skip the
checkout. That is wrong when you have local uncommitted changes in the
worktree. In such a state I would expect it to throw away those local
changes and checkout HEAD. So I think Stefans patch makes sense anyway
even though it might actually be to heavy for his use case.

Cheers Heiko

Re: submodule update --force

[Stefan Zager]

On Mon, May 14, 2012 at 9:52 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:

> Yes we should eliminate surprises thats true. On the other hand there is
> no way to setup submodules in the way Stefan had them by using the git
> submodule command or is there? So for his use case the command sequence
> you described seems to be more appropriate but I am not sure whether
> that justifies a separate option for it.
>
>> (3) is too heavy when I really only wanted (2).
>>
>> I do not understand that use case that led Stefan to the predicament
>> he was in where he had submodules with HEADs but with no checked out
>> files.  But I do not begrudge his being there.
>
> Yes, but currently -f is wrong in the way that when the submodules HEAD
> sha1 is the same as registered in the superproject it will skip the
> checkout. That is wrong when you have local uncommitted changes in the
> worktree. In such a state I would expect it to throw away those local
> changes and checkout HEAD. So I think Stefans patch makes sense anyway
> even though it might actually be to heavy for his use case.

To satisfy your curiosity, although it probably won't help me make my
case: I'm working on very large project with a lot of commit history.
Cloning this repository is prohibitively slow, so I'm trying to speed
it up by periodically creating a snapshot of the repository (and all
submodule repositories) that can be downloaded and cloned locally.

I first tried using `git bundle`, but cloning from the bundle files
was still very slow, because it took a long time to replay all the
commit history to recreate the index.  So, I hit upon another
solution: run `git clone -n` on the top-level repository and all
submodule repositories, and create a zip file of the empty checkout.
The resulting zip file is the same size as the git bundle file, but
unpacking it on the client (basically, `unzip` followed by `git
checkout HEAD`) is much faster.

Stefan

Re: submodule update --force

[Heiko Voigt]

Hi,

On Mon, May 14, 2012 at 10:17:53AM -0700, Stefan Zager wrote:
> On Mon, May 14, 2012 at 9:52 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> 
> > Yes we should eliminate surprises thats true. On the other hand there is
> > no way to setup submodules in the way Stefan had them by using the git
> > submodule command or is there? So for his use case the command sequence
> > you described seems to be more appropriate but I am not sure whether
> > that justifies a separate option for it.
> >
> >> (3) is too heavy when I really only wanted (2).
> >>
> >> I do not understand that use case that led Stefan to the predicament
> >> he was in where he had submodules with HEADs but with no checked out
> >> files. ?But I do not begrudge his being there.
> >
> > Yes, but currently -f is wrong in the way that when the submodules HEAD
> > sha1 is the same as registered in the superproject it will skip the
> > checkout. That is wrong when you have local uncommitted changes in the
> > worktree. In such a state I would expect it to throw away those local
> > changes and checkout HEAD. So I think Stefans patch makes sense anyway
> > even though it might actually be to heavy for his use case.
> 
> To satisfy your curiosity, although it probably won't help me make my
> case: I'm working on very large project with a lot of commit history.
> Cloning this repository is prohibitively slow, so I'm trying to speed
> it up by periodically creating a snapshot of the repository (and all
> submodule repositories) that can be downloaded and cloned locally.
> 
> I first tried using `git bundle`, but cloning from the bundle files
> was still very slow, because it took a long time to replay all the
> commit history to recreate the index.  So, I hit upon another
> solution: run `git clone -n` on the top-level repository and all
> submodule repositories, and create a zip file of the empty checkout.
> The resulting zip file is the same size as the git bundle file, but
> unpacking it on the client (basically, `unzip` followed by `git
> checkout HEAD`) is much faster.

For this use case running

	git submodule update -f

is just fine. The only objection I had was against a new option. So I am
all for making -f skip this sha1 comparing optimization like your patch
did.

On a side note: I am surprised that cloning through git is really that
much slower than copying a zip from the network. Do you run git gc
regularly enough on the server?

Cheers Heiko

Re: submodule update --force

[Stefan Zager]

On Mon, May 14, 2012 at 12:18 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:

> On a side note: I am surprised that cloning through git is really that
> much slower than copying a zip from the network. Do you run git gc
> regularly enough on the server?

Don't know about running `git gc`, but I can tell you that the
'resolving deltas' step on the client side is very slow; anecdotally,
it appears to take longer than the network transfer.  We also would
like to relieve the server of the burden of creating large pack files
many times a day (the repo is frequently cloned).  I'll check to see
whether `git gc` can help us.

Thanks,

Stefan

Re: submodule update --force

[Heiko Voigt]

On Mon, May 14, 2012 at 12:29:02PM -0700, Stefan Zager wrote:
> On Mon, May 14, 2012 at 12:18 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> 
> > On a side note: I am surprised that cloning through git is really that
> > much slower than copying a zip from the network. Do you run git gc
> > regularly enough on the server?
> 
> Don't know about running `git gc`, but I can tell you that the
> 'resolving deltas' step on the client side is very slow; anecdotally,
> it appears to take longer than the network transfer.  We also would
> like to relieve the server of the burden of creating large pack files
> many times a day (the repo is frequently cloned).  I'll check to see
> whether `git gc` can help us.

I do not think you need to do this many times a day a cronjob during the
night should be sufficient. Maybe also do a gc --aggressive one a week?

Cheers Heiko