Re: [PATCH v5] describe: refresh the index when 'broken' flag is used

From: Abhijeet Sonar <abhijeet.nkt@gmail.com>
Date: Jun 26, 2024, 12:06 UTC
Message-ID: <2e80306e-2474-4254-95eb-c2902a56ffdd@gmail.com>
In-Reply-To: <CAOLa=ZRz2KEGiBnX1YP6JG1nXXHLfw9A3dHKO3s_ViLhq+bWww@mail.gmail.com>

On 26/06/24 17:00, Karthik Nayak wrote:

> Not worth a reroll, but you don't have to create file.new twice.

Actually, now that I think of it, those two were better off being separate tests. It might so happen the first call to describe refreshes the index, due to which the second call with the --broken option does not bug-out in the way it would if the command was run by itself. Having them separate would give them enough isolation so that previous command does not interfere with the later.

Show 17 quoted lines

>> Range-diff against v4:
>> 1:  1da5fa48d9 ! 1:  52f590b70f describe: refresh the index when 'broken' flag is used
>>      @@ builtin/describe.c: int cmd_describe(int argc, const char **argv, const char *pr
>>       +			cp.git_cmd = 1;
>>       +			cp.no_stdin = 1;
>>       +			cp.no_stdout = 1;
>>      -+			run_command(&cp);
>>      -+			strvec_clear(&cp.args);
>>      ++			if (run_command(&cp))
>>      ++				child_process_clear(&cp);
>>       +
>>        			strvec_pushv(&cp.args, diff_index_args);
>>        			cp.git_cmd = 1;
>> --
>> 2.45.2.606.g9005149a4a.dirty
> 
> Other than this, this looks good to me.

I am not sure if I follow this one. Am I expected to not share the struct child_process between the two sub-process calls?

Thanks