Re: [PATCH v14 5/8] bisect: introduce --no-checkout support into porcelain.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 3, 2011 at 9:16 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> Jon Seymour <jon.seymour@xxxxxxxxx> writes:
>> ...
>>> +            if test "$BISECT_MODE" = "--no-checkout"; then
>>> +                git update-ref --no-deref HEAD "$start_head"
>>> +            else
>>> +                git checkout "$start_head" --
>>> +            fi
>>
>> Just a minor worry but I would not be surprised if somebody's "test"
>> implementation barfs upon:
>>
>>       test "--no-checkout" = "--no-checkout"
>>
>> mistaking the string with a dash at the beginning as an option unknown to
>> it. That is why we often have "z$variable" in our comparison, like so:
>>
>>       if test "z$BISECT_MODE" = "z--no-checkout"
>>         then
>>               git update-ref --no-deref BISECT_HEAD "$start_head"
>>       else
>>               git checkout "$start_head" --
>>       fi
>>
>>> -    git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
>>> +    git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES"
>>> +    echo "$BISECT_MODE" > "$GIT_DIR/BISECT_MODE" &&
>>
>> &&?
>
> Having said that, other than these minor nits, I think this round is
> almost ready. I didn't check how it behaves upon "bisect reset",
> though. It shouldn't touch the index, HEAD nor the working tree (it
> probably is just the matter of "update-ref -d BISECT_HEAD" and nothing
> else, but I haven't thought things through thoroughly).

That seems reasonable. In fact, none of these series properly cleaned
up the reset state properly, so I'll fix that and a test for it.

>
> Further polishing we may want to do while it is still in pu/next I can
> think of off the top of my head are:
>
>  - In this mode, I can bisect the history even inside a bare repository,
>   as the whole point of --no-checkout is that the mode does not require a
>   working tree. I however suspect "git bisect" requires working tree. Is
>   this something we want to fix?
>

I agree, that would be useful. Haven't tried it yet but I'll see what
happens.I may issue changes for this as separate commit that can be
squashed later, if required,  once it has been reviewed.

BTW: I'll squash v14 8/8 into the other commits, per Christian's suggestion.

>  - Further, perhaps should we default to this mode inside a bare
>   repository?

Seems reasonable.

jon.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]