Skip to content

Commit

Permalink
Merge branch 'mr/bisect-in-c-2'
Browse files Browse the repository at this point in the history
Rewrite of the "git bisect" script in C continues.

* mr/bisect-in-c-2:
  bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C
  bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'
  bisect--helper: reimplement `bisect_autostart` shell function in C
  bisect--helper: introduce new `write_in_file()` function
  bisect--helper: use '-res' in 'cmd_bisect__helper' return
  bisect--helper: BUG() in cmd_*() on invalid subcommand
  • Loading branch information
gitster committed Oct 4, 2020
2 parents 03a0182 + 517ecb3 commit f4cc68c
Show file tree
Hide file tree
Showing 3 changed files with 274 additions and 78 deletions.
13 changes: 11 additions & 2 deletions bisect.c
Original file line number Diff line number Diff line change
Expand Up @@ -988,8 +988,11 @@ void read_bisect_terms(const char **read_bad, const char **read_good)
* the bisection process finished successfully.
* 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.
*
* Checking BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND
* in bisect_helper::bisect_next() and only transforming it to 0 at
* the end of bisect_helper::cmd_bisect__helper() helps bypassing
* all the code related to finding a commit to test.
*/
enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
{
Expand All @@ -999,6 +1002,10 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
enum bisect_error res = BISECT_OK;
struct object_id *bisect_rev;
char *steps_msg;
/*
* If no_checkout is non-zero, the bisection process does not
* checkout the trial commit but instead simply updates BISECT_HEAD.
*/
int no_checkout = ref_exists("BISECT_HEAD");
unsigned bisect_flags = 0;

Expand Down Expand Up @@ -1082,6 +1089,8 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
"Bisecting: %d revisions left to test after this %s\n",
nr), nr, steps_msg);
free(steps_msg);
/* Clean up objects used, as they will be reused. */
clear_commit_marks_all(ALL_REV_FLAGS);

return bisect_checkout(bisect_rev, no_checkout);
}
Expand Down
269 changes: 258 additions & 11 deletions builtin/bisect--helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "run-command.h"
#include "prompt.h"
#include "quote.h"
#include "revision.h"

static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
Expand All @@ -29,9 +30,17 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
N_("git bisect--helper --bisect-start [--term-{new,bad}=<term> --term-{old,good}=<term>]"
" [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]"),
N_("git bisect--helper --bisect-next"),
N_("git bisect--helper --bisect-auto-next"),
N_("git bisect--helper --bisect-autostart"),
NULL
};

struct add_bisect_ref_data {
struct rev_info *revs;
unsigned int object_flags;
};

