git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v6] clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabled

From
Emily Shaffer <emilyshaffer@google.com>
Date
Aug 17, 2021, 23:02 UTC
Message-ID
<YRw/8tThN7djNE+E@google.com>
In-Reply-To
<xmqqy293ucju.fsf@gitster.g>
On Sat, Aug 14, 2021 at 11:05:41AM -0700, Junio C Hamano wrote:
Show 66 quoted lines
> 
> "Mahi Kolla via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > diff --git a/builtin/clone.c b/builtin/clone.c
> > index 66fe66679c8..a08d9012243 100644
> > --- a/builtin/clone.c
> > +++ b/builtin/clone.c
> > @@ -986,6 +986,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> >  	struct remote *remote;
> >  	int err = 0, complete_refs_before_fetch = 1;
> >  	int submodule_progress;
> > +	int sticky_recursive_clone;
> 
> This variable does not have to be in such a wider scope, I think.
> 
> 
> >  	struct transport_ls_refs_options transport_ls_refs_options =
> >  		TRANSPORT_LS_REFS_OPTIONS_INIT;
> > @@ -1130,6 +1131,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> >  					   strbuf_detach(&sb, NULL));
> >  		}
> 
> Just in this scope, where "struct string_list_item *item" and
> "struct strbuf sb" are declared, is where an extra int variable,
> which receives the configuration value, needs to exist.
> 
> Also, for a variable that is used only to receive value from
> git_config_get_bool(), immediately to be used and then never used
> again, we do not need such a long and descriptive name.
> 
> > +		if (!git_config_get_bool("submodule.stickyRecursiveClone", &sticky_recursive_clone)
> > +		    && sticky_recursive_clone) {
> > +		    string_list_append(&option_config, "submodule.recurse=true");
> > +		}
> 
> We do not need {} around a single statement block.
> 
> Taken together, perhaps like the attached.
> 
> I'll queue the patch posted as-is for now.
> 
> Thanks.
> 
> diff --git c/builtin/clone.c w/builtin/clone.c
> index 66fe66679c..c4e02d2f78 100644
> --- c/builtin/clone.c
> +++ w/builtin/clone.c
> @@ -1114,6 +1114,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  	if (option_recurse_submodules.nr > 0) {
>  		struct string_list_item *item;
>  		struct strbuf sb = STRBUF_INIT;
> +		int val;
>  
>  		/* remove duplicates */
>  		string_list_sort(&option_recurse_submodules);
> @@ -1130,6 +1131,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  					   strbuf_detach(&sb, NULL));
>  		}
>  
> +		if (!git_config_get_bool("submodule.stickyRecursiveClone", &val) &&
> +		    val)
> +		    string_list_append(&option_config, "submodule.recurse=true");
> +
>  		if (option_required_reference.nr &&
>  		    option_optional_reference.nr)
>  			die(_("clone --recursive is not compatible with "

I like the changes you propose here. It also looks to me like you wanted to wait for Mahi to send the updates herself, which I appreciate.

Mahi's internship ended last week and I believe she told me she's planning to be very busy this week; so if we don't hear from her by this time next week, I'll send a reroll with these changes (unless you want to just take them yourself without the list churn).

 - Emily
Previous: Junio C HamanoNext: Junio C Hamano
Message 40 of 46 in “clone: update submodule.recurse in config when using --recurse-submodule”
  1. 0/2 clone: update submodule.recurse in config when using --recurse-submoduleMahi Kolla via GitGitGadget, Aug 2, 2021
  2. 1/2 clone: update submodule.recurse in config when using --recurse-submoduleMahi Kolla via GitGitGadget, Aug 2, 2021
  3. 2/2 clone: update submodule.recurse in config when using --recurse-submoduleMahi Kolla via GitGitGadget, Aug 2, 2021
  4. 0/3 clone: update submodule.recurse in config when using --recurse-submoduleMahi Kolla via GitGitGadget, Aug 2, 2021
  5. 1/3 clone: update submodule.recurse in config when using --recurse-submoduleMahi Kolla via GitGitGadget, Aug 2, 2021
  6. 2/3 clone: update submodule.recurse in config when using --recurse-submoduleMahi Kolla via GitGitGadget, Aug 2, 2021
  7. 3/3 clone test: update whitespace according to style guideMahi Kolla via GitGitGadget, Aug 2, 2021
  8. 0/4 clone: update submodule.recurse in config when using --recurse-submoduleMahi Kolla via GitGitGadget, Aug 2, 2021
  9. 1/4 clone: update submodule.recurse in config when using --recurse-submoduleMahi Kolla via GitGitGadget, Aug 2, 2021
  10. Philippe BlainAug 3, 2021
  11. Mahi KollaAug 7, 2021
  12. 2/4 clone: update submodule.recurse in config when using --recurse-submoduleMahi Kolla via GitGitGadget, Aug 2, 2021
  13. 3/4 clone test: update whitespace according to style guideMahi Kolla via GitGitGadget, Aug 2, 2021
  14. 4/4 clone: update whitespace according to style guideMahi Kolla via GitGitGadget, Aug 2, 2021
  15. Philippe BlainAug 3, 2021
  16. Junio C HamanoAug 3, 2021
  17. clone: update submodule.recurse in config when using --recurse-submoduleMahi Kolla via GitGitGadget, Aug 9, 2021
  18. Junio C HamanoAug 9, 2021
  19. Mahi KollaAug 10, 2021
  20. Junio C HamanoAug 10, 2021
  21. Philippe BlainAug 10, 2021
  22. Mahi KollaAug 10, 2021
  23. Junio C HamanoAug 11, 2021
  24. clone: set submodule.recurse=true if user enables feature.experimental flagMahi Kolla via GitGitGadget, Aug 12, 2021
  25. Junio C HamanoAug 12, 2021
  26. Emily ShafferAug 12, 2021
  27. Philippe BlainAug 13, 2021
  28. Mahi KollaAug 13, 2021
  29. Junio C HamanoAug 13, 2021
  30. Junio C HamanoAug 13, 2021
  31. Junio C HamanoAug 13, 2021
  32. Emily ShafferAug 13, 2021
  33. Junio C HamanoAug 13, 2021
  34. Emily ShafferAug 13, 2021
  35. Mahi KollaAug 13, 2021
  36. Junio C HamanoAug 13, 2021
  37. Felipe ContrerasAug 13, 2021
  38. clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabledMahi Kolla via GitGitGadget, Aug 14, 2021
  39. Junio C HamanoAug 14, 2021
  40. Emily ShafferAug 17, 2021
  41. Junio C HamanoAug 18, 2021
  42. fixup! clone: set submodule.recurse=true if submodule.stickyRecursiveClone enabledJunio C Hamano, Aug 18, 2021
  43. Emily ShafferAug 19, 2021
  44. Emily ShafferAug 30, 2021
  45. Junio C HamanoAug 30, 2021
  46. Philippe BlainAug 18, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.