* [PATCH v13 1/8] bisect: move argument parsing before state modification.
2011-08-02 11:28 [PATCH v13 0/8] bisect: Add support for --no-checkout option Jon Seymour
@ 2011-08-02 11:28 ` Jon Seymour
2011-08-02 11:28 ` [PATCH v13 2/8] bisect: use && to connect statements that are deferred with eval Jon Seymour
` (6 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Jon Seymour @ 2011-08-02 11:28 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
Currently 'git bisect start' modifies some state prior to checking
that its arguments are valid.
This change moves argument validation before state modification
with the effect that state modification does not occur
unless argument validations succeeds.
An existing test is changed to check that new bisect state
is not created if arguments are invalid.
A new test is added to check that existing bisect state
is not modified if arguments are invalid.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
git-bisect.sh | 66 +++++++++++++++++++++---------------------
t/t6030-bisect-porcelain.sh | 14 +++++++--
2 files changed, 44 insertions(+), 36 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index b2186a8..20f6dd5 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -60,6 +60,39 @@ bisect_autostart() {
bisect_start() {
#
+ # Check for one bad and then some good revisions.
+ #
+ has_double_dash=0
+ for arg; do
+ case "$arg" in --) has_double_dash=1; break ;; esac
+ done
+ orig_args=$(git rev-parse --sq-quote "$@")
+ bad_seen=0
+ eval=''
+ while [ $# -gt 0 ]; do
+ arg="$1"
+ case "$arg" in
+ --)
+ shift
+ break
+ ;;
+ *)
+ rev=$(git rev-parse -q --verify "$arg^{commit}") || {
+ test $has_double_dash -eq 1 &&
+ die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
+ break
+ }
+ case $bad_seen in
+ 0) state='bad' ; bad_seen=1 ;;
+ *) state='good' ;;
+ esac
+ eval="$eval bisect_write '$state' '$rev' 'nolog'; "
+ shift
+ ;;
+ esac
+ done
+
+ #
# Verify HEAD.
#
head=$(GIT_DIR="$GIT_DIR" git symbolic-ref -q HEAD) ||
@@ -98,39 +131,6 @@ bisect_start() {
bisect_clean_state || exit
#
- # Check for one bad and then some good revisions.
- #
- has_double_dash=0
- for arg; do
- case "$arg" in --) has_double_dash=1; break ;; esac
- done
- orig_args=$(git rev-parse --sq-quote "$@")
- bad_seen=0
- eval=''
- while [ $# -gt 0 ]; do
- arg="$1"
- case "$arg" in
- --)
- shift
- break
- ;;
- *)
- rev=$(git rev-parse -q --verify "$arg^{commit}") || {
- test $has_double_dash -eq 1 &&
- die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
- break
- }
- case $bad_seen in
- 0) state='bad' ; bad_seen=1 ;;
- *) state='good' ;;
- esac
- eval="$eval bisect_write '$state' '$rev' 'nolog'; "
- shift
- ;;
- esac
- done
-
- #
# Change state.
# In case of mistaken revs or checkout error, or signals received,
# "bisect_auto_next" below may exit or misbehave.
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index b5063b6..b3d1b14 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -138,15 +138,23 @@ test_expect_success 'bisect start: back in good branch' '
grep "* other" branch.output > /dev/null
'
-test_expect_success 'bisect start: no ".git/BISECT_START" if junk rev' '
- git bisect start $HASH4 $HASH1 -- &&
- git bisect good &&
+test_expect_success 'bisect start: no ".git/BISECT_START" created if junk rev' '
+ git bisect reset &&
test_must_fail git bisect start $HASH4 foo -- &&
git branch > branch.output &&
grep "* other" branch.output > /dev/null &&
test_must_fail test -e .git/BISECT_START
'
+test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if junk rev' '
+ git bisect start $HASH4 $HASH1 -- &&
+ git bisect good &&
+ cp .git/BISECT_START saved &&
+ test_must_fail git bisect start $HASH4 foo -- &&
+ git branch > branch.output &&
+ grep "* (no branch)" branch.output > /dev/null &&
+ test_cmp saved .git/BISECT_START
+'
test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
git bisect start $HASH4 $HASH1 -- &&
git bisect good &&
--
1.7.6.353.g3461
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v13 2/8] bisect: use && to connect statements that are deferred with eval.
2011-08-02 11:28 [PATCH v13 0/8] bisect: Add support for --no-checkout option Jon Seymour
2011-08-02 11:28 ` [PATCH v13 1/8] bisect: move argument parsing before state modification Jon Seymour
@ 2011-08-02 11:28 ` Jon Seymour
2011-08-02 11:29 ` [PATCH v13 3/8] bisect: add tests to document expected behaviour in presence of broken trees Jon Seymour
` (5 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Jon Seymour @ 2011-08-02 11:28 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
Christian Couder pointed out that the existing eval strategy
swallows an initial non-zero return. Using && to connect
the statements should fix this.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
git-bisect.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index 20f6dd5..a44ffe1 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -86,7 +86,7 @@ bisect_start() {
0) state='bad' ; bad_seen=1 ;;
*) state='good' ;;
esac
- eval="$eval bisect_write '$state' '$rev' 'nolog'; "
+ eval="$eval bisect_write '$state' '$rev' 'nolog' &&"
shift
;;
esac
@@ -145,7 +145,7 @@ bisect_start() {
#
echo "$start_head" >"$GIT_DIR/BISECT_START" &&
git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
- eval "$eval" &&
+ eval "$eval true" &&
echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
#
# Check if we can proceed to the next bisect state.
--
1.7.6.353.g3461
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v13 3/8] bisect: add tests to document expected behaviour in presence of broken trees.
2011-08-02 11:28 [PATCH v13 0/8] bisect: Add support for --no-checkout option Jon Seymour
2011-08-02 11:28 ` [PATCH v13 1/8] bisect: move argument parsing before state modification Jon Seymour
2011-08-02 11:28 ` [PATCH v13 2/8] bisect: use && to connect statements that are deferred with eval Jon Seymour
@ 2011-08-02 11:29 ` Jon Seymour
2011-08-02 11:29 ` [PATCH v13 4/8] bisect: introduce support for --no-checkout option Jon Seymour
` (4 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Jon Seymour @ 2011-08-02 11:29 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
If the repo is broken, we expect bisect to fail.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
t/t6030-bisect-porcelain.sh | 48 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index b3d1b14..9ae2de8 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -581,5 +581,53 @@ test_expect_success 'erroring out when using bad path parameters' '
'
#
+# This creates a broken branch which cannot be checked out because
+# the tree created has been deleted.
#
+# H1-H2-H3-H4-H5-H6-H7 <--other
+# \
+# S5-S6'-S7'-S8'-S9 <--broken
+#
+# Commits marked with ' have a missing tree.
+#
+test_expect_success 'broken branch creation' '
+ git bisect reset &&
+ git checkout -b broken $HASH4 &&
+ git tag BROKEN_HASH4 $HASH4 &&
+ add_line_into_file "5(broken): first line on a broken branch" hello2 &&
+ git tag BROKEN_HASH5 &&
+ mkdir missing &&
+ :> missing/MISSING &&
+ git add missing/MISSING &&
+ git commit -m "6(broken): Added file that will be deleted"
+ git tag BROKEN_HASH6 &&
+ add_line_into_file "7(broken): second line on a broken branch" hello2 &&
+ git tag BROKEN_HASH7 &&
+ add_line_into_file "8(broken): third line on a broken branch" hello2 &&
+ git tag BROKEN_HASH8 &&
+ git rm missing/MISSING &&
+ git commit -m "9(broken): Remove missing file"
+ git tag BROKEN_HASH9 &&
+ rm .git/objects/39/f7e61a724187ab767d2e08442d9b6b9dab587d
+'
+
+echo "" > expected.ok
+cat > expected.missing-tree.default <<EOF
+fatal: unable to read tree 39f7e61a724187ab767d2e08442d9b6b9dab587d
+EOF
+
+test_expect_success 'bisect fails if tree is broken on start commit' '
+ git bisect reset &&
+ test_must_fail git bisect start BROKEN_HASH7 BROKEN_HASH4 2>error.txt &&
+ test_cmp expected.missing-tree.default error.txt
+'
+
+test_expect_success 'bisect fails if tree is broken on trial commit' '
+ git bisect reset &&
+ test_must_fail git bisect start BROKEN_HASH9 BROKEN_HASH4 2>error.txt &&
+ git reset --hard broken &&
+ git checkout broken &&
+ test_cmp expected.missing-tree.default error.txt
+'
+
test_done
--
1.7.6.353.g3461
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v13 4/8] bisect: introduce support for --no-checkout option.
2011-08-02 11:28 [PATCH v13 0/8] bisect: Add support for --no-checkout option Jon Seymour
` (2 preceding siblings ...)
2011-08-02 11:29 ` [PATCH v13 3/8] bisect: add tests to document expected behaviour in presence of broken trees Jon Seymour
@ 2011-08-02 11:29 ` Jon Seymour
2011-08-02 12:16 ` Christian Couder
2011-08-02 18:00 ` Junio C Hamano
2011-08-02 11:29 ` [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain Jon Seymour
` (3 subsequent siblings)
7 siblings, 2 replies; 23+ messages in thread
From: Jon Seymour @ 2011-08-02 11:29 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
If --no-checkout is specified, then the bisection process uses:
git update-ref --no-deref HEAD <trial>
at each trial instead of:
git checkout <trial>
Improved-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
bisect.c | 33 ++++++++++++++++++++++-----------
bisect.h | 2 +-
builtin/bisect--helper.c | 9 +++++++--
3 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/bisect.c b/bisect.c
index dd7e8ed..0427117 100644
--- a/bisect.c
+++ b/bisect.c
@@ -24,6 +24,7 @@ struct argv_array {
static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
+static const char *argv_update_ref[] = {"update-ref", "--no-deref", "HEAD", NULL, NULL};
/* bits #0-15 in revision.h */
@@ -707,16 +708,23 @@ static void mark_expected_rev(char *bisect_rev_hex)
die("closing file %s: %s", filename, strerror(errno));
}
-static int bisect_checkout(char *bisect_rev_hex)
+static int bisect_checkout(char *bisect_rev_hex, int no_checkout)
{
int res;
mark_expected_rev(bisect_rev_hex);
argv_checkout[2] = bisect_rev_hex;
- res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
- if (res)
- exit(res);
+ if (no_checkout) {
+ argv_update_ref[3] = bisect_rev_hex;
+ if (run_command_v_opt(argv_update_ref, RUN_GIT_CMD))
+ die("update-ref --no-deref HEAD failed on %s",
+ bisect_rev_hex);
+ } else {
+ res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
+ if (res)
+ exit(res);
+ }
argv_show_branch[1] = bisect_rev_hex;
return run_command_v_opt(argv_show_branch, RUN_GIT_CMD);
@@ -788,7 +796,7 @@ static void handle_skipped_merge_base(const unsigned char *mb)
* - If one is "skipped", we can't know but we should warn.
* - If we don't know, we should check it out and ask the user to test.
*/
-static void check_merge_bases(void)
+static void check_merge_bases(int no_checkout)
{
struct commit_list *result;
int rev_nr;
@@ -806,7 +814,7 @@ static void check_merge_bases(void)
handle_skipped_merge_base(mb);
} else {
printf("Bisecting: a merge base must be tested\n");
- exit(bisect_checkout(sha1_to_hex(mb)));
+ exit(bisect_checkout(sha1_to_hex(mb), no_checkout));
}
}
@@ -849,7 +857,7 @@ static int check_ancestors(const char *prefix)
* If a merge base must be tested by the user, its source code will be
* checked out to be tested by the user and we will exit.
*/
-static void check_good_are_ancestors_of_bad(const char *prefix)
+static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)
{
const char *filename = git_path("BISECT_ANCESTORS_OK");
struct stat st;
@@ -868,7 +876,7 @@ static void check_good_are_ancestors_of_bad(const char *prefix)
/* Check if all good revs are ancestor of the bad rev. */
if (check_ancestors(prefix))
- check_merge_bases();
+ check_merge_bases(no_checkout);
/* Create file BISECT_ANCESTORS_OK. */
fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);
@@ -908,8 +916,11 @@ static void show_diff_tree(const char *prefix, struct commit *commit)
* We use the convention that exiting with an exit code 10 means that
* the bisection process finished successfully.
* In this case the calling shell script should exit 0.
+ *
+ * If no_checkout is non-zero, the bisection process does not
+ * checkout the trial commit but instead simply updates HEAD.
*/
-int bisect_next_all(const char *prefix)
+int bisect_next_all(const char *prefix, int no_checkout)
{
struct rev_info revs;
struct commit_list *tried;
@@ -920,7 +931,7 @@ int bisect_next_all(const char *prefix)
if (read_bisect_refs())
die("reading bisect refs failed");
- check_good_are_ancestors_of_bad(prefix);
+ check_good_are_ancestors_of_bad(prefix, no_checkout);
bisect_rev_setup(&revs, prefix, "%s", "^%s", 1);
revs.limited = 1;
@@ -966,6 +977,6 @@ int bisect_next_all(const char *prefix)
"(roughly %d step%s)\n", nr, (nr == 1 ? "" : "s"),
steps, (steps == 1 ? "" : "s"));
- return bisect_checkout(bisect_rev_hex);
+ return bisect_checkout(bisect_rev_hex, no_checkout);
}
diff --git a/bisect.h b/bisect.h
index 0862ce5..22f2e4d 100644
--- a/bisect.h
+++ b/bisect.h
@@ -27,7 +27,7 @@ struct rev_list_info {
const char *header_prefix;
};
-extern int bisect_next_all(const char *prefix);
+extern int bisect_next_all(const char *prefix, int no_checkout);
extern int estimate_bisect_steps(int all);
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 5b22639..5cfac13 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -4,16 +4,20 @@
#include "bisect.h"
static const char * const git_bisect_helper_usage[] = {
- "git bisect--helper --next-all",
+ "git bisect--helper --next-all [--bisect-mode=checkout|update-ref]",
NULL
};
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
int next_all = 0;
+ int no_checkout = 0;
+ char *bisect_mode=NULL;
struct option options[] = {
OPT_BOOLEAN(0, "next-all", &next_all,
"perform 'git bisect next'"),
+ OPT_STRING(0, "bisect-mode", &bisect_mode, "mode",
+ "the bisection mode either checkout or update-ref. defaults to checkout."),
OPT_END()
};
@@ -23,6 +27,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
if (!next_all)
usage_with_options(git_bisect_helper_usage, options);
+ no_checkout = bisect_mode && !strcmp(bisect_mode, "update-ref");
/* next-all */
- return bisect_next_all(prefix);
+ return bisect_next_all(prefix, no_checkout);
}
--
1.7.6.353.g3461
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v13 4/8] bisect: introduce support for --no-checkout option.
2011-08-02 11:29 ` [PATCH v13 4/8] bisect: introduce support for --no-checkout option Jon Seymour
@ 2011-08-02 12:16 ` Christian Couder
2011-08-02 14:42 ` Jon Seymour
2011-08-02 18:00 ` Junio C Hamano
1 sibling, 1 reply; 23+ messages in thread
From: Christian Couder @ 2011-08-02 12:16 UTC (permalink / raw)
To: Jon Seymour; +Cc: git, chriscool, gitster, j6t, jnareb
On Tue, Aug 2, 2011 at 1:29 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
> If --no-checkout is specified, then the bisection process uses:
Yeah, but in this patch you are changing "git bisect--helper" by
adding the [--bisect-mode=checkout|update-ref] option. So it is
strange that you still talk about a --no-checkout option.
> git update-ref --no-deref HEAD <trial>
>
> at each trial instead of:
>
> git checkout <trial>
[...]
> int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> {
> int next_all = 0;
> + int no_checkout = 0;
> + char *bisect_mode=NULL;
> struct option options[] = {
> OPT_BOOLEAN(0, "next-all", &next_all,
> "perform 'git bisect next'"),
> + OPT_STRING(0, "bisect-mode", &bisect_mode, "mode",
> + "the bisection mode either checkout or update-ref. defaults to checkout."),
Nit: I would say : "bisection mode: 'checkout' (default) or 'update-ref'"
Thanks,
Christian.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 4/8] bisect: introduce support for --no-checkout option.
2011-08-02 12:16 ` Christian Couder
@ 2011-08-02 14:42 ` Jon Seymour
0 siblings, 0 replies; 23+ messages in thread
From: Jon Seymour @ 2011-08-02 14:42 UTC (permalink / raw)
To: Christian Couder; +Cc: git, chriscool, gitster, j6t, jnareb
On Tue, Aug 2, 2011 at 10:16 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> On Tue, Aug 2, 2011 at 1:29 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
>> If --no-checkout is specified, then the bisection process uses:
>
> Yeah, but in this patch you are changing "git bisect--helper" by
> adding the [--bisect-mode=checkout|update-ref] option. So it is
> strange that you still talk about a --no-checkout option.
>
>> git update-ref --no-deref HEAD <trial>
>>
>> at each trial instead of:
>>
>> git checkout <trial>
>
> [...]
>
>> int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>> {
>> int next_all = 0;
>> + int no_checkout = 0;
>> + char *bisect_mode=NULL;
>> struct option options[] = {
>> OPT_BOOLEAN(0, "next-all", &next_all,
>> "perform 'git bisect next'"),
>> + OPT_STRING(0, "bisect-mode", &bisect_mode, "mode",
>> + "the bisection mode either checkout or update-ref. defaults to checkout."),
>
> Nit: I would say : "bisection mode: 'checkout' (default) or 'update-ref'"
>
Thanks. Will address iboth comments s a future iteration.
jon.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 4/8] bisect: introduce support for --no-checkout option.
2011-08-02 11:29 ` [PATCH v13 4/8] bisect: introduce support for --no-checkout option Jon Seymour
2011-08-02 12:16 ` Christian Couder
@ 2011-08-02 18:00 ` Junio C Hamano
2011-08-02 21:24 ` Jon Seymour
1 sibling, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2011-08-02 18:00 UTC (permalink / raw)
To: Jon Seymour; +Cc: git, chriscool, j6t, jnareb
Jon Seymour <jon.seymour@gmail.com> writes:
> If --no-checkout is specified, then the bisection process uses:
>
> git update-ref --no-deref HEAD <trial>
>
> at each trial instead of:
>
> git checkout <trial>
> ...
Ok.
> int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> {
> int next_all = 0;
> + int no_checkout = 0;
> + char *bisect_mode=NULL;
> struct option options[] = {
> OPT_BOOLEAN(0, "next-all", &next_all,
> "perform 'git bisect next'"),
> + OPT_STRING(0, "bisect-mode", &bisect_mode, "mode",
> + "the bisection mode either checkout or update-ref. defaults to checkout."),
I think this is a regression from naming perspective from the previous
round. You would be either the normal (checkout) mode or no-checkout mode,
and honestly, --no-checkout would be understood by anybody while update-ref
would be understood only by Gitz.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 4/8] bisect: introduce support for --no-checkout option.
2011-08-02 18:00 ` Junio C Hamano
@ 2011-08-02 21:24 ` Jon Seymour
2011-08-02 22:05 ` Junio C Hamano
0 siblings, 1 reply; 23+ messages in thread
From: Jon Seymour @ 2011-08-02 21:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, chriscool, j6t, jnareb
On Wed, Aug 3, 2011 at 4:00 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jon Seymour <jon.seymour@gmail.com> writes:
>
>> If --no-checkout is specified, then the bisection process uses:
>>
>> git update-ref --no-deref HEAD <trial>
>>
>> at each trial instead of:
>>
>> git checkout <trial>
>> ...
>
> Ok.
>
>> int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>> {
>> int next_all = 0;
>> + int no_checkout = 0;
>> + char *bisect_mode=NULL;
>> struct option options[] = {
>> OPT_BOOLEAN(0, "next-all", &next_all,
>> "perform 'git bisect next'"),
>> + OPT_STRING(0, "bisect-mode", &bisect_mode, "mode",
>> + "the bisection mode either checkout or update-ref. defaults to checkout."),
>
> I think this is a regression from naming perspective from the previous
> round. You would be either the normal (checkout) mode or no-checkout mode,
> and honestly, --no-checkout would be understood by anybody while update-ref
> would be understood only by Gitz.
>
Is it relevant here that --bisect-mode here is on bisect--helper which
is really part of the plumbing and unlikely to be used by anything
other than git's own porcelain?
I did leave the porcelain option as --no-checkout.
I personally think the git-bisect.sh implementation which uses
BISECT_MODE internally is slightly simpler than the one that uses
BISECT_NO_CHECKOUT.
Are you ok if I keep roughly the v13 git-bisect.sh implementation, but
revert the bisect--helper.c to v11 (adjusting git-bisect.sh to
compensate)?
jon,
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 4/8] bisect: introduce support for --no-checkout option.
2011-08-02 21:24 ` Jon Seymour
@ 2011-08-02 22:05 ` Junio C Hamano
2011-08-02 22:19 ` Jon Seymour
0 siblings, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2011-08-02 22:05 UTC (permalink / raw)
To: Jon Seymour; +Cc: git, chriscool, j6t, jnareb
Jon Seymour <jon.seymour@gmail.com> writes:
>> I think this is a regression from naming perspective from the previous
>> round. You would be either the normal (checkout) mode or no-checkout mode,
>> and honestly, --no-checkout would be understood by anybody while update-ref
>> would be understood only by Gitz.
>>
>
> Is it relevant here that --bisect-mode here is on bisect--helper which
> is really part of the plumbing and unlikely to be used by anything
> other than git's own porcelain?
Ah, Ok, that is what I missed. I also was scratching my head after seeing
that there was not much change in the proposed log messages from the
earlier round.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 4/8] bisect: introduce support for --no-checkout option.
2011-08-02 22:05 ` Junio C Hamano
@ 2011-08-02 22:19 ` Jon Seymour
0 siblings, 0 replies; 23+ messages in thread
From: Jon Seymour @ 2011-08-02 22:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, chriscool, j6t, jnareb
On Wed, Aug 3, 2011 at 8:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jon Seymour <jon.seymour@gmail.com> writes:
>
>>> I think this is a regression from naming perspective from the previous
>>> round. You would be either the normal (checkout) mode or no-checkout mode,
>>> and honestly, --no-checkout would be understood by anybody while update-ref
>>> would be understood only by Gitz.
>>>
>>
>> Is it relevant here that --bisect-mode here is on bisect--helper which
>> is really part of the plumbing and unlikely to be used by anything
>> other than git's own porcelain?
>
> Ah, Ok, that is what I missed. I also was scratching my head after seeing
> that there was not much change in the proposed log messages from the
> earlier round.
>
>
I've issued a v14 that removes it - I was treating it as a boolean in
the C code anyway.
If anything v14 git-bisect.sh is slightly simpler than v13
jon,
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain.
2011-08-02 11:28 [PATCH v13 0/8] bisect: Add support for --no-checkout option Jon Seymour
` (3 preceding siblings ...)
2011-08-02 11:29 ` [PATCH v13 4/8] bisect: introduce support for --no-checkout option Jon Seymour
@ 2011-08-02 11:29 ` Jon Seymour
2011-08-02 12:04 ` Christian Couder
2011-08-02 11:29 ` [PATCH v13 6/8] bisect: add tests for the --no-checkout option Jon Seymour
` (2 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Jon Seymour @ 2011-08-02 11:29 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
git-bisect can now perform bisection of a history without performing
a checkout at each stage of the bisection process. Instead, HEAD is updated.
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>
---
git-bisect.sh | 32 +++++++++++++++++++++++++-------
1 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index a44ffe1..5f3c2c4 100755
--- a/git-bisect.sh
+++ b/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] [<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_MODE=$(test -f "$GIT_DIR/BISECT_MODE" && cat "$GIT_DIR/BISECT_MODE")
+
bisect_autostart() {
test -s "$GIT_DIR/BISECT_START" || {
(
@@ -69,6 +71,7 @@ bisect_start() {
orig_args=$(git rev-parse --sq-quote "$@")
bad_seen=0
eval=''
+ BISECT_MODE=checkout
while [ $# -gt 0 ]; do
arg="$1"
case "$arg" in
@@ -76,6 +79,11 @@ bisect_start() {
shift
break
;;
+ --no-checkout)
+ BISECT_MODE=update-ref;
+ shift ;;
+ --*)
+ die "$(eval_gettext "unrecognised option: '\$arg'")" ;;
*)
rev=$(git rev-parse -q --verify "$arg^{commit}") || {
test $has_double_dash -eq 1 &&
@@ -107,7 +115,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 "$BISECT_MODE" = "update-ref"; then
+ git update-ref --no-deref HEAD "$start_head"
+ else
+ git checkout "$start_head" --
+ fi
else
# Get rev from where we start.
case "$head" in
@@ -144,7 +156,8 @@ bisect_start() {
# Write new start state.
#
echo "$start_head" >"$GIT_DIR/BISECT_START" &&
- 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" &&
eval "$eval true" &&
echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
#
@@ -291,7 +304,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_MODE:+--bisect-mode=}${BISECT_MODE}
res=$?
# Check if we should exit because bisection is finished
@@ -340,11 +353,15 @@ bisect_reset() {
*)
usage ;;
esac
- if git checkout "$branch" -- ; then
- bisect_clean_state
+ if test "$BISECT_MODE" = "update-ref"; then
+ git symbolic-ref HEAD $(git rev-parse --symbolic-full-name "${branch}")
else
- die "$(eval_gettext "Could not check out original HEAD '\$branch'.
+ 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
fi
}
@@ -360,6 +377,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_MODE" &&
# Cleanup head-name if it got left by an old version of git-bisect
rm -f "$GIT_DIR/head-name" &&
--
1.7.6.353.g3461
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain.
2011-08-02 11:29 ` [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain Jon Seymour
@ 2011-08-02 12:04 ` Christian Couder
2011-08-02 14:41 ` Jon Seymour
0 siblings, 1 reply; 23+ messages in thread
From: Christian Couder @ 2011-08-02 12:04 UTC (permalink / raw)
To: Jon Seymour; +Cc: git, chriscool, gitster, j6t, jnareb
On Tue, Aug 2, 2011 at 1:29 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
> @@ -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_MODE=$(test -f "$GIT_DIR/BISECT_MODE" && cat "$GIT_DIR/BISECT_MODE")
Could you put this line just where it is needed, that is in
bisect_next() and bisect_reset()?
Thanks,
Christian.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain.
2011-08-02 12:04 ` Christian Couder
@ 2011-08-02 14:41 ` Jon Seymour
2011-08-03 5:27 ` Christian Couder
0 siblings, 1 reply; 23+ messages in thread
From: Jon Seymour @ 2011-08-02 14:41 UTC (permalink / raw)
To: Christian Couder; +Cc: git, chriscool, gitster, j6t, jnareb
On Tue, Aug 2, 2011 at 10:04 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> On Tue, Aug 2, 2011 at 1:29 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
>> @@ -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_MODE=$(test -f "$GIT_DIR/BISECT_MODE" && cat "$GIT_DIR/BISECT_MODE")
>
> Could you put this line just where it is needed, that is in
> bisect_next() and bisect_reset()?
>
Ultimately, it is also needed in paths that call bisect_state(), such
as bisect_run() and bisect_skip() so I am not keen to do this.
If I was to do this, I'd prefer to change uses of $BISECT_MODE with a
call to a function bisect_mode() that does the same thing.
jon.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain.
2011-08-02 14:41 ` Jon Seymour
@ 2011-08-03 5:27 ` Christian Couder
2011-08-03 12:46 ` Jon Seymour
0 siblings, 1 reply; 23+ messages in thread
From: Christian Couder @ 2011-08-03 5:27 UTC (permalink / raw)
To: Jon Seymour; +Cc: Christian Couder, git, gitster, j6t, jnareb
On Tuesday 02 August 2011 16:41:13 Jon Seymour wrote:
> On Tue, Aug 2, 2011 at 10:04 PM, Christian Couder
>
> <christian.couder@gmail.com> wrote:
> > On Tue, Aug 2, 2011 at 1:29 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
> >> @@ -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_MODE=$(test -f "$GIT_DIR/BISECT_MODE" && cat
> >> "$GIT_DIR/BISECT_MODE")
> >
> > Could you put this line just where it is needed, that is in
> > bisect_next() and bisect_reset()?
>
> Ultimately, it is also needed in paths that call bisect_state(), such
> as bisect_run() and bisect_skip() so I am not keen to do this.
>
> If I was to do this, I'd prefer to change uses of $BISECT_MODE with a
> call to a function bisect_mode() that does the same thing.
Yeah, I think it would be a good idea to have a bisect_mode() function.
I don't like very much to blindly call some code when we might not need it.
Thanks,
Christian.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain.
2011-08-03 5:27 ` Christian Couder
@ 2011-08-03 12:46 ` Jon Seymour
2011-08-03 13:24 ` Jon Seymour
0 siblings, 1 reply; 23+ messages in thread
From: Jon Seymour @ 2011-08-03 12:46 UTC (permalink / raw)
To: Christian Couder; +Cc: Christian Couder, git, gitster, j6t, jnareb
On Wed, Aug 3, 2011 at 3:27 PM, Christian Couder
<chriscool@tuxfamily.org> wrote:
> On Tuesday 02 August 2011 16:41:13 Jon Seymour wrote:
>> On Tue, Aug 2, 2011 at 10:04 PM, Christian Couder
>>
>> If I was to do this, I'd prefer to change uses of $BISECT_MODE with a
>> call to a function bisect_mode() that does the same thing.
>
> Yeah, I think it would be a good idea to have a bisect_mode() function.
> I don't like very much to blindly call some code when we might not need it.
>
Ok, I'll do that, though it does mean that during a bisect run
invocation, for example, the file BISECT_MODE file will be read log(N)
times instead of just once. Obviously, in the grand scheme of things
this is not a huge cost.
I could use a variable to avoid hitting the file each time, but if I
do that I am going to have to ensure it is blank to start with which
means I will need at least an assignment in the place where that
statement is now.
jon.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain.
2011-08-03 12:46 ` Jon Seymour
@ 2011-08-03 13:24 ` Jon Seymour
2011-08-03 14:09 ` Christian Couder
0 siblings, 1 reply; 23+ messages in thread
From: Jon Seymour @ 2011-08-03 13:24 UTC (permalink / raw)
To: Christian Couder; +Cc: Christian Couder, git, gitster, j6t, jnareb
On Wed, Aug 3, 2011 at 10:46 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
> On Wed, Aug 3, 2011 at 3:27 PM, Christian Couder
> <chriscool@tuxfamily.org> wrote:
>> On Tuesday 02 August 2011 16:41:13 Jon Seymour wrote:
>>> On Tue, Aug 2, 2011 at 10:04 PM, Christian Couder
>>>
>>> If I was to do this, I'd prefer to change uses of $BISECT_MODE with a
>>> call to a function bisect_mode() that does the same thing.
>>
>> Yeah, I think it would be a good idea to have a bisect_mode() function.
>> I don't like very much to blindly call some code when we might not need it.
>>
>
Mmmm.
Actually, there is a neater way to do this.
I'll such use the existence of BISECT_HEAD to inform the
implementation of bisect_mode().
This avoids the need for a separate .git/BISECT_MODE file.
jon.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain.
2011-08-03 13:24 ` Jon Seymour
@ 2011-08-03 14:09 ` Christian Couder
2011-08-03 15:13 ` Jon Seymour
0 siblings, 1 reply; 23+ messages in thread
From: Christian Couder @ 2011-08-03 14:09 UTC (permalink / raw)
To: Jon Seymour; +Cc: Christian Couder, git, gitster, j6t, jnareb
On Wed, Aug 3, 2011 at 3:24 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
> On Wed, Aug 3, 2011 at 10:46 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
>> On Wed, Aug 3, 2011 at 3:27 PM, Christian Couder
>> <chriscool@tuxfamily.org> wrote:
>>> On Tuesday 02 August 2011 16:41:13 Jon Seymour wrote:
>>>> On Tue, Aug 2, 2011 at 10:04 PM, Christian Couder
>>>>
>>>> If I was to do this, I'd prefer to change uses of $BISECT_MODE with a
>>>> call to a function bisect_mode() that does the same thing.
>>>
>>> Yeah, I think it would be a good idea to have a bisect_mode() function.
>>> I don't like very much to blindly call some code when we might not need it.
>>>
>>
> Mmmm.
>
> Actually, there is a neater way to do this.
>
> I'll such use the existence of BISECT_HEAD to inform the
> implementation of bisect_mode().
>
> This avoids the need for a separate .git/BISECT_MODE file.
Yeah, but then you have to be careful of the fact that BISECT_HEAD
might have not been properly deleted or might have been created by the
user for other purposes.
Thanks,
Christian.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain.
2011-08-03 14:09 ` Christian Couder
@ 2011-08-03 15:13 ` Jon Seymour
2011-08-03 19:41 ` Christian Couder
0 siblings, 1 reply; 23+ messages in thread
From: Jon Seymour @ 2011-08-03 15:13 UTC (permalink / raw)
To: Christian Couder; +Cc: Christian Couder, git, gitster, j6t, jnareb
On Thu, Aug 4, 2011 at 12:09 AM, Christian Couder
<christian.couder@gmail.com> wrote:
> On Wed, Aug 3, 2011 at 3:24 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
>> On Wed, Aug 3, 2011 at 10:46 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
>>> On Wed, Aug 3, 2011 at 3:27 PM, Christian Couder
>>> <chriscool@tuxfamily.org> wrote:
>>>> On Tuesday 02 August 2011 16:41:13 Jon Seymour wrote:
>>>>> On Tue, Aug 2, 2011 at 10:04 PM, Christian Couder
>>>>>
>>>>> If I was to do this, I'd prefer to change uses of $BISECT_MODE with a
>>>>> call to a function bisect_mode() that does the same thing.
>>>>
>>>> Yeah, I think it would be a good idea to have a bisect_mode() function.
>>>> I don't like very much to blindly call some code when we might not need it.
>>>>
>>>
>> Mmmm.
>>
>> Actually, there is a neater way to do this.
>>
>> I'll such use the existence of BISECT_HEAD to inform the
>> implementation of bisect_mode().
>>
>> This avoids the need for a separate .git/BISECT_MODE file.
>
> Yeah, but then you have to be careful of the fact that BISECT_HEAD
> might have not been properly deleted or might have been created by the
> user for other purposes.
>
I have removed $GIT_DIR/BISECT_MODE in v15.
If BISECT_HEAD was being used for other purposes, it is going to get
deleted anyway, irrespective of whether we have a separate BISECT_MODE
file, so I am not sure we need to consider that when deciding when we
need a separate BISECT_MODE file.
FWIW: bisect_mode() was only going to get called from one place so I
just inlined the implementation in that place. (on the call to
bisect--helper).
jon.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain.
2011-08-03 15:13 ` Jon Seymour
@ 2011-08-03 19:41 ` Christian Couder
0 siblings, 0 replies; 23+ messages in thread
From: Christian Couder @ 2011-08-03 19:41 UTC (permalink / raw)
To: Jon Seymour; +Cc: Christian Couder, git, gitster, j6t, jnareb
On Wednesday 03 August 2011 17:13:13 Jon Seymour wrote:
> On Thu, Aug 4, 2011 at 12:09 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
> > On Wed, Aug 3, 2011 at 3:24 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
> >> Mmmm.
> >>
> >> Actually, there is a neater way to do this.
> >>
> >> I'll such use the existence of BISECT_HEAD to inform the
> >> implementation of bisect_mode().
> >>
> >> This avoids the need for a separate .git/BISECT_MODE file.
> >
> > Yeah, but then you have to be careful of the fact that BISECT_HEAD
> > might have not been properly deleted or might have been created by the
> > user for other purposes.
>
> I have removed $GIT_DIR/BISECT_MODE in v15.
>
> If BISECT_HEAD was being used for other purposes, it is going to get
> deleted anyway, irrespective of whether we have a separate BISECT_MODE
> file, so I am not sure we need to consider that when deciding when we
> need a separate BISECT_MODE file.
Yeah, I was probably worrying too much.
> FWIW: bisect_mode() was only going to get called from one place so I
> just inlined the implementation in that place. (on the call to
> bisect--helper).
Ok.
Thanks,
Christian.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v13 6/8] bisect: add tests for the --no-checkout option.
2011-08-02 11:28 [PATCH v13 0/8] bisect: Add support for --no-checkout option Jon Seymour
` (4 preceding siblings ...)
2011-08-02 11:29 ` [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain Jon Seymour
@ 2011-08-02 11:29 ` Jon Seymour
2011-08-02 11:29 ` [PATCH v13 7/8] bisect: add documentation for " Jon Seymour
2011-08-02 11:29 ` [PATCH v13 8/8] bisect: change bisect function to update BISECT_HEAD, rather than HEAD Jon Seymour
7 siblings, 0 replies; 23+ messages in thread
From: Jon Seymour @ 2011-08-02 11:29 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
These tests verify that git-bisect --no-checkout can successfully
bisect commit histories that reference damaged trees.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
t/t6030-bisect-porcelain.sh | 70 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 70 insertions(+), 0 deletions(-)
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 9ae2de8..a1e0ddc 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -616,6 +616,14 @@ cat > expected.missing-tree.default <<EOF
fatal: unable to read tree 39f7e61a724187ab767d2e08442d9b6b9dab587d
EOF
+check_same()
+{
+ echo "Checking $1 is the same as $2" &&
+ git rev-parse "$1" > expected.same &&
+ git rev-parse "$2" > expected.actual &&
+ test_cmp expected.same expected.actual
+}
+
test_expect_success 'bisect fails if tree is broken on start commit' '
git bisect reset &&
test_must_fail git bisect start BROKEN_HASH7 BROKEN_HASH4 2>error.txt &&
@@ -630,4 +638,66 @@ test_expect_success 'bisect fails if tree is broken on trial commit' '
test_cmp expected.missing-tree.default error.txt
'
+test_expect_success 'bisect: --no-checkout - start commit bad' '
+ git bisect reset &&
+ git bisect start BROKEN_HASH7 BROKEN_HASH4 --no-checkout &&
+ check_same BROKEN_HASH6 HEAD &&
+ git bisect reset
+'
+
+test_expect_success 'bisect: --no-checkout - trial commit bad' '
+ git bisect reset &&
+ git bisect start broken BROKEN_HASH4 --no-checkout &&
+ check_same BROKEN_HASH6 HEAD &&
+ git bisect reset
+'
+
+test_expect_success 'bisect: --no-checkout - target before breakage' '
+ git bisect reset &&
+ git bisect start broken BROKEN_HASH4 --no-checkout &&
+ check_same BROKEN_HASH6 HEAD &&
+ git bisect bad HEAD &&
+ check_same BROKEN_HASH5 HEAD &&
+ git bisect bad HEAD &&
+ check_same BROKEN_HASH5 bisect/bad &&
+ git bisect reset
+'
+
+test_expect_success 'bisect: --no-checkout - target in breakage' '
+ git bisect reset &&
+ git bisect start broken BROKEN_HASH4 --no-checkout &&
+ check_same BROKEN_HASH6 HEAD &&
+ git bisect bad HEAD &&
+ check_same BROKEN_HASH5 HEAD &&
+ git bisect good HEAD &&
+ check_same BROKEN_HASH6 bisect/bad &&
+ git bisect reset
+'
+
+test_expect_success 'bisect: --no-checkout - target after breakage' '
+ git bisect reset &&
+ git bisect start broken BROKEN_HASH4 --no-checkout &&
+ check_same BROKEN_HASH6 HEAD &&
+ git bisect good HEAD &&
+ check_same BROKEN_HASH8 HEAD &&
+ git bisect good HEAD &&
+ check_same BROKEN_HASH9 bisect/bad &&
+ git bisect reset
+'
+
+test_expect_success 'bisect: demonstrate identification of damage boundary' "
+ git bisect reset &&
+ git checkout broken &&
+ git bisect start broken master --no-checkout &&
+ git bisect run eval '
+rc=1;
+if git rev-list --objects HEAD >tmp.$$; then
+ git pack-objects --stdout >/dev/null < tmp.$$ && rc=0;
+fi;
+rm tmp.$$;
+test \$rc -eq 0;' &&
+ check_same BROKEN_HASH6 bisect/bad &&
+ git bisect reset
+"
+
test_done
--
1.7.6.353.g3461
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v13 7/8] bisect: add documentation for --no-checkout option.
2011-08-02 11:28 [PATCH v13 0/8] bisect: Add support for --no-checkout option Jon Seymour
` (5 preceding siblings ...)
2011-08-02 11:29 ` [PATCH v13 6/8] bisect: add tests for the --no-checkout option Jon Seymour
@ 2011-08-02 11:29 ` Jon Seymour
2011-08-02 11:29 ` [PATCH v13 8/8] bisect: change bisect function to update BISECT_HEAD, rather than HEAD Jon Seymour
7 siblings, 0 replies; 23+ messages in thread
From: Jon Seymour @ 2011-08-02 11:29 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
Documentation/git-bisect.txt | 32 +++++++++++++++++++++++++++++++-
1 files changed, 31 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index ab60a18..2014894 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending
on the subcommand:
git bisect help
- git bisect start [<bad> [<good>...]] [--] [<paths>...]
+ git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<paths>...]
git bisect bad [<rev>]
git bisect good [<rev>...]
git bisect skip [(<rev>|<range>)...]
@@ -263,6 +263,17 @@ rewind the tree to the pristine state. Finally the script should exit
with the status of the real test to let the "git bisect run" command loop
determine the eventual outcome of the bisect session.
+OPTIONS
+-------
+--no-checkout::
++
+This option is used to specify that 'git bisect' should not modify the working
+tree or index on each iteration of the bisection process but should
+update HEAD instead.
++
+This option is useful in circumstances in which checkout is either not
+possible (because of a damaged respository) or is otherwise not required.
+
EXAMPLES
--------
@@ -343,6 +354,25 @@ $ git bisect run sh -c "make || exit 125; ~/check_test_case.sh"
This shows that you can do without a run script if you write the test
on a single line.
+* Locate a good region of the object graph in a damaged repository
++
+------------
+$ git bisect start HEAD <known-good-commit> [ <boundary-commit> ... ] --no-checkout
+$ git bisect run eval '
+rc=1;
+if git rev-list --objects HEAD >tmp.$$; then
+ git pack-objects --stdout >/dev/null < tmp.$$ && rc=0;
+fi;
+rm tmp.$$;
+test $rc -eq 0;'
+
+------------
++
+In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit that
+has at least one parent whose reachable graph is fully traversable in the sense
+required by 'git pack objects'.
+
+
SEE ALSO
--------
link:git-bisect-lk2009.html[Fighting regressions with git bisect],
--
1.7.6.353.g3461
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v13 8/8] bisect: change bisect function to update BISECT_HEAD, rather than HEAD.
2011-08-02 11:28 [PATCH v13 0/8] bisect: Add support for --no-checkout option Jon Seymour
` (6 preceding siblings ...)
2011-08-02 11:29 ` [PATCH v13 7/8] bisect: add documentation for " Jon Seymour
@ 2011-08-02 11:29 ` Jon Seymour
7 siblings, 0 replies; 23+ messages in thread
From: Jon Seymour @ 2011-08-02 11:29 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
This function modifies git-bisect so that the --no-checkout option
uses BISECT_HEAD rather than HEAD to record the current bisection
head.
The intent is to reduce the confusion that uses of --no-checkout
may experience when using the --no-checkout option since the bisection
process will no longer introduce spurious differences between the
HEAD reference and the working tree and index.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
Documentation/git-bisect.txt | 8 ++++----
bisect.c | 2 +-
git-bisect.sh | 17 +++++++++++++----
t/t6030-bisect-porcelain.sh | 30 +++++++++++++++---------------
4 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 2014894..a9b217b 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -267,9 +267,9 @@ OPTIONS
-------
--no-checkout::
+
-This option is used to specify that 'git bisect' should not modify the working
-tree or index on each iteration of the bisection process but should
-update HEAD instead.
+This option is used to specify that 'git bisect' should not checkout the
+new working tree at each iteration of the bisection process but should
+instead update BISECT_HEAD.
+
This option is useful in circumstances in which checkout is either not
possible (because of a damaged respository) or is otherwise not required.
@@ -360,7 +360,7 @@ on a single line.
$ git bisect start HEAD <known-good-commit> [ <boundary-commit> ... ] --no-checkout
$ git bisect run eval '
rc=1;
-if git rev-list --objects HEAD >tmp.$$; then
+if git rev-list --objects BISECT_HEAD >tmp.$$; then
git pack-objects --stdout >/dev/null < tmp.$$ && rc=0;
fi;
rm tmp.$$;
diff --git a/bisect.c b/bisect.c
index 0427117..46874be 100644
--- a/bisect.c
+++ b/bisect.c
@@ -24,7 +24,7 @@ struct argv_array {
static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
-static const char *argv_update_ref[] = {"update-ref", "--no-deref", "HEAD", NULL, NULL};
+static const char *argv_update_ref[] = {"update-ref", "--no-deref", "BISECT_HEAD", NULL, NULL};
/* bits #0-15 in revision.h */
diff --git a/git-bisect.sh b/git-bisect.sh
index 5f3c2c4..c9aabc0 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -36,6 +36,15 @@ _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
BISECT_MODE=$(test -f "$GIT_DIR/BISECT_MODE" && cat "$GIT_DIR/BISECT_MODE")
+bisect_head()
+{
+ if test "$BISECT_MODE" = "update-ref"; then
+ echo BISECT_HEAD;
+ else
+ echo HEAD
+ fi
+}
+
bisect_autostart() {
test -s "$GIT_DIR/BISECT_START" || {
(
@@ -116,7 +125,7 @@ bisect_start() {
# Reset to the rev from where we started.
start_head=$(cat "$GIT_DIR/BISECT_START")
if test "$BISECT_MODE" = "update-ref"; then
- git update-ref --no-deref HEAD "$start_head"
+ git update-ref --no-deref $(bisect_head) "$start_head"
else
git checkout "$start_head" --
fi
@@ -219,8 +228,8 @@ bisect_state() {
0,*)
die "$(gettext "Please call 'bisect_state' with at least one argument.")" ;;
1,bad|1,good|1,skip)
- rev=$(git rev-parse --verify HEAD) ||
- die "$(gettext "Bad rev input: HEAD")"
+ rev=$(git rev-parse --verify $(bisect_head)) ||
+ die "$(gettext "Bad rev input: $(bisect_head)")"
bisect_write "$state" "$rev"
check_expected_revs "$rev" ;;
2,bad|*,good|*,skip)
@@ -354,7 +363,7 @@ bisect_reset() {
usage ;;
esac
if test "$BISECT_MODE" = "update-ref"; then
- git symbolic-ref HEAD $(git rev-parse --symbolic-full-name "${branch}")
+ git symbolic-ref $(bisect_head) $(git rev-parse --symbolic-full-name "${branch}")
else
if git checkout "$branch" --; then
bisect_clean_state
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index a1e0ddc..ef422a1 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -641,24 +641,24 @@ test_expect_success 'bisect fails if tree is broken on trial commit' '
test_expect_success 'bisect: --no-checkout - start commit bad' '
git bisect reset &&
git bisect start BROKEN_HASH7 BROKEN_HASH4 --no-checkout &&
- check_same BROKEN_HASH6 HEAD &&
+ check_same BROKEN_HASH6 BISECT_HEAD &&
git bisect reset
'
test_expect_success 'bisect: --no-checkout - trial commit bad' '
git bisect reset &&
git bisect start broken BROKEN_HASH4 --no-checkout &&
- check_same BROKEN_HASH6 HEAD &&
+ check_same BROKEN_HASH6 BISECT_HEAD &&
git bisect reset
'
test_expect_success 'bisect: --no-checkout - target before breakage' '
git bisect reset &&
git bisect start broken BROKEN_HASH4 --no-checkout &&
- check_same BROKEN_HASH6 HEAD &&
- git bisect bad HEAD &&
- check_same BROKEN_HASH5 HEAD &&
- git bisect bad HEAD &&
+ check_same BROKEN_HASH6 BISECT_HEAD &&
+ git bisect bad BISECT_HEAD &&
+ check_same BROKEN_HASH5 BISECT_HEAD &&
+ git bisect bad BISECT_HEAD &&
check_same BROKEN_HASH5 bisect/bad &&
git bisect reset
'
@@ -666,10 +666,10 @@ test_expect_success 'bisect: --no-checkout - target before breakage' '
test_expect_success 'bisect: --no-checkout - target in breakage' '
git bisect reset &&
git bisect start broken BROKEN_HASH4 --no-checkout &&
- check_same BROKEN_HASH6 HEAD &&
- git bisect bad HEAD &&
- check_same BROKEN_HASH5 HEAD &&
- git bisect good HEAD &&
+ check_same BROKEN_HASH6 BISECT_HEAD &&
+ git bisect bad BISECT_HEAD &&
+ check_same BROKEN_HASH5 BISECT_HEAD &&
+ git bisect good BISECT_HEAD &&
check_same BROKEN_HASH6 bisect/bad &&
git bisect reset
'
@@ -677,10 +677,10 @@ test_expect_success 'bisect: --no-checkout - target in breakage' '
test_expect_success 'bisect: --no-checkout - target after breakage' '
git bisect reset &&
git bisect start broken BROKEN_HASH4 --no-checkout &&
- check_same BROKEN_HASH6 HEAD &&
- git bisect good HEAD &&
- check_same BROKEN_HASH8 HEAD &&
- git bisect good HEAD &&
+ check_same BROKEN_HASH6 BISECT_HEAD &&
+ git bisect good BISECT_HEAD &&
+ check_same BROKEN_HASH8 BISECT_HEAD &&
+ git bisect good BISECT_HEAD &&
check_same BROKEN_HASH9 bisect/bad &&
git bisect reset
'
@@ -691,7 +691,7 @@ test_expect_success 'bisect: demonstrate identification of damage boundary' "
git bisect start broken master --no-checkout &&
git bisect run eval '
rc=1;
-if git rev-list --objects HEAD >tmp.$$; then
+if git rev-list --objects BISECT_HEAD >tmp.$$; then
git pack-objects --stdout >/dev/null < tmp.$$ && rc=0;
fi;
rm tmp.$$;
--
1.7.6.353.g3461
^ permalink raw reply related [flat|nested] 23+ messages in thread