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

Re: [PATCH v2 5/5] fetch: Make --jobs control submodules and remotes

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 14, 2019, 15:54 UTC
Message-ID
<xmqqy2zv68va.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<20190814083245.GP20404@szeder.dev>
SZEDER Gábor <szeder.dev@gmail.com> writes:
Show 43 quoted lines
> On Mon, Aug 12, 2019 at 02:34:48PM -0700, Palmer Dabbelt wrote:
>> diff --git a/builtin/fetch.c b/builtin/fetch.c
>> index 67d001f3f78b..41498e9efb3b 100644
>> --- a/builtin/fetch.c
>> +++ b/builtin/fetch.c
>> @@ -114,6 +114,20 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
>>  	return git_default_config(k, v, cb);
>>  }
>>  
>> +static int parse_jobs_arg(const struct option *opt, const char *arg, int unset)
>> +{
>> +	int jobs;
>> +
>> +	jobs = atoi(arg);
>> +	if (jobs < 1)
>> +		die(_("There must be a positive number of jobs"));
>> +
>> +	max_children_for_submodules = jobs;
>> +	max_children_for_fetch = jobs;
>> +
>> +	return 0;
>> +}
>> +
>>  static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)
>>  {
>>  	BUG_ON_OPT_NEG(unset);
>> @@ -142,12 +156,13 @@ static struct option builtin_fetch_options[] = {
>>  		    N_("fetch all tags and associated objects"), TAGS_SET),
>>  	OPT_SET_INT('n', NULL, &tags,
>>  		    N_("do not fetch all tags (--no-tags)"), TAGS_UNSET),
>> -	OPT_INTEGER('j', "jobs", &max_children_for_submodules,
>> +	{ OPTION_CALLBACK, 'j', "jobs", NULL, N_("jobs"),
>> +		    N_("number of parallel tasks to run while fetching"),
>> +		    PARSE_OPT_OPTARG, &parse_jobs_arg },
>
> These changes result segmentation faults in the tests '--quiet
> propagates to parallel submodules' and 'fetching submodules respects
> parallel settings' in 't5526-fetch-submodules.sh'.
>
> If the number of jobs is specified as '-j 2' or '--jobs 7', i.e. as an
> unstuck argument of the option, as opposed to '-j2' or '--jobs=7',
> then 'arg' in the parse_jobs_arg() callback is NULL, which then causes
> the segfault somewhere inside that atoi() call.
True.  

An easier and more readable way would be to set another "nr-jobs" variable using plain vanilla OPT_INTEGER() and override the other two with it when they are not set after parse_options() returns, I guess.

Previous: SZEDER GáborNext: Palmer Dabbelt
Message 13 of 14 in “fetch: Extend --jobs to multiple remotes”
  1. 0/5 fetch: Extend --jobs to multiple remotesPalmer Dabbelt, Aug 12, 2019
  2. 1/5 fetch: Rename max_children to max_children_for_submodulesPalmer Dabbelt, Aug 12, 2019
  3. 2/5 fetch: Add the "--fetch-jobs" optionPalmer Dabbelt, Aug 12, 2019
  4. Eric WongAug 13, 2019
  5. 3/5 fetch: Add the fetch.jobs config keyPalmer Dabbelt, Aug 12, 2019
  6. 4/5 fetch: Add the --submodule-fetch-jobs optionPalmer Dabbelt, Aug 12, 2019
  7. 5/5 fetch: Make --jobs control submodules and remotesPalmer Dabbelt, Aug 12, 2019
  8. Johannes SchindelinAug 13, 2019
  9. Palmer DabbeltAug 13, 2019
  10. Junio C HamanoAug 13, 2019
  11. Palmer DabbeltAug 13, 2019
  12. SZEDER GáborAug 14, 2019
  13. Junio C HamanoAug 14, 2019
  14. Palmer DabbeltAug 14, 2019

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.