Subject: Re: git bisect bad @

From: Junio C Hamano <[email-masked]>
Date: Fri, 14 Jan 2022 10:42:31 -0800

René Scharfe <[email-masked]> writes:

> Am 13.01.22 um 16:16 schrieb Ramkumar Ramachandra:
>> René Scharfe wrote:
>>> The implementation language of git bisect is not immediately relevant
>>> here, but that the shell is used to call the user-supplied bisect run
>>> script is.  If we'd run it directly (without RUN_USING_SHELL) we could
>>> distinguish error code 126/127 from execution errors.  I assume the
>>> option is used to stay compatible with the old shell version of bisect.
>>
>> Sorry, my misunderstanding. I thought the external command was being
>> called from git-bisect.sh. I don't think I understand the purpose of
>> RUN_USING_SHELL (it just seems to put an "sh -c" in the beginning):
>>
>> 	static const char **prepare_shell_cmd(struct strvec *out, const char **argv)
>> 	{
>>         ...
>> 			strvec_push(out, "sh");
>> 			strvec_push(out, "-c");
>>
>> 			if (!argv[1])
>> 				strvec_push(out, argv[0]);
>> 			else
>> 				strvec_pushf(out, "%s \"$@\"", argv[0]);
>>        ...
>> 	}
>
> Using the shell allows the bisect run command to be any shell command,
> not just some script.  E.g. you could bisect a build failure with just
> "git bisect run make".  Quite useful.

True, but for example

	$ git bisect run make test

internally gets argv[] = { "make", "test", NULL } in bisect_run()
and then we are the one who make them into a single string, i.e.

	if (argc)
		sq_quote_argv(&command, argv);
	else {
		error(_("bisect run failed: no command provided."));
		return BISECT_FAILED;
	}

and that is what we run via the shell in the loop, i.e.

	while (1) {
		strvec_clear(&args);

		printf(_("running %s\n"), command.buf);
		res = run_command_v_opt(run_args.v, RUN_USING_SHELL);

I do not offhand recall the reason why we need to do that, instead
of using the original argv[] to invoke run_command_v_opt().

And my earlier "let's not go there" may need to be rethought.  I
somehow thought we are getting a single string from the end-user
and will become responsible for splitting it out or substituting
an environment variable with its value, but I was mistaken.