From: Eric Sunshine <sunshine@sunshineco.com>
To: Pranit Bauva <pranit.bauva@gmail.com>
Cc: "Git List" <git@vger.kernel.org>,
"Christian Couder" <christian.couder@gmail.com>,
"Christian Couder" <chriscool@tuxfamily.org>,
"Lars Schneider" <larsxschneider@gmail.com>,
"Torsten Bögershausen" <tboegi@web.de>
Subject: Re: [PATCH v2 6/6] bisect--helper: `bisect_write` shell function in C
Date: Thu, 16 Jun 2016 14:55:01 -0400 [thread overview]
Message-ID: <CAPig+cQV3FTGJBvS0Kuc3CNPiwZMZtApT6r8+Ojhw3y7O2VTiw@mail.gmail.com> (raw)
In-Reply-To: <20160615140026.10519-7-pranit.bauva@gmail.com>
On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva <pranit.bauva@gmail.com> 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 and 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. After the whole conversion, we can remove the extra
> arguments and make the method use the two variables from the global scope
> within the C code.
You could do this now rather than waiting for later. Instead of
passing these arguments to bisect_write(), create global variables in
this patch and assign them in the BISECT_WRITE case of
cmd_bisect__helper() before calling bisect_write().
Not necessarily worth a re-roll, but would save you the effort of
having to explain it here and then change it in some later patch.
> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> ---
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> @@ -192,6 +193,55 @@ static int check_expected_revs(const char **revs, int rev_nr)
> +static int bisect_write(const char *state, const char *rev,
> + const char *term_good, const char *term_bad,
> + int nolog)
> +{
> + struct strbuf tag = STRBUF_INIT;
> + struct strbuf commit_name = STRBUF_INIT;
> + struct object_id oid;
> + struct commit *commit;
> + struct pretty_print_context pp = {0};
> + FILE *fp;
> +
> + if (!strcmp(state, term_bad))
> + strbuf_addf(&tag, "refs/bisect/%s", state);
> + else if(one_of(state, term_good, "skip", NULL))
> + strbuf_addf(&tag, "refs/bisect/%s-%s", state, rev);
> + else
> + return error(_("Bad bisect_write argument: %s"), state);
> +
> + if (get_oid(rev, &oid)) {
> + strbuf_release(&tag);
> + return error(_("couldn't get the oid of the rev '%s'"), rev);
> + }
Minor: If you move the get_oid() conditional before the one above it,
then you won't have to worry about releasing 'strbuf tag' at this
point.
> + if (update_ref(NULL, tag.buf, oid.hash, NULL, 0,
> + UPDATE_REFS_MSG_ON_ERR)) {
> + strbuf_release(&tag);
> + return -1;
> + }
If you release 'strbuf tag' right here, after it's final use, then you
won't have to worry about releasing it anywhere below (particularly in
the error cases).
> + fp = fopen(git_path_bisect_log(), "a");
> + if (!fp) {
> + strbuf_release(&tag);
> + return error_errno(_("couldn't open the file '%s'"), git_path_bisect_log());
> + }
> +
> + commit = lookup_commit_reference(oid.hash);
> + format_commit_message(commit, "%s", &commit_name, &pp);
> + fprintf(fp, "# %s: [%s] %s\n", state, sha1_to_hex(oid.hash),
> + commit_name.buf);
> +
> + if (!nolog)
> + fprintf(fp, "git bisect %s %s\n", state, rev);
> +
> + strbuf_release(&commit_name);
> + strbuf_release(&tag);
> + fclose(fp);
> + return 0;
> +}
> @@ -241,6 +295,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> + case BISECT_WRITE:
> + if (argc != 4 && argc != 5)
> + die(_("--bisect-write requires either 4 or 5 arguments"));
> + nolog = (argc == 5) && !strcmp(argv[4], "nolog");
This is minor and won't matter in the long run when this code goes
away later in the C conversion, but this differs from the shell code
which only cared that a (non-empty) fifth argument was provided but
didn't care about the actual value, whereas this code expects the
argument to be exactly "nolog".
> + return bisect_write(argv[0], argv[1], argv[2], argv[3], nolog);
> default:
> die("BUG: unknown subcommand '%d'", cmdmode);
> }
next prev parent reply other threads:[~2016-06-16 18:55 UTC|newest]
Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-07 20:54 [PATCH 1/4] bisect--helper: `bisect_clean_state` shell function in C Pranit Bauva
2016-06-07 20:54 ` [PATCH 2/4] t6030: explicitly test for bisection cleanup Pranit Bauva
2016-06-07 23:21 ` Eric Sunshine
2016-06-08 8:07 ` Pranit Bauva
2016-06-08 8:17 ` Eric Sunshine
2016-06-08 10:20 ` Pranit Bauva
2016-06-07 20:54 ` [PATCH 3/4] dir: introduce file_size() to check the size of file Pranit Bauva
2016-06-08 7:37 ` Eric Sunshine
2016-06-08 7:57 ` Pranit Bauva
2016-06-08 8:13 ` Eric Sunshine
2016-06-08 10:03 ` Christian Couder
2016-06-08 8:17 ` Torsten Bögershausen
2016-06-08 13:08 ` Pranit Bauva
2016-06-12 10:44 ` Torsten Bögershausen
2016-06-13 6:21 ` Pranit Bauva
2016-06-07 20:54 ` [PATCH 4/4] bisect--helper: `bisect_reset` shell function in C Pranit Bauva
2016-06-08 7:59 ` Eric Sunshine
2016-06-08 9:51 ` Christian Couder
2016-06-08 9:53 ` Christian Couder
2016-06-08 17:50 ` Eric Sunshine
2016-06-08 13:20 ` Pranit Bauva
2016-06-08 17:53 ` Eric Sunshine
2016-06-08 18:04 ` Pranit Bauva
2016-06-07 22:31 ` [PATCH 1/4] bisect--helper: `bisect_clean_state` " Eric Sunshine
2016-06-08 1:51 ` Eric Sunshine
2016-06-08 7:46 ` Pranit Bauva
2016-06-08 8:02 ` Eric Sunshine
2016-06-08 8:09 ` Pranit Bauva
2016-06-08 9:41 ` Christian Couder
2016-06-08 17:59 ` Eric Sunshine
2016-06-08 18:04 ` Pranit Bauva
2016-06-15 14:00 ` [PATCH v2 0/6] convert various shell functions in git-bisect to C Pranit Bauva
2016-06-15 14:00 ` [PATCH v2 1/6] bisect--helper: `bisect_clean_state` shell function in C Pranit Bauva
2016-06-15 18:04 ` Eric Sunshine
2016-06-15 18:47 ` Pranit Bauva
2016-06-15 20:22 ` Eric Sunshine
2016-06-15 14:00 ` [PATCH v2 2/6] t6030: explicitly test for bisection cleanup Pranit Bauva
2016-06-15 14:00 ` [PATCH v2 3/6] wrapper: move is_empty_file() from builtin/am.c Pranit Bauva
2016-06-15 18:12 ` Junio C Hamano
2016-06-15 18:15 ` Pranit Bauva
2016-06-15 18:22 ` Eric Sunshine
2016-06-15 18:40 ` Pranit Bauva
2016-06-15 14:00 ` [PATCH v2 4/6] bisect--helper: `bisect_reset` shell function in C Pranit Bauva
2016-06-15 21:05 ` Eric Sunshine
2016-06-16 19:06 ` Pranit Bauva
2016-06-15 14:00 ` [PATCH v2 5/6] bisect--helper: `is_expected_rev` & `check_expected_revs` " Pranit Bauva
2016-06-15 21:14 ` Eric Sunshine
2016-06-16 19:05 ` Pranit Bauva
2016-06-16 19:16 ` Eric Sunshine
2016-06-16 19:25 ` Pranit Bauva
2016-06-16 20:47 ` Christian Couder
2016-06-17 12:49 ` Pranit Bauva
2016-06-15 14:00 ` [PATCH v2 6/6] bisect--helper: `bisect_write` " Pranit Bauva
2016-06-16 18:55 ` Eric Sunshine [this message]
2016-06-16 19:01 ` Pranit Bauva
2016-06-16 20:38 ` Christian Couder
2016-06-17 13:10 ` Pranit Bauva
2016-06-15 17:53 ` [PATCH v2 0/6] convert various shell functions in git-bisect to C Eric Sunshine
2016-06-15 18:09 ` Pranit Bauva
2016-06-26 12:23 ` [PATCH v3 " Pranit Bauva
2016-07-06 20:25 ` [PATCH v4 " Pranit Bauva
2016-07-06 20:25 ` [PATCH v4 1/6] bisect--helper: `bisect_clean_state` shell function in C Pranit Bauva
2016-07-06 20:25 ` [PATCH v4 2/6] t6030: explicitly test for bisection cleanup Pranit Bauva
2016-07-11 19:16 ` Junio C Hamano
2016-07-06 20:25 ` [PATCH v4 3/6] wrapper: move is_empty_file() and rename it as is_empty_or_missing_file() Pranit Bauva
2016-07-06 20:25 ` [PATCH v4 4/6] bisect--helper: `bisect_reset` shell function in C Pranit Bauva
2016-07-06 20:25 ` [PATCH v4 5/6] bisect--helper: `is_expected_rev` & `check_expected_revs` " Pranit Bauva
2016-07-06 20:25 ` [PATCH v4 6/6] bisect--helper: `bisect_write` " Pranit Bauva
2016-07-11 19:19 ` [PATCH v4 0/6] convert various shell functions in git-bisect to C Junio C Hamano
2016-06-26 12:23 ` [PATCH v3 1/6] bisect--helper: `bisect_clean_state` shell function in C Pranit Bauva
2016-06-26 12:23 ` [PATCH v3 2/6] t6030: explicitly test for bisection cleanup Pranit Bauva
2016-06-26 12:23 ` [PATCH v3 3/6] wrapper: move is_empty_file() and rename it as is_empty_or_missing_file() Pranit Bauva
2016-06-26 12:23 ` [PATCH v3 4/6] bisect--helper: `bisect_reset` shell function in C Pranit Bauva
2016-06-26 12:23 ` [PATCH v3 5/6] bisect--helper: `is_expected_rev` & `check_expected_revs` " Pranit Bauva
2016-06-26 12:23 ` [PATCH v3 6/6] bisect--helper: `bisect_write` " Pranit Bauva
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPig+cQV3FTGJBvS0Kuc3CNPiwZMZtApT6r8+Ojhw3y7O2VTiw@mail.gmail.com \
--to=sunshine@sunshineco.com \
--cc=chriscool@tuxfamily.org \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=larsxschneider@gmail.com \
--cc=pranit.bauva@gmail.com \
--cc=tboegi@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).