Re: [PATCH 4/5] rebase -i: don't fork git checkout

>>> While in general I think it's a good thing to avoid forking, this
>>> change might result in behavioral differences. Any config that
>>> affects 'git checkout' but not the internal 'reset.c::reset_head'
>>> function might play a role in the rebase UX.
>>>
>>> One that immediately came to mind is 'submodule.recurse'. This
>>> initial 'onto' checkout was pretty much the only part of 'git
>>> rebase' that did something useful for submodules, so it's kind of
>>> sad to see it regress.
>>
>> Thanks for pointing that out. As a non-submodule user my question
>> would be is it actually useful for the initial checkout to work that
>> way if the rest of rebase (and the checkout for the am backend)
>> ignores submodules? reset.c::reset_head() just uses unpack trees like
>> checkout so if rebase read 'submodule.recurse' then reset_head()
>> would work like 'git checkout' and also 'git rebase --abort' and the
>> "reset" command in the todo list would start checking out submodules.

>> I'm reluctant to do that until the merge backend also handles
>> submodules unless there is a good reason that such partial submodule
>> support would help submodule users.
> 
> Yeah, it's not that useful, I have to admit; it can also be very confusing
> since some parts of rebase are affected, and some not. For example, any 
> time
> the rebase stops, like for 'edit', 'break', and when there are 
> conflicts, the
> submodules are not updated. So I think a full solution is better than a 
> partial
> solution; in the meantime I'm thinking the change you are proposing 
> would actually
> be less confusing, even if it slightly changes behaviour...
> 
> As an aside, I *think* reading submodule.recurse in rebase like it's 
> done in checkout
> et al., i.e. something like this:
> 
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 33e0961900..125ec907e4 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -26,6 +26,7 @@
>   #include "rerere.h"
>   #include "branch.h"
>   #include "sequencer.h"
> +#include "submodule.h"
>   #include "rebase-interactive.h"
>   #include "reset.h"
> 
> @@ -1106,6 +1107,9 @@ static int rebase_config(const char *var, const 
> char *value, void *data)
>           return git_config_string(&opts->default_backend, var, value);
>       }
> 
> +    if (!strcmp(var, "submodule.recurse"))
> +        return git_default_submodule_config(var, value, data);

>       return git_default_config(var, value, data);
>   }
> 
> 
> would actually also affect the merges
> performed during the rebase, since that would affect the "global" state 
> in submodule.c.
> I hacked exactly that the other day but did not test extensively...