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.

quoted

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