Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
From: Ramsay Jones <hidden>
Date: 2017-11-08 00:26:40
On 27/10/17 16:06, Pranit Bauva wrote:
quoted hunk ↗ jump to hunk
Reimplement the `bisect_write` shell function in C and add a `bisect-write` subcommand to `git bisect--helper` to call it from git-bisect.sh Using `--bisect-write` subcommand is a temporary measure to port shell function in C so as to use the existing test suite. As more functions are ported, this subcommand will be retired but its implementation will be called by some other methods. Note: bisect_write() uses two variables namely TERM_GOOD and TERM_BAD from the global shell script thus we need to pass it to the subcommand using the arguments. We then store them in a struct bisect_terms and pass the memory address around functions. Add a log_commit() helper function to write the contents of the commit message header to a file which will be re-used in future parts of the code as well. Also introduce a function free_terms() to free the memory of `struct bisect_terms` and set_terms() to set the values of members in `struct bisect_terms`. Helped-by: Ramsay Jones [off-list ref] Mentored-by: Lars Schneider [off-list ref] Mentored-by: Christian Couder [off-list ref] Signed-off-by: Pranit Bauva <redacted> --- builtin/bisect--helper.c | 107 +++++++++++++++++++++++++++++++++++++++++++++-- git-bisect.sh | 25 ++--------- 2 files changed, 108 insertions(+), 24 deletions(-)diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 12754448f7b6a..6295f53c850a8 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c@@ -12,15 +12,37 @@ 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_head, "BISECT_HEAD") +static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG") static const char * const git_bisect_helper_usage[] = { N_("git bisect--helper --next-all [--no-checkout]"), N_("git bisect--helper --write-terms <bad_term> <good_term>"), N_("git bisect--helper --bisect-clean-state"), N_("git bisect--helper --bisect-reset [<commit>]"), + N_("git bisect--helper --bisect-write <state> <revision> <good_term> <bad_term> [<nolog>]"), NULL }; +struct bisect_terms { + const char *term_good; + const char *term_bad; +}; + +static void free_terms(struct bisect_terms *terms) +{ + if (!terms->term_good) + free((void *) terms->term_good);> + if (!terms->term_bad) + free((void *) terms->term_bad);
You can pass a NULL pointer to free(), so the conditionals here are not required.
quoted hunk ↗ jump to hunk
+} + +static void set_terms(struct bisect_terms *terms, const char *bad, + const char *good) +{ + terms->term_good = xstrdup(good); + terms->term_bad = xstrdup(bad); +} + /* * Check whether the string `term` belongs to the set of strings * included in the variable arguments.@@ -146,6 +168,72 @@ static int bisect_reset(const char *commit) return bisect_clean_state(); } +static void log_commit(FILE *fp, char *fmt, const char *state, + struct commit *commit) +{ + struct pretty_print_context pp = {0}; + struct strbuf commit_msg = STRBUF_INIT; + char *label = xstrfmt(fmt, state); + + format_commit_message(commit, "%s", &commit_msg, &pp); + + fprintf(fp, "# %s: [%s] %s\n", label, oid_to_hex(&commit->object.oid), + commit_msg.buf); + + strbuf_release(&commit_msg); + free(label); +} + +static int bisect_write(const char *state, const char *rev, + const struct bisect_terms *terms, int nolog) +{ + struct strbuf tag = STRBUF_INIT; + struct object_id oid; + struct commit *commit; + FILE *fp = NULL; + int retval = 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 { + error(_("Bad bisect_write argument: %s"), state); + goto fail; + } + + if (get_oid(rev, &oid)) { + error(_("couldn't get the oid of the rev '%s'"), rev); + goto fail; + } + + if (update_ref(NULL, tag.buf, &oid, NULL, 0, + UPDATE_REFS_MSG_ON_ERR)) + goto fail; + + fp = fopen(git_path_bisect_log(), "a"); + if (!fp) { + error_errno(_("couldn't open the file '%s'"), git_path_bisect_log()); + goto fail; + } + + commit = lookup_commit_reference(&oid); + log_commit(fp, "%s", state, commit); + + if (!nolog) + fprintf(fp, "git bisect %s %s\n", state, rev); + + goto finish; + +fail: + retval = -1; +finish: + if (fp) + fclose(fp); + strbuf_release(&tag); + return retval; +} + int cmd_bisect__helper(int argc, const char **argv, const char *prefix) { enum {@@ -153,9 +241,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) WRITE_TERMS, BISECT_CLEAN_STATE, CHECK_EXPECTED_REVS, - BISECT_RESET + BISECT_RESET, + BISECT_WRITE } cmdmode = 0; - int no_checkout = 0; + int no_checkout = 0, res = 0; struct option options[] = { OPT_CMDMODE(0, "next-all", &cmdmode, N_("perform 'git bisect next'"), NEXT_ALL),@@ -167,10 +256,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) N_("check for expected revs"), CHECK_EXPECTED_REVS), OPT_CMDMODE(0, "bisect-reset", &cmdmode, N_("reset the bisection state"), BISECT_RESET), + OPT_CMDMODE(0, "bisect-write", &cmdmode, + N_("update the refs according to the bisection state and may write it to BISECT_LOG"), BISECT_WRITE),
Is the "... and may write it to" necessary? ;-) [The original was just: "write out the bisection state in BISECT_LOG"] ATB, Ramsay Jones