Re: [PATCH v6 2/3] branch: report invalid tracking branch as broken

2013-08-15 Thread Junio C Hamano
Jiang Xin  writes:

>  /*
> - * Return false if cannot stat a tracking branch (not exist or invalid),
> - * otherwise true.
> + * Compare a branch with its tracking branch, and save their differences
> + * (number of commits) in *num_ours and *num_theirs.
> + *
> + * Return 0 if branch has no upstream, -1 if upstream is missing or invalid,
> + * otherwise 1.
>   */

What is the difference between a branch that has no upstream and
upstream being missing?  Or between missing and invalid?

I think you are trying to say the difference between
"branch..merge is not set at all" and "branch..merge is
in the configuration, but the named upstream ref does not exist".

You are calling the latter "missing or invalid", but how does one
tell missing ones from invalid ones?  I think there isn't a
distinction, so it would be better to just say "missing" (or "gone",
which is very much more likely reason why you still have
configuration without a ref).

I am not sure it is a good idea to label "missing" as "broken" or
"invalid", but it seems that your tests, in code comments and
variable names are full of these negative connotations.

Hmph...


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 2/3] branch: report invalid tracking branch as broken

2013-08-15 Thread Junio C Hamano
Jiang Xin  writes:

> If a branch has been set to track a upstream, but the upstream branch
> is missing or invalid, the tracking info is silently ignored in the
> output of some commands such as "git branch -vv" and "git status",
> as if there were no such tracking settings.
>
> Junio suggested broken upstream should be reported [1]. E.g.
>
> $ git branch -v -v
>   mastere67ac84 initial
> * topic 3fc0f2a [topicbase: broken] topic

I'd assume this is s/broken/gone/ to match what the rest of the log
message says?

> $ git status
> # On branch topic
> # Your branch is based on a broken ref 'topicbase'.
> #   (use "git branch --unset-upstream" to fixup)
> ...
>
> $ git status -b -s
> ## topic...topicbase [broken]
> ...
>
> In order to do like that, we need to distinguish these three cases
> (i.e. no tracking, with configured but no longer valid tracking, and
> with tracking) in function stat_tracking_info(). So the refactored
> function stat_tracking_info() has three return values: -1 (with "gone"
> base), 0 (no base), and 1 (with base).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 2/3] branch: report invalid tracking branch as broken

2013-08-15 Thread Jiang Xin
If a branch has been set to track a upstream, but the upstream branch
is missing or invalid, the tracking info is silently ignored in the
output of some commands such as "git branch -vv" and "git status",
as if there were no such tracking settings.

Junio suggested broken upstream should be reported [1]. E.g.

$ git branch -v -v
  mastere67ac84 initial
* topic 3fc0f2a [topicbase: broken] topic

$ git status
# On branch topic
# Your branch is based on a broken ref 'topicbase'.
#   (use "git branch --unset-upstream" to fixup)
...

$ git status -b -s
## topic...topicbase [broken]
...

In order to do like that, we need to distinguish these three cases
(i.e. no tracking, with configured but no longer valid tracking, and
with tracking) in function stat_tracking_info(). So the refactored
function stat_tracking_info() has three return values: -1 (with "gone"
base), 0 (no base), and 1 (with base).

[1]: http://thread.gmane.org/gmane.comp.version-control.git/231830/focus=232288

Suggested-by: Junio C Hamano 
Signed-off-by: Jiang Xin 
---
 builtin/branch.c | 17 +++--
 remote.c | 43 ++
 t/t6040-tracking-info.sh | 49 ++--
 wt-status.c  | 27 --
 4 files changed, 110 insertions(+), 26 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 3e016a6..247785e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -423,9 +423,19 @@ static void fill_tracking_info(struct strbuf *stat, const 
char *branch_name,
char *ref = NULL;
struct branch *branch = branch_get(branch_name);
struct strbuf fancy = STRBUF_INIT;
+   int broken_upstream = 0;
 
-   if (!stat_tracking_info(branch, &ours, &theirs))
+   switch (stat_tracking_info(branch, &ours, &theirs)) {
+   case 0:
+   /* Not set upstream. */
return;
+   case -1:
+   /* Upstream is missing or invalid. */
+   broken_upstream = 1;
+   break;
+   default:
+   break;
+   }
 
if (show_upstream_ref) {
ref = shorten_unambiguous_ref(branch->merge[0]->dst, 0);
@@ -437,7 +447,10 @@ static void fill_tracking_info(struct strbuf *stat, const 
char *branch_name,
strbuf_addstr(&fancy, ref);
}
 
-   if (!ours && !theirs) {
+   if (broken_upstream) {
+   if (show_upstream_ref)
+   strbuf_addf(stat, _("[%s: broken]"), fancy.buf);
+   } else if (!ours && !theirs) {
if (show_upstream_ref)
strbuf_addf(stat, _("[%s]"), fancy.buf);
} else if (!ours) {
diff --git a/remote.c b/remote.c
index 26bd543..aa87381 100644
--- a/remote.c
+++ b/remote.c
@@ -1729,8 +1729,11 @@ int ref_newer(const unsigned char *new_sha1, const 
unsigned char *old_sha1)
 }
 
 /*
- * Return false if cannot stat a tracking branch (not exist or invalid),
- * otherwise true.
+ * Compare a branch with its tracking branch, and save their differences
+ * (number of commits) in *num_ours and *num_theirs.
+ *
+ * Return 0 if branch has no upstream, -1 if upstream is missing or invalid,
+ * otherwise 1.
  */
 int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs)
 {
@@ -1749,16 +1752,16 @@ int stat_tracking_info(struct branch *branch, int 
*num_ours, int *num_theirs)
/* Cannot stat if what we used to build on no longer exists */
base = branch->merge[0]->dst;
if (read_ref(base, sha1))
-   return 0;
+   return -1;
theirs = lookup_commit_reference(sha1);
if (!theirs)
-   return 0;
+   return -1;
 
if (read_ref(branch->refname, sha1))
-   return 0;
+   return -1;
ours = lookup_commit_reference(sha1);
if (!ours)
-   return 0;
+   return -1;
 
/* are we the same? */
if (theirs == ours) {
@@ -1808,17 +1811,33 @@ int format_tracking_info(struct branch *branch, struct 
strbuf *sb)
 {
int ours, theirs;
const char *base;
+   int broken_upstream = 0;
 
-   if (!stat_tracking_info(branch, &ours, &theirs))
-   return 0;
-
-   /* Nothing to report if neither side has changes. */
-   if (!ours && !theirs)
+   switch (stat_tracking_info(branch, &ours, &theirs)) {
+   case 0:
+   /* Not set upstream. */
return 0;
+   case -1:
+   /* Upstream is missing or invalid. */
+   broken_upstream = 1;
+   break;
+   default:
+   /* Nothing to report if neither side has changes. */
+   if (!ours && !theirs)
+   return 0;
+   break;
+   }
 
base = branch->merge[0]->dst;
base = shorten_unambig