Browse files

bisect: introduce --no-checkout, --update-ref=<ref> support into porc…

…elain.

git-bisect can now perform bisection of a history without performing
a checkout at each stage of the bisection process.

Instead, the reference specified by <ref> is updated. If <ref> is not
specified, HEAD is used instead.

One use-case for this function is allow git bisect to be used with
damaged repositories where git checkout would fail because the tree
referenced by the commit is damaged.

It can also be used in other cases where actual checkout of the tree
is not required to progress the bisection.

Improved-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
  • Loading branch information...
1 parent 3ac5d92 commit 9eda7f9f63613363e6a4d8fd0a3d0c7ebbab5c36 @jonseymour committed Jul 31, 2011
Showing with 50 additions and 7 deletions.
  1. +50 −7 git-bisect.sh
View
57 git-bisect.sh
@@ -3,7 +3,7 @@
USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
LONG_USAGE='git bisect help
print this long help message.
-git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
+git bisect start [--no-checkout [--update-ref=<ref>]] [<bad> [<good>...]] [--] [<pathspec>...]
reset bisect state and start bisection.
git bisect bad [<rev>]
mark <rev> a known-bad revision.
@@ -34,6 +34,8 @@ require_work_tree
_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+BISECT_UPDATE_REF=$(test -f "$GIT_DIR/BISECT_UPDATE_REF" && cat "$GIT_DIR/BISECT_UPDATE_REF")
+
bisect_autostart() {
test -s "$GIT_DIR/BISECT_START" || {
(
@@ -59,6 +61,30 @@ bisect_autostart() {
}
bisect_start() {
+ no_checkout=
+ BISECT_UPDATE_REF=
+ for arg in "$@"; do
+ case "$arg" in
+ --no-checkout)
+ no_checkout=true ;;
+ --update-ref=*)
+ BISECT_UPDATE_REF=$arg ;;

I would prefer:

BISECT_UPDATE_REF=${arg#--update-ref=} ;;

It could avoid many ${BISECT_UPDATE_REF#--update-ref=} later.

Also it would be nice if the user could use a space between "--update-ref" and the ref.

@jonseymour
Owner

Have implemented the first suggestion.

With regard to second suggestion, you'd like to support:

--update-ref HEAD

and

--update-ref=HEAD

Is this pattern used elsewhere in git. If so, I am happy to implement it, if not, I'd prefer not to introduce a new idiom.

Yes it is implemented in git-web--browse.sh for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ --)
+ break; ;;
+ esac
+ done
+
+ if test -z "$no_checkout" -a -n "$BISECT_UPDATE_REF"
+ then
+ BISECT_UPDATE_REF=
+ gettext "warn: --update-ref has no effect unless --no-checkout is also specified." >&1
+ fi
+
+ if test -n "$no_checkout" -a -z "$BISECT_UPDATE_REF"
+ then
+ BISECT_UPDATE_REF=--update-ref=HEAD
+ fi
+
#
# Verify HEAD.
#
@@ -74,7 +100,11 @@ bisect_start() {
then
# Reset to the rev from where we started.
start_head=$(cat "$GIT_DIR/BISECT_START")
- git checkout "$start_head" -- || exit
+ if test -z "${BISECT_UPDATE_REF}"; then

Please use "$BISECT_UPDATE_REF" instead of "${BISECT_UPDATE_REF}".

@jonseymour
Owner

Fixed. Missed one instance in v6. Will be fixed in next iteration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ git checkout "$start_head" --
+ else
+ git update-ref --no-deref "${BISECT_UPDATE_REF#--update-ref=}" "$start_head"
+ fi
else
# Get rev from where we start.
case "$head" in
@@ -114,6 +144,13 @@ bisect_start() {
shift
break
;;
+ --update-ref=*|--no-checkout)

It looks like options are parsed twice. Once at the beginning of bisect_start() and once here. It would be better if that could be avoided.

Maybe you could move the following line:

orig_args=$(git rev-parse --sq-quote "$@")

at the beginning of the function. And then you could use "shift" when you first parse the arguments at the beginning of the function.

@jonseymour
Owner

Chris,

Moving the argument parsing forward is slightly trickier than it looks. In particular, there is one test that assumes that bisect_clean_state is called before argument parsing begins.

One might argue that it is correct behaviour to not change any state if the arguments don't parse. Anyway, I'll factor that change out as a separate commit.

@jonseymour
Owner

Ok.

Have done this and changed the test so that it doesn't expect state to be updated if argument parsing fails.

I will have a look at what you did. But basically, what I wanted to suggest is just to remove the "--no-checkout" and "--update-ref=" from the arguments during the first loop over the arguments. If there needs to be 2 loops over the arguments it's not too bad.

@jonseymour
Owner

Ok, the reason I needed the detection of --no-checkout early is that the existing code decides to do a checkout before parsing the arguments. I need to influence that decision, so I need at least --no-checkout and --update-ref parsed first.

That said, I can't see any reason to modify any state in the case that argument parsing fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ echo "${BISECT_UPDATE_REF}" > "$GIT_DIR/BISECT_UPDATE_REF"

We make sure to write state on disk at the end of the bisect_start() function. This line is breaking the rule.

@jonseymour
Owner

Thanks. I hadn't noticed this. Fixed in v6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ shift
+ ;;
+ --*)
+ die $(gettext "unrecognised option: $arg")
+ ;;
*)
rev=$(git rev-parse -q --verify "$arg^{commit}") || {
test $has_double_dash -eq 1 &&
@@ -291,7 +328,7 @@ bisect_next() {
bisect_next_check good
# Perform all bisection computation, display and checkout
- git bisect--helper --next-all
+ git bisect--helper --next-all "${BISECT_UPDATE_REF}"
res=$?
# Check if we should exit because bisection is finished
@@ -340,11 +377,16 @@ bisect_reset() {
*)
usage ;;
esac
- if git checkout "$branch" -- ; then
- bisect_clean_state
- else
- die "$(eval_gettext "Could not check out original HEAD '\$branch'.
+ if test -z "${BISECT_UPDATE_REF}"; then
+ if git checkout "$branch" --; then
+ bisect_clean_state
+ else
+ die "$(eval_gettext "Could not check out original HEAD '\$branch'.
Try 'git bisect reset <commit>'.")"
+ fi
+ else
+ ref=${BISECT_UPDATE_REF#--update-ref=}
+ git symbolic-ref "$ref" $(git rev-parse --symbolic-full-name "${branch}")
fi
}
@@ -360,6 +402,7 @@ bisect_clean_state() {
rm -f "$GIT_DIR/BISECT_LOG" &&
rm -f "$GIT_DIR/BISECT_NAMES" &&
rm -f "$GIT_DIR/BISECT_RUN" &&
+ rm -f "$GIT_DIR/BISECT_UPDATE_REF" &&
# Cleanup head-name if it got left by an old version of git-bisect
rm -f "$GIT_DIR/head-name" &&

0 comments on commit 9eda7f9

Please sign in to comment.