struct bisect_terms {
char *term_good;
char *term_bad;
Expand All @@ -55,6 +64,8 @@ static void set_terms(struct bisect_terms *terms, const char *bad,
static const char vocab_bad[] = "bad|new";
static const char vocab_good[] = "good|old";

static int bisect_autostart(struct bisect_terms *terms);

/*
* Check whether the string `term` belongs to the set of strings
* included in the variable arguments.
Expand All @@ -74,6 +85,52 @@ static int one_of(const char *term, ...)
return res;
}

static int write_in_file(const char *path, const char *mode, const char *format, va_list args)
{
FILE *fp = NULL;
int res = 0;

if (strcmp(mode, "w") && strcmp(mode, "a"))
BUG("write-in-file does not support '%s' mode", mode);
fp = fopen(path, mode);
if (!fp)
return error_errno(_("cannot open file '%s' in mode '%s'"), path, mode);
res = vfprintf(fp, format, args);

if (res < 0) {
int saved_errno = errno;
fclose(fp);
errno = saved_errno;
return error_errno(_("could not write to file '%s'"), path);
}

return fclose(fp);
}

static int write_to_file(const char *path, const char *format, ...)
{
int res;
va_list args;

va_start(args, format);
res = write_in_file(path, "w", format, args);
va_end(args);

return res;
}

static int append_to_file(const char *path, const char *format, ...)
{
int res;
va_list args;

va_start(args, format);
res = write_in_file(path, "a", format, args);
va_end(args);

return res;
}

static int check_term_format(const char *term, const char *orig_term)
{
int res;
Expand Down Expand Up @@ -104,7 +161,6 @@ static int check_term_format(const char *term, const char *orig_term)

static int write_terms(const char *bad, const char *good)
{
FILE *fp = NULL;
int res;

if (!strcmp(bad, good))
Expand All @@ -113,13 +169,9 @@ static int write_terms(const char *bad, const char *good)
if (check_term_format(bad, "bad") || check_term_format(good, "good"))
return -1;

fp = fopen(git_path_bisect_terms(), "w");
if (!fp)
return error_errno(_("could not open the file BISECT_TERMS"));
res = write_to_file(git_path_bisect_terms(), "%s\n%s\n", bad, good);

res = fprintf(fp, "%s\n%s\n", bad, good);
res |= fclose(fp);
return (res < 0) ? -1 : 0;
return res;
}

static int is_expected_rev(const char *expected_hex)
Expand Down Expand Up @@ -421,6 +473,142 @@ static int bisect_append_log_quoted(const char **argv)
return res;
}

static int add_bisect_ref(const char *refname, const struct object_id *oid,
int flags, void *cb)
{
struct add_bisect_ref_data *data = cb;

add_pending_oid(data->revs, refname, oid, data->object_flags);

return 0;
}

static int prepare_revs(struct bisect_terms *terms, struct rev_info *revs)
{
int res = 0;
struct add_bisect_ref_data cb = { revs };
char *good = xstrfmt("%s-*", terms->term_good);

/*
* We cannot use terms->term_bad directly in
* for_each_glob_ref_in() and we have to append a '*' to it,
* otherwise for_each_glob_ref_in() will append '/' and '*'.
*/
char *bad = xstrfmt("%s*", terms->term_bad);

/*
* It is important to reset the flags used by revision walks
* as the previous call to bisect_next_all() in turn
* sets up a revision walk.
*/
reset_revision_walk();
init_revisions(revs, NULL);
setup_revisions(0, NULL, revs, NULL);
for_each_glob_ref_in(add_bisect_ref, bad, "refs/bisect/", &cb);
cb.object_flags = UNINTERESTING;
for_each_glob_ref_in(add_bisect_ref, good, "refs/bisect/", &cb);
if (prepare_revision_walk(revs))
res = error(_("revision walk setup failed\n"));

free(good);
free(bad);
return res;
}

static int bisect_skipped_commits(struct bisect_terms *terms)
{
int res;
FILE *fp = NULL;
struct rev_info revs;
struct commit *commit;
struct pretty_print_context pp = {0};
struct strbuf commit_name = STRBUF_INIT;

res = prepare_revs(terms, &revs);
if (res)
return res;

fp = fopen(git_path_bisect_log(), "a");
if (!fp)
return error_errno(_("could not open '%s' for appending"),
git_path_bisect_log());

if (fprintf(fp, "# only skipped commits left to test\n") < 0)
return error_errno(_("failed to write to '%s'"), git_path_bisect_log());

while ((commit = get_revision(&revs)) != NULL) {
strbuf_reset(&commit_name);
format_commit_message(commit, "%s",
&commit_name, &pp);
fprintf(fp, "# possible first %s commit: [%s] %s\n",
terms->term_bad, oid_to_hex(&commit->object.oid),
commit_name.buf);
}

/*
* Reset the flags used by revision walks in case
* there is another revision walk after this one.
*/
reset_revision_walk();

strbuf_release(&commit_name);
fclose(fp);
return 0;
}

static int bisect_successful(struct bisect_terms *terms)
{
struct object_id oid;
struct commit *commit;
struct pretty_print_context pp = {0};
struct strbuf commit_name = STRBUF_INIT;
char *bad_ref = xstrfmt("refs/bisect/%s",terms->term_bad);
int res;

read_ref(bad_ref, &oid);
commit = lookup_commit_reference_by_name(bad_ref);
format_commit_message(commit, "%s", &commit_name, &pp);

res = append_to_file(git_path_bisect_log(), "# first %s commit: [%s] %s\n",
terms->term_bad, oid_to_hex(&commit->object.oid),
commit_name.buf);

strbuf_release(&commit_name);
free(bad_ref);
return res;
}

static enum bisect_error bisect_next(struct bisect_terms *terms, const char *prefix)
{
enum bisect_error res;

if (bisect_autostart(terms))
return BISECT_FAILED;

if (bisect_next_check(terms, terms->term_good))
return BISECT_FAILED;

/* Perform all bisection computation */
res = bisect_next_all(the_repository, prefix);

if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
res = bisect_successful(terms);
return res ? res : BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;
} else if (res == BISECT_ONLY_SKIPPED_LEFT) {
res = bisect_skipped_commits(terms);
return res ? res : BISECT_ONLY_SKIPPED_LEFT;
}
return res;
}

