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

On Wed, Aug 3, 2011 at 9:16 AM, Junio C Hamano [off-list ref] wrote:

Junio C Hamano [off-list ref] writes:

quoted

Jon Seymour [off-list ref] writes:
...

quoted

+            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

quoted

-    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.