From: Junio C Hamano <gitster@pobox.com>
To: Jeff Hostetler <git@jeffhostetler.com>
Cc: git@vger.kernel.org, peff@peff.net,
Jeff Hostetler <jeffhost@microsoft.com>
Subject: Re: [PATCH v3 1/5] stat_tracking_info: return +1 when branches not equal
Date: Thu, 04 Jan 2018 13:41:42 -0800 [thread overview]
Message-ID: <xmqqr2r5e1yh.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20180103214733.797-2-git@jeffhostetler.com> (Jeff Hostetler's message of "Wed, 3 Jan 2018 21:47:29 +0000")
Jeff Hostetler <git@jeffhostetler.com> writes:
> - if (stat_tracking_info(branch, &num_ours,
> - &num_theirs, NULL)) {
> + if (stat_tracking_info(branch, &num_ours, &num_theirs,
> + NULL, AHEAD_BEHIND_FULL) < 0) {
> ...
> - if (stat_tracking_info(branch, &num_ours,
> - &num_theirs, NULL))
> + if (stat_tracking_info(branch, &num_ours, &num_theirs,
> + NULL, AHEAD_BEHIND_FULL) < 0)
Mental note: any code that reacted to stat_tracking_info() returning
non-zero was reacting to "no useful info in num_{ours,theirs}".
They now have to compare the returned value with "< 0" for the same
purpose.
> ...
> * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no
> - * upstream defined, or ref does not exist), 0 otherwise.
> + * upstream defined, or ref does not exist). Returns 0 if the commits are
> + * identical. Returns 1 if commits are different.
> */
> int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
> - const char **upstream_name)
> + const char **upstream_name, enum ahead_behind_flags abf)
> {
> struct object_id oid;
> struct commit *ours, *theirs;
> @@ -2019,6 +2026,8 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
> *num_theirs = *num_ours = 0;
> return 0;
> }
> + if (abf == AHEAD_BEHIND_QUICK)
> + return 1;
> ...
> argv_array_clear(&argv);
> - return 0;
> + return 1;
> }
When a caller gets +1 from the function, it is not safe to peek into
*num_{ours,theirs} if it passed _QUICK.
> @@ -2064,7 +2073,8 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
> - if (stat_tracking_info(branch, &ours, &theirs, &full_base) < 0) {
> + if (stat_tracking_info(branch, &ours, &theirs, &full_base,
> + AHEAD_BEHIND_FULL) < 0) {
Sane conversion to the new return value convention.
> diff --git a/wt-status.c b/wt-status.c
> index 94e5eba..8f7fdc6 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1791,7 +1791,8 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)
>
> color_fprintf(s->fp, branch_color_local, "%s", branch_name);
>
> - if (stat_tracking_info(branch, &num_ours, &num_theirs, &base) < 0) {
> + if (stat_tracking_info(branch, &num_ours, &num_theirs, &base,
> + AHEAD_BEHIND_FULL) < 0) {
Ditto.
> @@ -1928,7 +1929,8 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)
> /* Lookup stats on the upstream tracking branch, if set. */
> branch = branch_get(branch_name);
> base = NULL;
> - ab_info = (stat_tracking_info(branch, &nr_ahead, &nr_behind, &base) == 0);
> + ab_info = (stat_tracking_info(branch, &nr_ahead, &nr_behind,
> + &base, AHEAD_BEHIND_FULL) >= 0);
If a later step plans to (conditionally) allow _QUICK to be passed
here, this conversion is questionable, because ab_info being true
no longer is a sign that nr_{ahead,behind} are valid.
I suspect that moving the "s/ab_info/sti/" bits around here from
step [2/5] to this commit may make the result after this patch more
consistent, but it is not a big deal either way.
> if (base) {
> base = shorten_unambiguous_ref(base, 0);
> fprintf(s->fp, "# branch.upstream %s%c", base, eol);
next prev parent reply other threads:[~2018-01-04 21:41 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-03 21:47 [PATCH v3 0/5] Add --no-ahead-behind to status Jeff Hostetler
2018-01-03 21:47 ` [PATCH v3 1/5] stat_tracking_info: return +1 when branches not equal Jeff Hostetler
2018-01-04 21:41 ` Junio C Hamano [this message]
2018-01-03 21:47 ` [PATCH v3 2/5] status: add --[no-]ahead-behind to status and commit for V2 format Jeff Hostetler
2018-01-04 22:05 ` Junio C Hamano
2018-01-05 16:31 ` Jeff Hostetler
2018-01-03 21:47 ` [PATCH v3 3/5] status: update short status to respect --no-ahead-behind Jeff Hostetler
2018-01-03 21:47 ` [PATCH v3 4/5] status: support --no-ahead-behind in long format Jeff Hostetler
2018-01-03 21:47 ` [PATCH v3 5/5] status: add status.aheadBehind value for porcelain output Jeff Hostetler
2018-01-04 23:06 ` [PATCH v3 0/5] Add --no-ahead-behind to status Jeff King
2018-01-05 16:46 ` Jeff Hostetler
2018-01-05 19:56 ` Junio C Hamano
2018-01-08 6:37 ` Jeff King
2018-01-08 14:22 ` Jeff Hostetler
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=xmqqr2r5e1yh.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@jeffhostetler.com \
--cc=git@vger.kernel.org \
--cc=jeffhost@microsoft.com \
--cc=peff@peff.net \
/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).