* [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c
@ 2022-12-15 9:47 Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 1/6] bisect--helper: simplify exit code computation Ævar Arnfjörð Bjarmason
` (7 more replies)
0 siblings, 8 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-15 9:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
The js/bisect-in-c topic got replaced by dd/bisect-helper-subcommand
and later dd/git-bisect-builtin to fix a regression in "bisect".
This small set of fixes is a cherry-pick of various miscellanious
fixes that were part of js/bisect-in-c that are still worth
having. Johannes and I have coordinated my submission of this
follow-up topic off-list.
The only "new" commit here is the 2/6. It's a cherry-pick of
Johannes's commit to do the same, but as the change to the base was so
extensive (the range diff won't even pick it up anymore with the
default --creation-factor) he suggested I change the authorship, which
I've done here.
There's still other stuff worth picking up from js/bisect-in-c. In
particular we should be using parse_options() for the various
sub-commands of "bisect". But as those changes had much more extensive
conflicts let's leave them for later.
The range-diff here is to pr-1132/dscho/bisect-in-c-v6[1]. My own
branch & CI for this is at [2].
1. https://lore.kernel.org/git/pull.1132.v6.git.1661885419.gitgitgadget@gmail.com/
2. https://github.com/avar/git/tree/avar-js/bisect-in-c-rebased
Johannes Schindelin (5):
bisect--helper: simplify exit code computation
bisect: verify that a bogus option won't try to start a bisection
bisect run: fix the error message
bisect: remove Cogito-related code
bisect: no longer try to clean up left-over `.git/head-name` files
Ævar Arnfjörð Bjarmason (1):
bisect--helper: make the order consistently `argc, argv`
bisect.c | 3 ---
builtin/bisect.c | 52 ++++++++++++++-----------------------
t/t6030-bisect-porcelain.sh | 21 ++++++++++++++-
3 files changed, 40 insertions(+), 36 deletions(-)
Range-diff:
1: 05262b6a7d1 < -: ----------- bisect--helper: retire the --no-log option
2: 1e43148864a < -: ----------- bisect--helper: really retire --bisect-next-check
3: 1a1649d9d0d < -: ----------- bisect--helper: really retire `--bisect-autostart`
4: 9ab30552c6a ! 1: c8c648e4b8c bisect--helper: simplify exit code computation
@@ Commit message
Let's use it instead of duplicating the logic.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- ## builtin/bisect--helper.c ##
-@@ builtin/bisect--helper.c: int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
+ ## builtin/bisect.c ##
+@@ builtin/bisect.c: int cmd_bisect(int argc, const char **argv, const char *prefix)
+ res = fn(argc, argv, prefix);
}
- free_terms(&terms);
- /*
- * Handle early success
5: 92b3b116ef8 < -: ----------- bisect--helper: make `terms` an explicit singleton
6: c9dc0281e38 < -: ----------- bisect--helper: make the order consistently `argc, argv`
7: e97e187bbec < -: ----------- bisect--helper: migrate to OPT_SUBCOMMAND()
-: ----------- > 2: a0de7ad6836 bisect--helper: make the order consistently `argc, argv`
8: 30c87f2e92e ! 3: e1e31278fef bisect: verify that a bogus option won't try to start a bisection
@@ Commit message
by "git bisect start"` and fail if it was found.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t6030-bisect-porcelain.sh ##
@@ t/t6030-bisect-porcelain.sh: test_expect_success 'bisect start with one term1 and term2' '
9: 4696652b99c ! 4: 59a8a3085b1 bisect run: fix the error message
@@ Commit message
Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- ## builtin/bisect--helper.c ##
-@@ builtin/bisect--helper.c: static int cmd_bisect_run(int argc, const char **argv, const char *prefix)
- printf(_("bisect found first bad commit"));
+ ## builtin/bisect.c ##
+@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
+ puts(_("bisect found first bad commit"));
res = BISECT_OK;
} else if (res) {
-- error(_("bisect run failed: 'git bisect--helper --bisect-state"
-+ error(_("bisect run failed: 'git bisect"
- " %s' exited with error code %d"), new_state, res);
+- error(_("bisect run failed: 'bisect-state %s'"
++ error(_("bisect run failed: 'git bisect %s'"
+ " exited with error code %d"), new_state, res);
} else {
continue;
10: b202a0e386c < -: ----------- bisect: avoid double-quoting when printing the failed command
11: 3376b450867 < -: ----------- bisect--helper: calling `bisect_state()` without an argument is a bug
12: e7623508f90 < -: ----------- bisect--helper: make `state` optional
13: 3f052580c95 < -: ----------- bisect: move even the command-line parsing to `bisect--helper`
14: a83fe3dc3c2 < -: ----------- Turn `git bisect` into a full built-in
15: f2132b61ff7 ! 5: 1b70cd79cae bisect: remove Cogito-related code
@@ Commit message
remove the last remnant of Cogito-accommodating code.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## builtin/bisect.c ##
@@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
@@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXP
static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
-@@ builtin/bisect.c: static int cmd_bisect_start(int argc, const char **argv, const char *prefix)
+@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
strbuf_addstr(&start_head, oid_to_hex(&head_oid));
} else if (!get_oid(head, &head_oid) &&
skip_prefix(head, "refs/heads/", &head)) {
16: 4f93692e071 ! 6: 2ad89aca728 bisect: no longer try to clean up left-over `.git/head-name` files
@@ Commit message
So let's remove that code, at long last.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## bisect.c ##
@@ bisect.c: static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
--
2.39.0.rc2.1048.g0e5493b8d5b
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/6] bisect--helper: simplify exit code computation
2022-12-15 9:47 [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c Ævar Arnfjörð Bjarmason
@ 2022-12-15 9:47 ` Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 2/6] bisect--helper: make the order consistently `argc, argv` Ævar Arnfjörð Bjarmason
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-15 9:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh, Johannes Schindelin,
Ævar Arnfjörð Bjarmason
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We _already_ have a function to determine whether a given `enum
bisect_error` value is non-zero but still _actually_ indicates success.
Let's use it instead of duplicating the logic.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/bisect.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c
index cc9483e8515..09505fc4dce 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -1440,12 +1440,5 @@ int cmd_bisect(int argc, const char **argv, const char *prefix)
res = fn(argc, argv, prefix);
}
- /*
- * Handle early success
- * From check_merge_bases > check_good_are_ancestors_of_bad > bisect_next_all
- */
- if ((res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) || (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND))
- res = BISECT_OK;
-
- return -res;
+ return is_bisect_success(res) ? 0 : -res;
}
--
2.39.0.rc2.1048.g0e5493b8d5b
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/6] bisect--helper: make the order consistently `argc, argv`
2022-12-15 9:47 [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 1/6] bisect--helper: simplify exit code computation Ævar Arnfjörð Bjarmason
@ 2022-12-15 9:47 ` Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 3/6] bisect: verify that a bogus option won't try to start a bisection Ævar Arnfjörð Bjarmason
` (5 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-15 9:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason, Johannes Schindelin
In C, the natural order is for `argc` to come before `argv` by virtue of
the `main()` function declaring the parameters in precisely that order.
It is confusing & distracting, then, when readers familiar with the C
language read code where that order is switched around.
Let's just change the order and avoid that type of developer friction.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/bisect.c | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 09505fc4dce..9fc8db06944 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -678,7 +678,8 @@ static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char
return bisect_next(terms, prefix);
}
-static enum bisect_error bisect_start(struct bisect_terms *terms, const char **argv, int argc)
+static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
+ const char **argv)
{
int no_checkout = 0;
int first_parent_only = 0;
@@ -908,13 +909,13 @@ static int bisect_autostart(struct bisect_terms *terms)
yesno = git_prompt(_("Do you want me to do it for you "
"[Y/n]? "), PROMPT_ECHO);
res = tolower(*yesno) == 'n' ?
- -1 : bisect_start(terms, empty_strvec, 0);
+ -1 : bisect_start(terms, 0, empty_strvec);
return res;
}
-static enum bisect_error bisect_state(struct bisect_terms *terms, const char **argv,
- int argc)
+static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
+ const char **argv)
{
const char *state;
int i, verify_expected = 1;
@@ -1033,7 +1034,7 @@ static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)
struct strvec argv = STRVEC_INIT;
int res;
sq_dequote_to_strvec(rev, &argv);
- res = bisect_start(terms, argv.v, argv.nr);
+ res = bisect_start(terms, argv.nr, argv.v);
strvec_clear(&argv);
return res;
}
@@ -1083,7 +1084,8 @@ static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f
return bisect_auto_next(terms, NULL);
}
-static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **argv, int argc)
+static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc,
+ const char **argv)
{
int i;
enum bisect_error res;
@@ -1113,13 +1115,14 @@ static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **ar
strvec_push(&argv_state, argv[i]);
}
}
- res = bisect_state(terms, argv_state.v, argv_state.nr);
+ res = bisect_state(terms, argv_state.nr, argv_state.v);
strvec_clear(&argv_state);
return res;
}
-static int bisect_visualize(struct bisect_terms *terms, const char **argv, int argc)
+static int bisect_visualize(struct bisect_terms *terms, int argc,
+ const char **argv)
{
struct child_process cmd = CHILD_PROCESS_INIT;
struct strbuf sb = STRBUF_INIT;
@@ -1202,7 +1205,7 @@ static int verify_good(const struct bisect_terms *terms, const char *command)
return rc;
}
-static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
+static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
{
int res = BISECT_OK;
struct strbuf command = STRBUF_INIT;
@@ -1271,7 +1274,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
saved_stdout = dup(1);
dup2(temporary_stdout_fd, 1);
- res = bisect_state(terms, &new_state, 1);
+ res = bisect_state(terms, 1, &new_state);
fflush(stdout);
dup2(saved_stdout, 1);
@@ -1328,7 +1331,7 @@ static int cmd_bisect__start(int argc, const char **argv, const char *prefix UNU
struct bisect_terms terms = { 0 };
set_terms(&terms, "bad", "good");
- res = bisect_start(&terms, argv, argc);
+ res = bisect_start(&terms, argc, argv);
free_terms(&terms);
return res;
}
@@ -1372,7 +1375,7 @@ static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUS
set_terms(&terms, "bad", "good");
get_terms(&terms);
- res = bisect_skip(&terms, argv, argc);
+ res = bisect_skip(&terms, argc, argv);
free_terms(&terms);
return res;
}
@@ -1383,7 +1386,7 @@ static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix
struct bisect_terms terms = { 0 };
get_terms(&terms);
- res = bisect_visualize(&terms, argv, argc);
+ res = bisect_visualize(&terms, argc, argv);
free_terms(&terms);
return res;
}
@@ -1396,7 +1399,7 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE
if (!argc)
return error(_("'%s' failed: no command provided."), "git bisect run");
get_terms(&terms);
- res = bisect_run(&terms, argv, argc);
+ res = bisect_run(&terms, argc, argv);
free_terms(&terms);
return res;
}
@@ -1432,7 +1435,7 @@ int cmd_bisect(int argc, const char **argv, const char *prefix)
if (check_and_set_terms(&terms, argv[0]))
usage_msg_optf(_("unknown command: '%s'"), git_bisect_usage,
options, argv[0]);
- res = bisect_state(&terms, argv, argc);
+ res = bisect_state(&terms, argc, argv);
free_terms(&terms);
} else {
argc--;
--
2.39.0.rc2.1048.g0e5493b8d5b
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/6] bisect: verify that a bogus option won't try to start a bisection
2022-12-15 9:47 [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 1/6] bisect--helper: simplify exit code computation Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 2/6] bisect--helper: make the order consistently `argc, argv` Ævar Arnfjörð Bjarmason
@ 2022-12-15 9:47 ` Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 4/6] bisect run: fix the error message Ævar Arnfjörð Bjarmason
` (4 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-15 9:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh, Johannes Schindelin,
Ævar Arnfjörð Bjarmason
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We do not want `git bisect --bogus-option` to start a bisection. To
verify that, we look for the tell-tale error message `You need to start
by "git bisect start"` and fail if it was found.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t6030-bisect-porcelain.sh | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 98a72ff78a7..9e56b42b5da 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -1058,6 +1058,16 @@ test_expect_success 'bisect start with one term1 and term2' '
git bisect reset
'
+test_expect_success 'bogus command does not start bisect' '
+ git bisect reset &&
+ test_must_fail git bisect --bisect-terms 1 2 2>out &&
+ ! grep "You need to start" out &&
+ test_must_fail git bisect --bisect-terms 2>out &&
+ ! grep "You need to start" out &&
+ grep "git bisect.*visualize" out &&
+ git bisect reset
+'
+
test_expect_success 'bisect replay with term1 and term2' '
git bisect replay log_to_replay.txt >bisect_result &&
grep "$HASH2 is the first term1 commit" bisect_result &&
--
2.39.0.rc2.1048.g0e5493b8d5b
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/6] bisect run: fix the error message
2022-12-15 9:47 [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c Ævar Arnfjörð Bjarmason
` (2 preceding siblings ...)
2022-12-15 9:47 ` [PATCH 3/6] bisect: verify that a bogus option won't try to start a bisection Ævar Arnfjörð Bjarmason
@ 2022-12-15 9:47 ` Ævar Arnfjörð Bjarmason
2022-12-15 15:08 ` Đoàn Trần Công Danh
2022-12-15 9:47 ` [PATCH 5/6] bisect: remove Cogito-related code Ævar Arnfjörð Bjarmason
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-15 9:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh, Johannes Schindelin,
Elijah Newren, Ævar Arnfjörð Bjarmason
From: Johannes Schindelin <johannes.schindelin@gmx.de>
In d1bbbe45df8 (bisect--helper: reimplement `bisect_run` shell function
in C, 2021-09-13), we ported the `bisect run` subcommand to C, including
the part that prints out an error message when the implicit `git bisect
bad` or `git bisect good` failed.
However, the error message was supposed to print out whether the state
was "good" or "bad", but used a bogus (because non-populated) `args`
variable for it. This was fixed in 80c2e9657f2 (bisect--helper: report
actual bisect_state() argument on error, 2022-01-18), but the error
message still talks about `bisect--helper`, which is an implementation
detail that should not concern end users.
Fix that, and add a regression test to ensure that the intended form of
the error message.
Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/bisect.c | 2 +-
t/t6030-bisect-porcelain.sh | 10 ++++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 9fc8db06944..0786ebf4012 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -1292,7 +1292,7 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
puts(_("bisect found first bad commit"));
res = BISECT_OK;
} else if (res) {
- error(_("bisect run failed: 'bisect-state %s'"
+ error(_("bisect run failed: 'git bisect %s'"
" exited with error code %d"), new_state, res);
} else {
continue;
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 9e56b42b5da..0a62ea2b3ce 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -1221,4 +1221,14 @@ test_expect_success 'bisect state output with bad commit' '
grep -F "waiting for good commit(s), bad commit known" output
'
+test_expect_success 'verify correct error message' '
+ git bisect reset &&
+ git bisect start $HASH4 $HASH1 &&
+ write_script test_script.sh <<-\EOF &&
+ rm .git/BISECT*
+ EOF
+ test_must_fail git bisect run ./test_script.sh 2>error &&
+ grep "git bisect good.*exited with error code" error
+'
+
test_done
--
2.39.0.rc2.1048.g0e5493b8d5b
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/6] bisect: remove Cogito-related code
2022-12-15 9:47 [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c Ævar Arnfjörð Bjarmason
` (3 preceding siblings ...)
2022-12-15 9:47 ` [PATCH 4/6] bisect run: fix the error message Ævar Arnfjörð Bjarmason
@ 2022-12-15 9:47 ` Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 6/6] bisect: no longer try to clean up left-over `.git/head-name` files Ævar Arnfjörð Bjarmason
` (2 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-15 9:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh, Johannes Schindelin,
Ævar Arnfjörð Bjarmason
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Once upon a time, there was this idea that Git would not actually be a
single coherent program, but rather a set of low-level programs that
users cobble together via shell scripts, or develop high-level user
interfaces for Git, or both.
Cogito was such a high-level user interface, incidentally implemented
via shell scripts that cobble together Git calls.
It did turn out relatively quickly that Git would much rather provide a
useful high-level user interface itself.
As of April 19th, 2007, Cogito was therefore discontinued (see
https://lore.kernel.org/git/20070419124648.GL4489@pasky.or.cz/).
Nevertheless, for almost 15 years after that announcement, Git carried
special code in `git bisect` to accommodate Cogito.
Since it is beyond doubt that there are no more Cogito users, let's
remove the last remnant of Cogito-accommodating code.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/bisect.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 0786ebf4012..73017402671 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -15,7 +15,6 @@ static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
-static GIT_PATH_FUNC(git_path_head_name, "head-name")
static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
@@ -808,13 +807,6 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
strbuf_addstr(&start_head, oid_to_hex(&head_oid));
} else if (!get_oid(head, &head_oid) &&
skip_prefix(head, "refs/heads/", &head)) {
- /*
- * This error message should only be triggered by
- * cogito usage, and cogito users should understand
- * it relates to cg-seek.
- */
- if (!is_empty_or_missing_file(git_path_head_name()))
- return error(_("won't bisect on cg-seek'ed tree"));
strbuf_addstr(&start_head, head);
} else {
return error(_("bad HEAD - strange symbolic ref"));
--
2.39.0.rc2.1048.g0e5493b8d5b
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 6/6] bisect: no longer try to clean up left-over `.git/head-name` files
2022-12-15 9:47 [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c Ævar Arnfjörð Bjarmason
` (4 preceding siblings ...)
2022-12-15 9:47 ` [PATCH 5/6] bisect: remove Cogito-related code Ævar Arnfjörð Bjarmason
@ 2022-12-15 9:47 ` Ævar Arnfjörð Bjarmason
2022-12-15 10:45 ` [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c Christian Couder
2023-01-12 15:19 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
7 siblings, 0 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-15 9:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh, Johannes Schindelin,
Ævar Arnfjörð Bjarmason
From: Johannes Schindelin <johannes.schindelin@gmx.de>
As per the code comment, the `.git/head-name` files were cleaned up for
backwards-compatibility: an old version of `git bisect` could have left
them behind.
Now, just how old would such a version be? As of 0f497e75f05 (Eliminate
confusing "won't bisect on seeked tree" failure, 2008-02-23), `git
bisect` does not write that file anymore. Which corresponds to Git
v1.5.4.4.
Even if the likelihood is non-nil that there might still be users out
there who use such an old version to start a bisection, but then decide
to continue bisecting with a current Git version, it is highly
improbable.
So let's remove that code, at long last.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
bisect.c | 3 ---
t/t6030-bisect-porcelain.sh | 1 -
2 files changed, 4 deletions(-)
diff --git a/bisect.c b/bisect.c
index ec7487e6836..ef5ee5a6436 100644
--- a/bisect.c
+++ b/bisect.c
@@ -472,7 +472,6 @@ static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
-static GIT_PATH_FUNC(git_path_head_name, "head-name")
static void read_bisect_paths(struct strvec *array)
{
@@ -1188,8 +1187,6 @@ int bisect_clean_state(void)
unlink_or_warn(git_path_bisect_run());
unlink_or_warn(git_path_bisect_terms());
unlink_or_warn(git_path_bisect_first_parent());
- /* Cleanup head-name if it got left by an old version of git-bisect */
- unlink_or_warn(git_path_head_name());
/*
* Cleanup BISECT_START last to support the --no-checkout option
* introduced in the commit 4796e823a.
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 0a62ea2b3ce..3ba4fdf6153 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -1158,7 +1158,6 @@ test_expect_success 'git bisect reset cleans bisection state properly' '
test_path_is_missing ".git/BISECT_LOG" &&
test_path_is_missing ".git/BISECT_RUN" &&
test_path_is_missing ".git/BISECT_TERMS" &&
- test_path_is_missing ".git/head-name" &&
test_path_is_missing ".git/BISECT_HEAD" &&
test_path_is_missing ".git/BISECT_START"
'
--
2.39.0.rc2.1048.g0e5493b8d5b
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c
2022-12-15 9:47 [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c Ævar Arnfjörð Bjarmason
` (5 preceding siblings ...)
2022-12-15 9:47 ` [PATCH 6/6] bisect: no longer try to clean up left-over `.git/head-name` files Ævar Arnfjörð Bjarmason
@ 2022-12-15 10:45 ` Christian Couder
2023-01-12 15:19 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
7 siblings, 0 replies; 16+ messages in thread
From: Christian Couder @ 2022-12-15 10:45 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh
On Thu, Dec 15, 2022 at 11:27 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> This small set of fixes is a cherry-pick of various miscellanious
s/miscellanious/miscellaneous/
> fixes that were part of js/bisect-in-c that are still worth
> having.
Thanks! All the patches make sense and look good to me.
> Johannes Schindelin (5):
> bisect--helper: simplify exit code computation
> bisect: verify that a bogus option won't try to start a bisection
> bisect run: fix the error message
I just noticed a nit in the commit message of the above patch. It contains:
"Fix that, and add a regression test to ensure that the intended form of
the error message."
And it seems to me that something like "is printed" is missing from
the end of that sentence.
> bisect: remove Cogito-related code
> bisect: no longer try to clean up left-over `.git/head-name` files
>
> Ævar Arnfjörð Bjarmason (1):
> bisect--helper: make the order consistently `argc, argv`
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/6] bisect run: fix the error message
2022-12-15 9:47 ` [PATCH 4/6] bisect run: fix the error message Ævar Arnfjörð Bjarmason
@ 2022-12-15 15:08 ` Đoàn Trần Công Danh
0 siblings, 0 replies; 16+ messages in thread
From: Đoàn Trần Công Danh @ 2022-12-15 15:08 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Johannes Schindelin, Elijah Newren
On 2022-12-15 10:47:47+0100, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In d1bbbe45df8 (bisect--helper: reimplement `bisect_run` shell function
> in C, 2021-09-13), we ported the `bisect run` subcommand to C, including
> the part that prints out an error message when the implicit `git bisect
> bad` or `git bisect good` failed.
>
> However, the error message was supposed to print out whether the state
> was "good" or "bad", but used a bogus (because non-populated) `args`
> variable for it. This was fixed in 80c2e9657f2 (bisect--helper: report
> actual bisect_state() argument on error, 2022-01-18), but the error
> message still talks about `bisect--helper`, which is an implementation
> detail that should not concern end users.
I don't think the error still talks about `bisect--helper`.
The error is talking about some "bisect-state" which is not it Git's
glossary, though.
> Fix that, and add a regression test to ensure that the intended form of
> the error message.
>
> Helped-by: Elijah Newren <newren@gmail.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> builtin/bisect.c | 2 +-
> t/t6030-bisect-porcelain.sh | 10 ++++++++++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/bisect.c b/builtin/bisect.c
> index 9fc8db06944..0786ebf4012 100644
> --- a/builtin/bisect.c
> +++ b/builtin/bisect.c
> @@ -1292,7 +1292,7 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
> puts(_("bisect found first bad commit"));
> res = BISECT_OK;
> } else if (res) {
> - error(_("bisect run failed: 'bisect-state %s'"
> + error(_("bisect run failed: 'git bisect %s'"
> " exited with error code %d"), new_state, res);
> } else {
> continue;
> diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
> index 9e56b42b5da..0a62ea2b3ce 100755
> --- a/t/t6030-bisect-porcelain.sh
> +++ b/t/t6030-bisect-porcelain.sh
> @@ -1221,4 +1221,14 @@ test_expect_success 'bisect state output with bad commit' '
> grep -F "waiting for good commit(s), bad commit known" output
> '
>
> +test_expect_success 'verify correct error message' '
> + git bisect reset &&
> + git bisect start $HASH4 $HASH1 &&
> + write_script test_script.sh <<-\EOF &&
> + rm .git/BISECT*
> + EOF
> + test_must_fail git bisect run ./test_script.sh 2>error &&
> + grep "git bisect good.*exited with error code" error
> +'
> +
> test_done
> --
> 2.39.0.rc2.1048.g0e5493b8d5b
>
--
Danh
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 0/6] bisect: follow-up fixes from js/bisect-in-c
2022-12-15 9:47 [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c Ævar Arnfjörð Bjarmason
` (6 preceding siblings ...)
2022-12-15 10:45 ` [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c Christian Couder
@ 2023-01-12 15:19 ` Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 1/6] bisect--helper: simplify exit code computation Ævar Arnfjörð Bjarmason
` (5 more replies)
7 siblings, 6 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 15:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
A trivial update to this topic of follow-up fixes to "bisect", which
is now a built-in. See
https://lore.kernel.org/git/cover-0.6-00000000000-20221215T094038Z-avarab@gmail.com/
for the v1 & general summary.
Change since v1:
* Rephrased & updated a commit message which was outdated as of
f37d0bdd42d, see the range-diff. Thanks to Đoàn Trần Công Danh for
spotting it.
Johannes Schindelin (5):
bisect--helper: simplify exit code computation
bisect: verify that a bogus option won't try to start a bisection
bisect run: fix the error message
bisect: remove Cogito-related code
bisect: no longer try to clean up left-over `.git/head-name` files
Ævar Arnfjörð Bjarmason (1):
bisect--helper: make the order consistently `argc, argv`
bisect.c | 3 ---
builtin/bisect.c | 52 ++++++++++++++-----------------------
t/t6030-bisect-porcelain.sh | 21 ++++++++++++++-
3 files changed, 40 insertions(+), 36 deletions(-)
Range-diff against v1:
1: c8c648e4b8c = 1: 32c45bbf851 bisect--helper: simplify exit code computation
2: a0de7ad6836 = 2: 1f4449dd081 bisect--helper: make the order consistently `argc, argv`
3: e1e31278fef = 3: 0cfb7dc572c bisect: verify that a bogus option won't try to start a bisection
4: 59a8a3085b1 ! 4: 4dda1019767 bisect run: fix the error message
@@ Commit message
However, the error message was supposed to print out whether the state
was "good" or "bad", but used a bogus (because non-populated) `args`
- variable for it. This was fixed in 80c2e9657f2 (bisect--helper: report
- actual bisect_state() argument on error, 2022-01-18), but the error
- message still talks about `bisect--helper`, which is an implementation
- detail that should not concern end users.
+ variable for it. This was fixed in [1], but as of [2] (when
+ `bisect--helper` was changed to the present `bisect-state') the error
+ message still talks about implementation details that should not
+ concern end users.
Fix that, and add a regression test to ensure that the intended form of
the error message.
+ 1. 80c2e9657f2 (bisect--helper: report actual bisect_state() argument
+ on error, 2022-01-18
+ 2. f37d0bdd42d (bisect: fix output regressions in v2.30.0, 2022-11-10)
+
Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
5: 1b70cd79cae = 5: 1600ef41608 bisect: remove Cogito-related code
6: 2ad89aca728 = 6: 817fe726b4b bisect: no longer try to clean up left-over `.git/head-name` files
--
2.39.0.1215.g1ba3f685d4f
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/6] bisect--helper: simplify exit code computation
2023-01-12 15:19 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
@ 2023-01-12 15:19 ` Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 2/6] bisect--helper: make the order consistently `argc, argv` Ævar Arnfjörð Bjarmason
` (4 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 15:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh, Johannes Schindelin,
Ævar Arnfjörð Bjarmason
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We _already_ have a function to determine whether a given `enum
bisect_error` value is non-zero but still _actually_ indicates success.
Let's use it instead of duplicating the logic.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/bisect.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c
index cc9483e8515..09505fc4dce 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -1440,12 +1440,5 @@ int cmd_bisect(int argc, const char **argv, const char *prefix)
res = fn(argc, argv, prefix);
}
- /*
- * Handle early success
- * From check_merge_bases > check_good_are_ancestors_of_bad > bisect_next_all
- */
- if ((res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) || (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND))
- res = BISECT_OK;
-
- return -res;
+ return is_bisect_success(res) ? 0 : -res;
}
--
2.39.0.1215.g1ba3f685d4f
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/6] bisect--helper: make the order consistently `argc, argv`
2023-01-12 15:19 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 1/6] bisect--helper: simplify exit code computation Ævar Arnfjörð Bjarmason
@ 2023-01-12 15:19 ` Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 3/6] bisect: verify that a bogus option won't try to start a bisection Ævar Arnfjörð Bjarmason
` (3 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 15:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason, Johannes Schindelin
In C, the natural order is for `argc` to come before `argv` by virtue of
the `main()` function declaring the parameters in precisely that order.
It is confusing & distracting, then, when readers familiar with the C
language read code where that order is switched around.
Let's just change the order and avoid that type of developer friction.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/bisect.c | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 09505fc4dce..9fc8db06944 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -678,7 +678,8 @@ static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char
return bisect_next(terms, prefix);
}
-static enum bisect_error bisect_start(struct bisect_terms *terms, const char **argv, int argc)
+static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
+ const char **argv)
{
int no_checkout = 0;
int first_parent_only = 0;
@@ -908,13 +909,13 @@ static int bisect_autostart(struct bisect_terms *terms)
yesno = git_prompt(_("Do you want me to do it for you "
"[Y/n]? "), PROMPT_ECHO);
res = tolower(*yesno) == 'n' ?
- -1 : bisect_start(terms, empty_strvec, 0);
+ -1 : bisect_start(terms, 0, empty_strvec);
return res;
}
-static enum bisect_error bisect_state(struct bisect_terms *terms, const char **argv,
- int argc)
+static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
+ const char **argv)
{
const char *state;
int i, verify_expected = 1;
@@ -1033,7 +1034,7 @@ static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)
struct strvec argv = STRVEC_INIT;
int res;
sq_dequote_to_strvec(rev, &argv);
- res = bisect_start(terms, argv.v, argv.nr);
+ res = bisect_start(terms, argv.nr, argv.v);
strvec_clear(&argv);
return res;
}
@@ -1083,7 +1084,8 @@ static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f
return bisect_auto_next(terms, NULL);
}
-static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **argv, int argc)
+static enum bisect_error bisect_skip(struct bisect_terms *terms, int argc,
+ const char **argv)
{
int i;
enum bisect_error res;
@@ -1113,13 +1115,14 @@ static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **ar
strvec_push(&argv_state, argv[i]);
}
}
- res = bisect_state(terms, argv_state.v, argv_state.nr);
+ res = bisect_state(terms, argv_state.nr, argv_state.v);
strvec_clear(&argv_state);
return res;
}
-static int bisect_visualize(struct bisect_terms *terms, const char **argv, int argc)
+static int bisect_visualize(struct bisect_terms *terms, int argc,
+ const char **argv)
{
struct child_process cmd = CHILD_PROCESS_INIT;
struct strbuf sb = STRBUF_INIT;
@@ -1202,7 +1205,7 @@ static int verify_good(const struct bisect_terms *terms, const char *command)
return rc;
}
-static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
+static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
{
int res = BISECT_OK;
struct strbuf command = STRBUF_INIT;
@@ -1271,7 +1274,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
saved_stdout = dup(1);
dup2(temporary_stdout_fd, 1);
- res = bisect_state(terms, &new_state, 1);
+ res = bisect_state(terms, 1, &new_state);
fflush(stdout);
dup2(saved_stdout, 1);
@@ -1328,7 +1331,7 @@ static int cmd_bisect__start(int argc, const char **argv, const char *prefix UNU
struct bisect_terms terms = { 0 };
set_terms(&terms, "bad", "good");
- res = bisect_start(&terms, argv, argc);
+ res = bisect_start(&terms, argc, argv);
free_terms(&terms);
return res;
}
@@ -1372,7 +1375,7 @@ static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUS
set_terms(&terms, "bad", "good");
get_terms(&terms);
- res = bisect_skip(&terms, argv, argc);
+ res = bisect_skip(&terms, argc, argv);
free_terms(&terms);
return res;
}
@@ -1383,7 +1386,7 @@ static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix
struct bisect_terms terms = { 0 };
get_terms(&terms);
- res = bisect_visualize(&terms, argv, argc);
+ res = bisect_visualize(&terms, argc, argv);
free_terms(&terms);
return res;
}
@@ -1396,7 +1399,7 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE
if (!argc)
return error(_("'%s' failed: no command provided."), "git bisect run");
get_terms(&terms);
- res = bisect_run(&terms, argv, argc);
+ res = bisect_run(&terms, argc, argv);
free_terms(&terms);
return res;
}
@@ -1432,7 +1435,7 @@ int cmd_bisect(int argc, const char **argv, const char *prefix)
if (check_and_set_terms(&terms, argv[0]))
usage_msg_optf(_("unknown command: '%s'"), git_bisect_usage,
options, argv[0]);
- res = bisect_state(&terms, argv, argc);
+ res = bisect_state(&terms, argc, argv);
free_terms(&terms);
} else {
argc--;
--
2.39.0.1215.g1ba3f685d4f
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/6] bisect: verify that a bogus option won't try to start a bisection
2023-01-12 15:19 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 1/6] bisect--helper: simplify exit code computation Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 2/6] bisect--helper: make the order consistently `argc, argv` Ævar Arnfjörð Bjarmason
@ 2023-01-12 15:19 ` Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 4/6] bisect run: fix the error message Ævar Arnfjörð Bjarmason
` (2 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 15:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh, Johannes Schindelin,
Ævar Arnfjörð Bjarmason
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We do not want `git bisect --bogus-option` to start a bisection. To
verify that, we look for the tell-tale error message `You need to start
by "git bisect start"` and fail if it was found.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t6030-bisect-porcelain.sh | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 98a72ff78a7..9e56b42b5da 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -1058,6 +1058,16 @@ test_expect_success 'bisect start with one term1 and term2' '
git bisect reset
'
+test_expect_success 'bogus command does not start bisect' '
+ git bisect reset &&
+ test_must_fail git bisect --bisect-terms 1 2 2>out &&
+ ! grep "You need to start" out &&
+ test_must_fail git bisect --bisect-terms 2>out &&
+ ! grep "You need to start" out &&
+ grep "git bisect.*visualize" out &&
+ git bisect reset
+'
+
test_expect_success 'bisect replay with term1 and term2' '
git bisect replay log_to_replay.txt >bisect_result &&
grep "$HASH2 is the first term1 commit" bisect_result &&
--
2.39.0.1215.g1ba3f685d4f
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 4/6] bisect run: fix the error message
2023-01-12 15:19 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
` (2 preceding siblings ...)
2023-01-12 15:19 ` [PATCH v2 3/6] bisect: verify that a bogus option won't try to start a bisection Ævar Arnfjörð Bjarmason
@ 2023-01-12 15:19 ` Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 5/6] bisect: remove Cogito-related code Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 6/6] bisect: no longer try to clean up left-over `.git/head-name` files Ævar Arnfjörð Bjarmason
5 siblings, 0 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 15:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh, Johannes Schindelin,
Elijah Newren, Ævar Arnfjörð Bjarmason
From: Johannes Schindelin <johannes.schindelin@gmx.de>
In d1bbbe45df8 (bisect--helper: reimplement `bisect_run` shell function
in C, 2021-09-13), we ported the `bisect run` subcommand to C, including
the part that prints out an error message when the implicit `git bisect
bad` or `git bisect good` failed.
However, the error message was supposed to print out whether the state
was "good" or "bad", but used a bogus (because non-populated) `args`
variable for it. This was fixed in [1], but as of [2] (when
`bisect--helper` was changed to the present `bisect-state') the error
message still talks about implementation details that should not
concern end users.
Fix that, and add a regression test to ensure that the intended form of
the error message.
1. 80c2e9657f2 (bisect--helper: report actual bisect_state() argument
on error, 2022-01-18
2. f37d0bdd42d (bisect: fix output regressions in v2.30.0, 2022-11-10)
Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/bisect.c | 2 +-
t/t6030-bisect-porcelain.sh | 10 ++++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 9fc8db06944..0786ebf4012 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -1292,7 +1292,7 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
puts(_("bisect found first bad commit"));
res = BISECT_OK;
} else if (res) {
- error(_("bisect run failed: 'bisect-state %s'"
+ error(_("bisect run failed: 'git bisect %s'"
" exited with error code %d"), new_state, res);
} else {
continue;
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 9e56b42b5da..0a62ea2b3ce 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -1221,4 +1221,14 @@ test_expect_success 'bisect state output with bad commit' '
grep -F "waiting for good commit(s), bad commit known" output
'
+test_expect_success 'verify correct error message' '
+ git bisect reset &&
+ git bisect start $HASH4 $HASH1 &&
+ write_script test_script.sh <<-\EOF &&
+ rm .git/BISECT*
+ EOF
+ test_must_fail git bisect run ./test_script.sh 2>error &&
+ grep "git bisect good.*exited with error code" error
+'
+
test_done
--
2.39.0.1215.g1ba3f685d4f
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 5/6] bisect: remove Cogito-related code
2023-01-12 15:19 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
` (3 preceding siblings ...)
2023-01-12 15:19 ` [PATCH v2 4/6] bisect run: fix the error message Ævar Arnfjörð Bjarmason
@ 2023-01-12 15:19 ` Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 6/6] bisect: no longer try to clean up left-over `.git/head-name` files Ævar Arnfjörð Bjarmason
5 siblings, 0 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 15:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh, Johannes Schindelin,
Ævar Arnfjörð Bjarmason
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Once upon a time, there was this idea that Git would not actually be a
single coherent program, but rather a set of low-level programs that
users cobble together via shell scripts, or develop high-level user
interfaces for Git, or both.
Cogito was such a high-level user interface, incidentally implemented
via shell scripts that cobble together Git calls.
It did turn out relatively quickly that Git would much rather provide a
useful high-level user interface itself.
As of April 19th, 2007, Cogito was therefore discontinued (see
https://lore.kernel.org/git/20070419124648.GL4489@pasky.or.cz/).
Nevertheless, for almost 15 years after that announcement, Git carried
special code in `git bisect` to accommodate Cogito.
Since it is beyond doubt that there are no more Cogito users, let's
remove the last remnant of Cogito-accommodating code.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/bisect.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 0786ebf4012..73017402671 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -15,7 +15,6 @@ static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
-static GIT_PATH_FUNC(git_path_head_name, "head-name")
static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
@@ -808,13 +807,6 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
strbuf_addstr(&start_head, oid_to_hex(&head_oid));
} else if (!get_oid(head, &head_oid) &&
skip_prefix(head, "refs/heads/", &head)) {
- /*
- * This error message should only be triggered by
- * cogito usage, and cogito users should understand
- * it relates to cg-seek.
- */
- if (!is_empty_or_missing_file(git_path_head_name()))
- return error(_("won't bisect on cg-seek'ed tree"));
strbuf_addstr(&start_head, head);
} else {
return error(_("bad HEAD - strange symbolic ref"));
--
2.39.0.1215.g1ba3f685d4f
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 6/6] bisect: no longer try to clean up left-over `.git/head-name` files
2023-01-12 15:19 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
` (4 preceding siblings ...)
2023-01-12 15:19 ` [PATCH v2 5/6] bisect: remove Cogito-related code Ævar Arnfjörð Bjarmason
@ 2023-01-12 15:19 ` Ævar Arnfjörð Bjarmason
5 siblings, 0 replies; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-01-12 15:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Johannes Schindelin,
Đoàn Trần Công Danh, Johannes Schindelin,
Ævar Arnfjörð Bjarmason
From: Johannes Schindelin <johannes.schindelin@gmx.de>
As per the code comment, the `.git/head-name` files were cleaned up for
backwards-compatibility: an old version of `git bisect` could have left
them behind.
Now, just how old would such a version be? As of 0f497e75f05 (Eliminate
confusing "won't bisect on seeked tree" failure, 2008-02-23), `git
bisect` does not write that file anymore. Which corresponds to Git
v1.5.4.4.
Even if the likelihood is non-nil that there might still be users out
there who use such an old version to start a bisection, but then decide
to continue bisecting with a current Git version, it is highly
improbable.
So let's remove that code, at long last.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
bisect.c | 3 ---
t/t6030-bisect-porcelain.sh | 1 -
2 files changed, 4 deletions(-)
diff --git a/bisect.c b/bisect.c
index ec7487e6836..ef5ee5a6436 100644
--- a/bisect.c
+++ b/bisect.c
@@ -472,7 +472,6 @@ static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
-static GIT_PATH_FUNC(git_path_head_name, "head-name")
static void read_bisect_paths(struct strvec *array)
{
@@ -1188,8 +1187,6 @@ int bisect_clean_state(void)
unlink_or_warn(git_path_bisect_run());
unlink_or_warn(git_path_bisect_terms());
unlink_or_warn(git_path_bisect_first_parent());
- /* Cleanup head-name if it got left by an old version of git-bisect */
- unlink_or_warn(git_path_head_name());
/*
* Cleanup BISECT_START last to support the --no-checkout option
* introduced in the commit 4796e823a.
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 0a62ea2b3ce..3ba4fdf6153 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -1158,7 +1158,6 @@ test_expect_success 'git bisect reset cleans bisection state properly' '
test_path_is_missing ".git/BISECT_LOG" &&
test_path_is_missing ".git/BISECT_RUN" &&
test_path_is_missing ".git/BISECT_TERMS" &&
- test_path_is_missing ".git/head-name" &&
test_path_is_missing ".git/BISECT_HEAD" &&
test_path_is_missing ".git/BISECT_START"
'
--
2.39.0.1215.g1ba3f685d4f
^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-01-12 15:27 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-15 9:47 [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 1/6] bisect--helper: simplify exit code computation Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 2/6] bisect--helper: make the order consistently `argc, argv` Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 3/6] bisect: verify that a bogus option won't try to start a bisection Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 4/6] bisect run: fix the error message Ævar Arnfjörð Bjarmason
2022-12-15 15:08 ` Đoàn Trần Công Danh
2022-12-15 9:47 ` [PATCH 5/6] bisect: remove Cogito-related code Ævar Arnfjörð Bjarmason
2022-12-15 9:47 ` [PATCH 6/6] bisect: no longer try to clean up left-over `.git/head-name` files Ævar Arnfjörð Bjarmason
2022-12-15 10:45 ` [PATCH 0/6] bisect: follow-up fixes from js/bisect-in-c Christian Couder
2023-01-12 15:19 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 1/6] bisect--helper: simplify exit code computation Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 2/6] bisect--helper: make the order consistently `argc, argv` Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 3/6] bisect: verify that a bogus option won't try to start a bisection Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 4/6] bisect run: fix the error message Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 5/6] bisect: remove Cogito-related code Ævar Arnfjörð Bjarmason
2023-01-12 15:19 ` [PATCH v2 6/6] bisect: no longer try to clean up left-over `.git/head-name` files Ævar Arnfjörð Bjarmason
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).