Re: [RFC PATCH 2/2] branch: add --recurse-submodules option for branch creation

From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Sep 22, 2021, 12:28 UTC
Message-ID: <084caed9-5854-9fe2-8476-b5325ddac52c@gmail.com>
In-Reply-To: <20210921232529.81811-3-chooglen@google.com>

Hi Glen,

Le 2021-09-21 à 19:25, Glen Choo a écrit :

Show 12 quoted lines

> When working with a superproject and submodules, it can be helpful to
> create topic branches to coordinate the work between each repository.
> 
> Teach cmd_branch to accept the --recurse-submodules option when creating
> branches. When specified, like
> 
>    git branch --recurse-submodules topic
> 
> git-branch will create the "topic" branch in the superproject and all
> submodules.
> 
> recurse_submodules is only supported for creating a branch. git-branch

small nit: we usually don't refer to the dashed form of commands anymore: 'git branch' would be preferable.

> will fail if the user attempts to use --recurse-submodules with anything
> other than creating a branch. If a user has submodule.recurse in their
> config, git-branch will use --recurse-submodules for creating a branch,
> but will treat it as unset for any other operation.

That seems like a good starting point.

However, I think that for this new feature, I would prefer *also* having a new config 'branch.recurseSubmodules' (or similar) that would allow more granular control than 'submodule.recurse', which influences several commands. Personnally I have 'submodule.recurse' set to true in my '~/.gitconfig', because I want the submodules working trees to be updated when I use 'git checkout' to change branches, 'git grep' to search the working tree, etc., but I usually do not *work* on the submodules in my project and I would not like new branches being created in them every time I create a new branch in the superproject. In other words, 'branch.recurseSubmodules=false' would have higher priority than 'submodule.recurse=true'.

Show 7 quoted lines

> 
> There is no easy way to get the remotes of a submodule because remotes.c
> stores state as static variables. As such, branch tracking is disabled
> when using --recurse-submodules.
> 
> Signed-off-by: Glen Choo <chooglen@google.com>
> ---

Show 11 quoted lines

> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index e575ffb4ff..858831d4cf 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -1421,5 +1421,63 @@ test_expect_success 'invalid sort parameter in configuration' '
>   		test_must_fail git branch
>   	)
>   '
> +# Submodule tests
> +
>

Most tests for submodules are usually in separate test files. I don't think this is a set-in-stone rule, but if more tests are coming in the future, maybe a new test file t????-branch-submodule.sh would be appropriate ? Just a small suggestion.

Thanks for working on this! It's exciting to see new developments in the submodule area :)

Philippe.