Thread (27 messages) 27 messages, 3 authors, 2020-09-24

Re: [PATCH v7 00/13] Finish converting git bisect to C part 2

From: Miriam R. <hidden>
Date: 2020-09-24 12:53:33

Hi,

El jue., 24 sept. 2020 a las 12:46, Christian Couder
([off-list ref]) escribió:
Hi Dscho,

On Thu, Sep 24, 2020 at 12:06 PM Johannes Schindelin
[off-list ref] wrote:
quoted
On Thu, 24 Sep 2020, Christian Couder wrote:
quoted
On Wed, Sep 23, 2020 at 11:26 PM Johannes Schindelin
[off-list ref] wrote:
quoted
quoted
quoted
Instead, those `eval` calls are required because the arguments are
provided in quoted form. For example, during the execution of t6030.68,
the `eval` would expand the call

        eval "git bisect--helper --bisect-start $rev $tail"

to

        git bisect--helper --bisect-start '--term-old' 'term2' '--term-new' 'term1'
Yeah, that was also what I found (along with the bug I sent a patch for).
I suspected that you had found that out, but I really wanted a record on
the Git mailing list about our findings.

It might be a good idea to add a paragraph to the respective patches,
along these lines:

        Note that the `eval` in the changed line of `git-bisect.sh` cannot be
        dropped: it is necessary because the `rev` and the `tail`
        variables may contain multiple, quoted arguments that need to be
        passed to `bisect--helper` (without the quotes, naturally).
Yeah, sure. Hopefully Miriam will send this in the commit message of
the right patch which is in the subset of the patch series she hasn't
sent.
Ok. Noted!!
quoted
quoted
quoted
Therefore, the `eval` really needs to stay in place (also the other `eval`
I had originally suggested to remove, for the same reason).

I would still recommend appending `|| exit`, even if it just so happens
that we will eventually abort when the `bisect--helper` command failed
anyway, because the next command will then fail, and abort. But it's
cleaner to abort already when this invocation failed rather than relying
on that side effect.
Yeah, I think it's a good solution.
Excellent. I think we can actually move forward with the entire patch
series now, not just the first subset, right?
Yeah, I think so too.
Oops, I have already sent the first subset.

Best,
Miriam
Thanks,
Christian.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help