static enum bisect_error bisect_auto_next(struct bisect_terms *terms, const char *prefix)
{
if (bisect_next_check(terms, NULL))
return BISECT_OK;

return bisect_next(terms, prefix);
}

static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)
{
int no_checkout = 0;
Expand Down Expand Up @@ -623,6 +811,38 @@ static int bisect_start(struct bisect_terms *terms, const char **argv, int argc)
return res;
}

static inline int file_is_not_empty(const char *path)
{
return !is_empty_or_missing_file(path);
}

static int bisect_autostart(struct bisect_terms *terms)
{
int res;
const char *yesno;

if (file_is_not_empty(git_path_bisect_start()))
return 0;

fprintf_ln(stderr, _("You need to start by \"git bisect "
"start\"\n"));

if (!isatty(STDIN_FILENO))
return -1;

/*
* TRANSLATORS: Make sure to include [Y] and [n] in your
* translation. The program will only accept English input
* at this point.
*/
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);

return res;
}

int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
Expand All @@ -635,7 +855,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
CHECK_AND_SET_TERMS,
BISECT_NEXT_CHECK,
BISECT_TERMS,
BISECT_START
BISECT_START,
BISECT_AUTOSTART,
BISECT_NEXT,
BISECT_AUTO_NEXT
} cmdmode = 0;
int res = 0, nolog = 0;
struct option options[] = {
Expand All @@ -659,6 +882,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("print out the bisect terms"), BISECT_TERMS),
OPT_CMDMODE(0, "bisect-start", &cmdmode,
N_("start the bisect session"), BISECT_START),
OPT_CMDMODE(0, "bisect-next", &cmdmode,
N_("find the next bisection commit"), BISECT_NEXT),
OPT_CMDMODE(0, "bisect-auto-next", &cmdmode,
N_("verify the next bisection state then checkout the next bisection commit"), BISECT_AUTO_NEXT),
OPT_CMDMODE(0, "bisect-autostart", &cmdmode,
N_("start the bisection if it has not yet been started"), BISECT_AUTOSTART),
OPT_BOOL(0, "no-log", &nolog,
N_("no log for BISECT_WRITE")),
OPT_END()
Expand Down Expand Up @@ -718,17 +947,35 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
set_terms(&terms, "bad", "good");
res = bisect_start(&terms, argv, argc);
break;
case BISECT_NEXT:
if (argc)
return error(_("--bisect-next requires 0 arguments"));
get_terms(&terms);
res = bisect_next(&terms, prefix);
break;
case BISECT_AUTO_NEXT:
if (argc)
return error(_("--bisect-auto-next requires 0 arguments"));
get_terms(&terms);
res = bisect_auto_next(&terms, prefix);
break;
case BISECT_AUTOSTART:
if (argc)
return error(_("--bisect-autostart does not accept arguments"));
set_terms(&terms, "bad", "good");
res = bisect_autostart(&terms);
break;
default:
return error("BUG: unknown subcommand '%d'", cmdmode);
BUG("unknown subcommand %d", cmdmode);
}
free_terms(&terms);

/*
* Handle early success
* From check_merge_bases > check_good_are_ancestors_of_bad > bisect_next_all
*/
if (res == BISECT_INTERNAL_SUCCESS_MERGE_BASE)
if ((res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) || (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND))
res = BISECT_OK;

return abs(res);
return -res;
}
Loading

0 comments on commit f4cc68c

Please sign in to comment.