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 <ram...@ramsayjones.plus.com>
> Mentored-by: Lars Schneider <larsxschnei...@gmail.com>
> Mentored-by: Christian Couder <chrisc...@tuxfamily.org>
> Signed-off-by: Pranit Bauva <pranit.ba...@gmail.com>
> ---
>  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

Reply via email to