From: Junio C Hamano Date: Tue, 02 Aug 2011 23:16:54 GMT Subject: Re: [PATCH v14 5/8] bisect: introduce --no-checkout support into porcelain. Message-ID: <7vk4avcsk9.fsf@alter.siamese.dyndns.org> In-Reply-To: <7voc07ct9z.fsf@alter.siamese.dyndns.org> Junio C Hamano writes: > Jon Seymour 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). 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? - Further, perhaps should we default to this mode inside a bare repository? Christian, do you think of anything else? Also do you see flaws in our reasoning that updating only BISECT_HEAD and doing nothing else is a good way to do this?