On 27/10/17 16:06, Pranit Bauva wrote:
> 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 <ramsay@xxxxxxxxxxxxxxxxxxxx>
> Mentored-by: Lars Schneider <larsxschneider@xxxxxxxxx>
> Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> Signed-off-by: Pranit Bauva <pranit.bauva@xxxxxxxxx>
> ---
> 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.
> +}
> +
> +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
