undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge branch 'mr/bisect-in-c-1'

Browse Source 
Underlying machinery of "git bisect--helper" is being refactored
into pieces that are more easily reused.

* mr/bisect-in-c-1:
  bisect: libify `bisect_next_all`
  bisect: libify `handle_bad_merge_base` and its dependents
  bisect: libify `check_good_are_ancestors_of_bad` and its dependents
  bisect: libify `check_merge_bases` and its dependents
  bisect: libify `bisect_checkout`
  bisect: libify `exit_if_skipped_commits` to `error_if_skipped*` and its dependents
  bisect--helper: return error codes from `cmd_bisect__helper()`
  bisect: add enum to represent bisect returning codes
  bisect--helper: introduce new `decide_next()` function
  bisect: use the standard 'if (!var)' way to check for 0
  bisect--helper: change `retval` to `res`
  bisect--helper: convert `vocab_*` char pointers to char arrays
This commit is contained in:
Junio C Hamano 2020-03-05 10:43:02 -08:00
commit 25063e2530
3 changed files with 189 additions and 98 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

135
bisect.c
View File

@ -572,7 +572,7 @@ static int sqrti(int val)
{
float d, x = val;
if (val == 0)
if (!val)
return 0;
do {
@ -661,11 +661,11 @@ static void bisect_common(struct rev_info *revs)
mark_edges_uninteresting(revs, NULL, 0);
}
static void exit_if_skipped_commits(struct commit_list *tried,
static enum bisect_error error_if_skipped_commits(struct commit_list *tried,
const struct object_id *bad)
{
if (!tried)
return;
return BISECT_OK;
printf("There are only 'skip'ped commits left to test.\n"
"The first %s commit could be any of:\n", term_bad);
@ -676,7 +676,8 @@ static void exit_if_skipped_commits(struct commit_list *tried,
if (bad)
printf("%s\n", oid_to_hex(bad));
printf(_("We cannot bisect more!\n"));
exit(2);
return BISECT_ONLY_SKIPPED_LEFT;
}
static int is_expected_rev(const struct object_id *oid)
@ -703,9 +704,10 @@ static int is_expected_rev(const struct object_id *oid)
return res;
}
static int bisect_checkout(const struct object_id *bisect_rev, int no_checkout)
static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int no_checkout)
{
char bisect_rev_hex[GIT_MAX_HEXSZ + 1];
enum bisect_error res = BISECT_OK;
memcpy(bisect_rev_hex, oid_to_hex(bisect_rev), the_hash_algo->hexsz + 1);
update_ref(NULL, "BISECT_EXPECTED_REV", bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
@ -715,14 +717,24 @@ static int bisect_checkout(const struct object_id *bisect_rev, int no_checkout)
update_ref(NULL, "BISECT_HEAD", bisect_rev, NULL, 0,
UPDATE_REFS_DIE_ON_ERR);
} else {
int res;
res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
if (res)
exit(res);
/*
* Errors in `run_command()` itself, signaled by res < 0,
* and errors in the child process, signaled by res > 0
* can both be treated as regular BISECT_FAILURE (-1).
*/
return -abs(res);
}
argv_show_branch[1] = bisect_rev_hex;
return run_command_v_opt(argv_show_branch, RUN_GIT_CMD);
res = run_command_v_opt(argv_show_branch, RUN_GIT_CMD);
/*
* Errors in `run_command()` itself, signaled by res < 0,
* and errors in the child process, signaled by res > 0
* can both be treated as regular BISECT_FAILURE (-1).
*/
return -abs(res);
}
static struct commit *get_commit_reference(struct repository *r,
@ -749,7 +761,7 @@ static struct commit **get_bad_and_good_commits(struct repository *r,
return rev;
}
static void handle_bad_merge_base(void)
static enum bisect_error handle_bad_merge_base(void)
{
if (is_expected_rev(current_bad_oid)) {
char *bad_hex = oid_to_hex(current_bad_oid);
@ -770,14 +782,14 @@ static void handle_bad_merge_base(void)
"between %s and [%s].\n"),
bad_hex, term_bad, term_good, bad_hex, good_hex);
}
exit(3);
return BISECT_MERGE_BASE_CHECK;
}
fprintf(stderr, _("Some %s revs are not ancestors of the %s rev.\n"
"git bisect cannot work properly in this case.\n"
"Maybe you mistook %s and %s revs?\n"),
term_good, term_bad, term_good, term_bad);
exit(1);
return BISECT_FAILED;
}
static void handle_skipped_merge_base(const struct object_id *mb)
@ -799,13 +811,18 @@ static void handle_skipped_merge_base(const struct object_id *mb)
* "check_merge_bases" checks that merge bases are not "bad" (or "new").
*
* - If one is "bad" (or "new"), it means the user assumed something wrong
* and we must exit with a non 0 error code.
* and we must return error with a non 0 error code.
* - If one is "good" (or "old"), that's good, we have nothing to do.
* - 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.
* - If a merge base must be tested, on success return
* BISECT_INTERNAL_SUCCESS_MERGE_BASE (-11) a special condition
* for early success, this will be converted back to 0 in
* check_good_are_ancestors_of_bad().
*/
static void check_merge_bases(int rev_nr, struct commit **rev, int no_checkout)
static enum bisect_error check_merge_bases(int rev_nr, struct commit **rev, int no_checkout)
{
enum bisect_error res = BISECT_OK;
struct commit_list *result;
result = get_merge_bases_many(rev[0], rev_nr - 1, rev + 1);
@ -813,18 +830,24 @@ static void check_merge_bases(int rev_nr, struct commit **rev, int no_checkout)
for (; result; result = result->next) {
const struct object_id *mb = &result->item->object.oid;
if (oideq(mb, current_bad_oid)) {
handle_bad_merge_base();
res = handle_bad_merge_base();
break;
} else if (0 <= oid_array_lookup(&good_revs, mb)) {
continue;
} else if (0 <= oid_array_lookup(&skipped_revs, mb)) {
handle_skipped_merge_base(mb);
} else {
printf(_("Bisecting: a merge base must be tested\n"));
exit(bisect_checkout(mb, no_checkout));
res = bisect_checkout(mb, no_checkout);
if (!res)
/* indicate early success */
res = BISECT_INTERNAL_SUCCESS_MERGE_BASE;
break;
}
}
free_commit_list(result);
return res;
}
static int check_ancestors(struct repository *r, int rev_nr,
@ -850,43 +873,58 @@ static int check_ancestors(struct repository *r, int rev_nr,
*
* If that's not the case, we need to check the merge bases.
* 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.
* checked out to be tested by the user and we will return.
*/
static void check_good_are_ancestors_of_bad(struct repository *r,
static enum bisect_error check_good_are_ancestors_of_bad(struct repository *r,
const char *prefix,
int no_checkout)
{
char *filename = git_pathdup("BISECT_ANCESTORS_OK");
char *filename;
struct stat st;
int fd, rev_nr;
enum bisect_error res = BISECT_OK;
struct commit **rev;
if (!current_bad_oid)
die(_("a %s revision is needed"), term_bad);
return error(_("a %s revision is needed"), term_bad);
filename = git_pathdup("BISECT_ANCESTORS_OK");
/* Check if file BISECT_ANCESTORS_OK exists. */
if (!stat(filename, &st) && S_ISREG(st.st_mode))
goto done;
/* Bisecting with no good rev is ok. */
if (good_revs.nr == 0)
if (!good_revs.nr)
goto done;
/* Check if all good revs are ancestor of the bad rev. */
rev = get_bad_and_good_commits(r, &rev_nr);
if (check_ancestors(r, rev_nr, rev, prefix))
check_merge_bases(rev_nr, rev, no_checkout);
res = check_merge_bases(rev_nr, rev, no_checkout);
free(rev);
/* Create file BISECT_ANCESTORS_OK. */
fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);
if (fd < 0)
warning_errno(_("could not create file '%s'"),
filename);
else
close(fd);
if (!res) {
/* Create file BISECT_ANCESTORS_OK. */
fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);
if (fd < 0)
/*
* BISECT_ANCESTORS_OK file is not absolutely necessary,
* the bisection process will continue at the next
* bisection step.
* So, just signal with a warning that something
* might be wrong.
*/
warning_errno(_("could not create file '%s'"),
filename);
else
close(fd);
}
done:
free(filename);
return res;
}
/*
@ -938,18 +976,19 @@ void read_bisect_terms(const char **read_bad, const char **read_good)
}
/*
* We use the convention that exiting with an exit code 10 means that
* We use the convention that return BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND (-10) means
* the bisection process finished successfully.
* In this case the calling shell script should exit 0.
*
* In this case the calling function or command should not turn a
* BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND return code into an error or a non zero exit code.
* If no_checkout is non-zero, the bisection process does not
* checkout the trial commit but instead simply updates BISECT_HEAD.
*/
int bisect_next_all(struct repository *r, const char *prefix, int no_checkout)
enum bisect_error bisect_next_all(struct repository *r, const char *prefix, int no_checkout)
{
struct rev_info revs;
struct commit_list *tried;
int reaches = 0, all = 0, nr, steps;
enum bisect_error res = BISECT_OK;
struct object_id *bisect_rev;
char *steps_msg;
@ -957,7 +996,9 @@ int bisect_next_all(struct repository *r, const char *prefix, int no_checkout)
if (read_bisect_refs())
die(_("reading bisect refs failed"));
check_good_are_ancestors_of_bad(r, prefix, no_checkout);
res = check_good_are_ancestors_of_bad(r, prefix, no_checkout);
if (res)
return res;
bisect_rev_setup(r, &revs, prefix, "%s", "^%s", 1);
revs.limited = 1;
@ -969,33 +1010,45 @@ int bisect_next_all(struct repository *r, const char *prefix, int no_checkout)
if (!revs.commits) {
/*
* We should exit here only if the "bad"
* We should return error here only if the "bad"
* commit is also a "skip" commit.
*/
exit_if_skipped_commits(tried, NULL);
res = error_if_skipped_commits(tried, NULL);
if (res < 0)
return res;
printf(_("%s was both %s and %s\n"),
oid_to_hex(current_bad_oid),
term_good,
term_bad);
exit(1);
return BISECT_FAILED;
}
if (!all) {
fprintf(stderr, _("No testable commit found.\n"
"Maybe you started with bad path parameters?\n"));
exit(4);
return BISECT_NO_TESTABLE_COMMIT;
}
bisect_rev = &revs.commits->item->object.oid;
if (oideq(bisect_rev, current_bad_oid)) {
exit_if_skipped_commits(tried, current_bad_oid);
res = error_if_skipped_commits(tried, current_bad_oid);
if (res)
return res;
printf("%s is the first %s commit\n", oid_to_hex(bisect_rev),
term_bad);
show_diff_tree(r, prefix, revs.commits->item);
/* This means the bisection process succeeded. */
exit(10);
/*
* This means the bisection process succeeded.
* Using BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND (-10)
* so that the call chain can simply check
* for negative return values for early returns up
* until the cmd_bisect__helper() caller.
*/
return BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;
}
nr = all - reaches - 1;

29
bisect.h
View File

@ -31,7 +31,34 @@ struct rev_list_info {
const char *header_prefix;
};
int bisect_next_all(struct repository *r,
/*
* enum bisect_error represents the following return codes:
* BISECT_OK: success code. Internally, it means that next
* commit has been found (and possibly checked out) and it
* should be tested.
* BISECT_FAILED error code: default error code.
* BISECT_ONLY_SKIPPED_LEFT error code: only skipped
* commits left to be tested.
* BISECT_MERGE_BASE_CHECK error code: merge base check failed.
* BISECT_NO_TESTABLE_COMMIT error code: no testable commit found.
* BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND early success code:
* first term_bad commit found.
* BISECT_INTERNAL_SUCCESS_MERGE_BASE early success
* code: found merge base that should be tested.
* Early success codes BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND and
* BISECT_INTERNAL_SUCCESS_MERGE_BASE should be only internal codes.
*/
enum bisect_error {
BISECT_OK = 0,
BISECT_FAILED = -1,
BISECT_ONLY_SKIPPED_LEFT = -2,
BISECT_MERGE_BASE_CHECK = -3,
BISECT_NO_TESTABLE_COMMIT = -4,
BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND = -10,
BISECT_INTERNAL_SUCCESS_MERGE_BASE = -11
};
enum bisect_error bisect_next_all(struct repository *r,
const char *prefix,
int no_checkout);

123
builtin/bisect--helper.c
View File

@ -52,8 +52,8 @@ static void set_terms(struct bisect_terms *terms, const char *bad,
terms->term_bad = xstrdup(bad);
}
static const char *vocab_bad = "bad|new";
static const char *vocab_good = "good|old";
static const char vocab_bad[] = "bad|new";
static const char vocab_good[] = "good|old";
/*
* Check whether the string `term` belongs to the set of strings
@ -206,31 +206,31 @@ static int bisect_write(const char *state, const char *rev,
struct object_id oid;
struct commit *commit;
FILE *fp = NULL;
int retval = 0;
int res = 0;
if (!strcmp(state, terms->term_bad)) {
strbuf_addf(&tag, "refs/bisect/%s", state);
} else if (one_of(state, terms->term_good, "skip", NULL)) {
strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
} else {
retval = error(_("Bad bisect_write argument: %s"), state);
res = error(_("Bad bisect_write argument: %s"), state);
goto finish;
}
if (get_oid(rev, &oid)) {
retval = error(_("couldn't get the oid of the rev '%s'"), rev);
res = error(_("couldn't get the oid of the rev '%s'"), rev);
goto finish;
}
if (update_ref(NULL, tag.buf, &oid, NULL, 0,
UPDATE_REFS_MSG_ON_ERR)) {
retval = -1;
res = -1;
goto finish;
}
fp = fopen(git_path_bisect_log(), "a");
if (!fp) {
retval = error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
res = error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
goto finish;
}
@ -244,7 +244,7 @@ finish:
if (fp)
fclose(fp);
strbuf_release(&tag);
return retval;
return res;
}
static int check_and_set_terms(struct bisect_terms *terms, const char *cmd)
@ -291,26 +291,14 @@ static const char need_bisect_start_warning[] =
"You then need to give me at least one %s and %s revision.\n"
"You can use \"git bisect %s\" and \"git bisect %s\" for that.");
static int bisect_next_check(const struct bisect_terms *terms,
const char *current_term)
static int decide_next(const struct bisect_terms *terms,
const char *current_term, int missing_good,
int missing_bad)
{
int missing_good = 1, missing_bad = 1, retval = 0;
const char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
const char *good_glob = xstrfmt("%s-*", terms->term_good);
if (ref_exists(bad_ref))
missing_bad = 0;
for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
(void *) &missing_good);
if (!missing_good && !missing_bad)
goto finish;
if (!current_term) {
retval = -1;
goto finish;
}
return 0;
if (!current_term)
return -1;
if (missing_good && !missing_bad &&
!strcmp(current_term, terms->term_good)) {
@ -321,7 +309,7 @@ static int bisect_next_check(const struct bisect_terms *terms,
*/
warning(_("bisecting only with a %s commit"), terms->term_bad);
if (!isatty(0))
goto finish;
return 0;
/*
* TRANSLATORS: Make sure to include [Y] and [n] in your
* translation. The program will only accept English input
@ -329,21 +317,35 @@ static int bisect_next_check(const struct bisect_terms *terms,
*/
yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
if (starts_with(yesno, "N") || starts_with(yesno, "n"))
retval = -1;
goto finish;
}
if (!is_empty_or_missing_file(git_path_bisect_start())) {
retval = error(_(need_bad_and_good_revision_warning),
vocab_bad, vocab_good, vocab_bad, vocab_good);
} else {
retval = error(_(need_bisect_start_warning),
vocab_good, vocab_bad, vocab_good, vocab_bad);
return -1;
return 0;
}
finish:
free((void *) good_glob);
free((void *) bad_ref);
return retval;
if (!is_empty_or_missing_file(git_path_bisect_start()))
return error(_(need_bad_and_good_revision_warning),
vocab_bad, vocab_good, vocab_bad, vocab_good);
else
return error(_(need_bisect_start_warning),
vocab_good, vocab_bad, vocab_good, vocab_bad);
}
static int bisect_next_check(const struct bisect_terms *terms,
const char *current_term)
{
int missing_good = 1, missing_bad = 1;
char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
char *good_glob = xstrfmt("%s-*", terms->term_good);
if (ref_exists(bad_ref))
missing_bad = 0;
for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
(void *) &missing_good);
free(good_glob);
free(bad_ref);
return decide_next(terms, current_term, missing_good, missing_bad);
}
static int get_terms(struct bisect_terms *terms)
@ -397,7 +399,7 @@ static int bisect_terms(struct bisect_terms *terms, const char *option)
static int bisect_append_log_quoted(const char **argv)
{
int retval = 0;
int res = 0;
FILE *fp = fopen(git_path_bisect_log(), "a");
struct strbuf orig_args = STRBUF_INIT;
@ -405,25 +407,25 @@ static int bisect_append_log_quoted(const char **argv)
return -1;
if (fprintf(fp, "git bisect start") < 1) {
retval = -1;
res = -1;
goto finish;
}
sq_quote_argv(&orig_args, argv);
if (fprintf(fp, "%s\n", orig_args.buf) < 1)
retval = -1;
res = -1;
finish:
fclose(fp);
strbuf_release(&orig_args);
return retval;
return res;
}
static int bisect_start(struct bisect_terms *terms, int no_checkout,
const char **argv, int argc)
{
int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
int flags, pathspec_pos, retval = 0;
int flags, pathspec_pos, res = 0;
struct string_list revs = STRING_LIST_INIT_DUP;
struct string_list states = STRING_LIST_INIT_DUP;
struct strbuf start_head = STRBUF_INIT;
@ -524,7 +526,7 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,
argv_array_pushl(&argv, "checkout", start_head.buf,
"--", NULL);
if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
retval = error(_("checking out '%s' failed."
res = error(_("checking out '%s' failed."
" Try 'git bisect start "
"<valid-branch>'."),
start_head.buf);
@ -572,12 +574,12 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,
if (no_checkout) {
if (get_oid(start_head.buf, &oid) < 0) {
retval = error(_("invalid ref: '%s'"), start_head.buf);
res = error(_("invalid ref: '%s'"), start_head.buf);
goto finish;
}
if (update_ref(NULL, "BISECT_HEAD", &oid, NULL, 0,
UPDATE_REFS_MSG_ON_ERR)) {
retval = -1;
res = -1;
goto finish;
}
}
@ -589,26 +591,26 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,
for (i = 0; i < states.nr; i++)
if (bisect_write(states.items[i].string,
revs.items[i].string, terms, 1)) {
retval = -1;
res = -1;
goto finish;
}
if (must_write_terms && write_terms(terms->term_bad,
terms->term_good)) {
retval = -1;
res = -1;
goto finish;
}
retval = bisect_append_log_quoted(argv);
if (retval)
retval = -1;
res = bisect_append_log_quoted(argv);
if (res)
res = -1;
finish:
string_list_clear(&revs, 0);
string_list_clear(&states, 0);
strbuf_release(&start_head);
strbuf_release(&bisect_names);
return retval;
return res;
}
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
@ -664,7 +666,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
switch (cmdmode) {
case NEXT_ALL:
return bisect_next_all(the_repository, prefix, no_checkout);
res = bisect_next_all(the_repository, prefix, no_checkout);
break;
case WRITE_TERMS:
if (argc != 2)
return error(_("--write-terms requires two arguments"));
@ -711,5 +714,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
return error("BUG: unknown subcommand '%d'", cmdmode);
}
free_terms(&terms);
return !!res;
/*
* 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_OK;
return abs(res);
}