SZEDER Gábor <szeder.dev@xxxxxxxxx> writes:
> this patch below and a7f3240877 (grep: ignore --recurse-submodules if
> --no-index is given, 2020-01-26) on topic
> 'pb/do-not-recurse-grep-no-index' don't work well together, and cause
> failure of the test 'grep --recurse-submodules --no-index ignores
> --recurse-submodules' in 't7814-grep-recurse-submodules.sh', i.e. in
> the new test added in a7f3240877.
Yup, I was looking at it and trying to see what was going on.
>> + if (recurse_submodules && (!use_index || untracked))
>> + die(_("option not supported with --recurse-submodules"));
>
> So this patch moves this condition here, expecting git to die with
> '--recurse-submodules --no-index'. However, a7f3240877 removes the
> '!use_index' part of the condition, so we won't die here ...
>
>> if (list.nr || cached || show_in_pager) {
>> if (num_threads > 1)
>> warning(_("invalid option combination, ignoring --threads"));
>> @@ -1071,6 +1072,14 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>> && (opt.pre_context || opt.post_context ||
>> opt.file_break || opt.funcbody))
>> skip_first_line = 1;
>> +
>> + /*
>> + * Pre-read gitmodules (if not read already) to prevent racy
>> + * lazy reading in worker threads.
>> + */
>> + if (recurse_submodules)
>> + repo_read_gitmodules(the_repository, 1);
>
> ... and eventually reach this condition, which then reads the
> submodules even with '--no-index', which is just what a7f3240877 tried
> to avoid, thus triggering the test failure.
>
> It might be that all we need is changing this condition to:
>
> if (recurse_submodules && use_index)
>
> Or maybe not, but this change on top of 'pu' makes t7814 succeed
> again.
Sounds like a sensible idea.
