All of lore.kernel.org
 help / color / mirror / Atom feed
* Tracking of local branches
@ 2009-03-20 14:22 Michael J Gruber
  2009-03-20 16:13 ` Michael J Gruber
  2009-03-20 16:46 ` Junio C Hamano
  0 siblings, 2 replies; 15+ messages in thread
From: Michael J Gruber @ 2009-03-20 14:22 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Johannes Schindelin

Hi there,

me again. In connection with Dscho's recent patch which rang the bell on
tracked branches I noticed that local branches are treated somewhat
inconsistently by git. There are 2 ways to fix it, and I ask you for
your input on which one to choose.

First of all:
The documentation seems to imply that it's okay to follow local
branches, i.e. to have tracked local branches. Specifically, the option
--track allows setting up tracking info (branch.foo.merge) in cases
where it's not set up automatically (it is when you branch off a remote
tracking branch).

If it's not OK to say "git checkout -b newbranch --track local" when
local is a local branch you can stop reading here and tell me to stop
writing...

Now, assuming it's okay to have a local branch being tracked, the
current situation is:

git fetch/pull is okay (respects the setting)
git status/checkout/rev-parse BEL is not (acts as if there is no
tracking info)

I think I have tracked it down (pun intended) to the fact that one sort
of commands looks at the struct member branch->merge, the other at
branch->merge_name. The latter is set for branches which follow
something, the former only for followers of remote branches.

I semi-successfully messed around in remote.c (format_tracking_info(),
stat_tracking_info()) to make it use branch->merge_name rather than
branch->merge. This makes "git status" work as expected ("Your branch
is... severely screwed.") for tracked local branches. (It's messed up
for remote ones but hey it was a first shot; merge[0]->dst is really
needed here I guess.)

Now I could go after sha1_name.c and do the same,

OR

make it so that all branches have their merge member set up, uhm. Any
possible side effects?

What do you think?
Michael

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Tracking of local branches
  2009-03-20 14:22 Tracking of local branches Michael J Gruber
@ 2009-03-20 16:13 ` Michael J Gruber
  2009-03-20 16:46 ` Junio C Hamano
  1 sibling, 0 replies; 15+ messages in thread
From: Michael J Gruber @ 2009-03-20 16:13 UTC (permalink / raw)
  Cc: Git Mailing List, Junio C Hamano, Johannes Schindelin

Michael J Gruber venit, vidit, dixit 20.03.2009 15:22:
> Hi there,
> 
> me again. In connection with Dscho's recent patch which rang the bell on
> tracked branches I noticed that local branches are treated somewhat
> inconsistently by git. There are 2 ways to fix it, and I ask you for
> your input on which one to choose.
> 
> First of all:
> The documentation seems to imply that it's okay to follow local
> branches, i.e. to have tracked local branches. Specifically, the option
> --track allows setting up tracking info (branch.foo.merge) in cases
> where it's not set up automatically (it is when you branch off a remote
> tracking branch).
> 
> If it's not OK to say "git checkout -b newbranch --track local" when
> local is a local branch you can stop reading here and tell me to stop
> writing...
> 
> Now, assuming it's okay to have a local branch being tracked, the
> current situation is:
> 
> git fetch/pull is okay (respects the setting)
> git status/checkout/rev-parse BEL is not (acts as if there is no
> tracking info)
> 
> I think I have tracked it down (pun intended) to the fact that one sort
> of commands looks at the struct member branch->merge, the other at
> branch->merge_name. The latter is set for branches which follow
> something, the former only for followers of remote branches.
> 
> I semi-successfully messed around in remote.c (format_tracking_info(),
> stat_tracking_info()) to make it use branch->merge_name rather than
> branch->merge. This makes "git status" work as expected ("Your branch
> is... severely screwed.") for tracked local branches. (It's messed up
> for remote ones but hey it was a first shot; merge[0]->dst is really
> needed here I guess.)
> 
> Now I could go after sha1_name.c and do the same,
> 
> OR
> 
> make it so that all branches have their merge member set up, uhm. Any
> possible side effects?
> 
> What do you think?
> Michael

OK, I think I got this working with approach 2 above. All existing tests
pass. Now I'll cook up tests which only my new code passes ;) But that
may take a few days.

Michael

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Tracking of local branches
  2009-03-20 14:22 Tracking of local branches Michael J Gruber
  2009-03-20 16:13 ` Michael J Gruber
@ 2009-03-20 16:46 ` Junio C Hamano
  2009-03-20 18:10   ` Daniel Barkalow
  1 sibling, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2009-03-20 16:46 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List, Johannes Schindelin, Daniel Barkalow

Michael J Gruber <git@drmicha.warpmail.net> writes:

> I semi-successfully messed around in remote.c (format_tracking_info(),
> stat_tracking_info()) to make it use branch->merge_name rather than
> branch->merge. This makes "git status" work as expected ("Your branch
> is... severely screwed.") for tracked local branches. (It's messed up
> for remote ones but hey it was a first shot; merge[0]->dst is really
> needed here I guess.)
>
> Now I could go after sha1_name.c and do the same,
>
> OR
>
> make it so that all branches have their merge member set up, uhm. Any
> possible side effects?

My gut feeling is that the latter if works should be preferable for
consistency if nothing else.

The "struct branch" hasn't changed ever since it was introduced by cf81834
(Report information on branches from remote.h, 2007-09-10) and Daniel
might know about some corner cases that rely on branch.merge not being set
up for local ones, but honestly, I would think it would be a bug in the
existing code if there were such cases.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Tracking of local branches
  2009-03-20 16:46 ` Junio C Hamano
@ 2009-03-20 18:10   ` Daniel Barkalow
  2009-03-26 20:53     ` [PATCH 0/2] Make local branches behave like remote branches when --tracked Michael J Gruber
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Barkalow @ 2009-03-20 18:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael J Gruber, Git Mailing List, Johannes Schindelin

On Fri, 20 Mar 2009, Junio C Hamano wrote:

> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
> > I semi-successfully messed around in remote.c (format_tracking_info(),
> > stat_tracking_info()) to make it use branch->merge_name rather than
> > branch->merge. This makes "git status" work as expected ("Your branch
> > is... severely screwed.") for tracked local branches. (It's messed up
> > for remote ones but hey it was a first shot; merge[0]->dst is really
> > needed here I guess.)
> >
> > Now I could go after sha1_name.c and do the same,
> >
> > OR
> >
> > make it so that all branches have their merge member set up, uhm. Any
> > possible side effects?
> 
> My gut feeling is that the latter if works should be preferable for
> consistency if nothing else.
> 
> The "struct branch" hasn't changed ever since it was introduced by cf81834
> (Report information on branches from remote.h, 2007-09-10) and Daniel
> might know about some corner cases that rely on branch.merge not being set
> up for local ones, but honestly, I would think it would be a bug in the
> existing code if there were such cases.

As long as the semantics for tracking local branches are the same as for 
tracking remote ones, I'm pretty sure there's nothing that relies on 
branch.merge not being something local.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 0/2] Make local branches behave like remote branches when --tracked
  2009-03-20 18:10   ` Daniel Barkalow
@ 2009-03-26 20:53     ` Michael J Gruber
  2009-03-26 20:53       ` [PATCH 1/2] Test for local branches being followed with --track Michael J Gruber
  2009-03-26 20:57       ` [PATCH 0/2] " Michael J Gruber
  0 siblings, 2 replies; 15+ messages in thread
From: Michael J Gruber @ 2009-03-26 20:53 UTC (permalink / raw)
  To: git; +Cc: Daniel Barkalow, Johannes Schindelin, Junio C Hamano

[Sorry it took so long to finish... This is from my "sick bed", I hope
it doesn't show ;)]

This mini series makes local branches behave the same as remote ones
when they are used as --tracked branches. This means differences are
reported by git status and git checkout, and also that the soon to be
released tracking branch short cut (aka BEL) will work.

Michael J Gruber (2):
  Test for local branches being followed with --track
  Make local branches behave like remote branches when --tracked

 remote.c                 |    9 +++++----
 t/t6040-tracking-info.sh |   10 +++++++++-
 2 files changed, 14 insertions(+), 5 deletions(-)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/2] Test for local branches being followed with --track
  2009-03-26 20:53     ` [PATCH 0/2] Make local branches behave like remote branches when --tracked Michael J Gruber
@ 2009-03-26 20:53       ` Michael J Gruber
  2009-03-26 20:53         ` [PATCH 2/2] Make local branches behave like remote branches when --tracked Michael J Gruber
  2009-03-26 20:57       ` [PATCH 0/2] " Michael J Gruber
  1 sibling, 1 reply; 15+ messages in thread
From: Michael J Gruber @ 2009-03-26 20:53 UTC (permalink / raw)
  To: git; +Cc: Daniel Barkalow, Johannes Schindelin, Junio C Hamano

According to the documentation, it is perfectly okay to follow local
branches using the --track option. Introduce a test which checks whether
they behave the same. Currently 1 test fails.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/t6040-tracking-info.sh |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index ba90601..2a2b6b6 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -29,7 +29,9 @@ test_expect_success setup '
 		git checkout -b b4 origin &&
 		advance e &&
 		advance f
-	)
+	) &&
+	git checkout -b follower --track master &&
+	advance g
 '
 
 script='s/^..\(b.\)[	 0-9a-f]*\[\([^]]*\)\].*/\1 \2/p'
@@ -56,6 +58,12 @@ test_expect_success 'checkout' '
 	grep "have 1 and 1 different" actual
 '
 
+test_expect_failure 'checkout with local tracked branch' '
+	git checkout master &&
+	git checkout follower >actual
+	grep "is ahead of" actual
+'
+
 test_expect_success 'status' '
 	(
 		cd test &&
-- 
1.6.2.1.507.g0e68d

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/2] Make local branches behave like remote branches when --tracked
  2009-03-26 20:53       ` [PATCH 1/2] Test for local branches being followed with --track Michael J Gruber
@ 2009-03-26 20:53         ` Michael J Gruber
  2009-03-27  8:08           ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Michael J Gruber @ 2009-03-26 20:53 UTC (permalink / raw)
  To: git; +Cc: Daniel Barkalow, Johannes Schindelin, Junio C Hamano

This makes sure that local branches, when followed using --track, behave
the same as remote ones (e.g. differences being reported by git status
and git checkout). This fixes 1 known failure.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 remote.c                 |    9 +++++----
 t/t6040-tracking-info.sh |    2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/remote.c b/remote.c
index 2b037f1..5d2d7a1 100644
--- a/remote.c
+++ b/remote.c
@@ -1170,8 +1170,9 @@ struct branch *branch_get(const char *name)
 			for (i = 0; i < ret->merge_nr; i++) {
 				ret->merge[i] = xcalloc(1, sizeof(**ret->merge));
 				ret->merge[i]->src = xstrdup(ret->merge_name[i]);
-				remote_find_tracking(ret->remote,
-						     ret->merge[i]);
+				if(remote_find_tracking(ret->remote,
+						     ret->merge[i]) && !strcmp(ret->remote_name, "."))
+					ret->merge[i]->dst = xstrdup(ret->merge_name[i]);
 			}
 		}
 	}
@@ -1449,8 +1450,8 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 		return 0;
 
 	base = branch->merge[0]->dst;
-	if (!prefixcmp(base, "refs/remotes/")) {
-		base += strlen("refs/remotes/");
+	if (!prefixcmp(base, "refs/")) {
+		base += strlen("refs/");
 	}
 	if (!num_theirs)
 		strbuf_addf(sb, "Your branch is ahead of '%s' "
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 2a2b6b6..3d6db4d 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -58,7 +58,7 @@ test_expect_success 'checkout' '
 	grep "have 1 and 1 different" actual
 '
 
-test_expect_failure 'checkout with local tracked branch' '
+test_expect_success 'checkout with local tracked branch' '
 	git checkout master &&
 	git checkout follower >actual
 	grep "is ahead of" actual
-- 
1.6.2.1.507.g0e68d

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/2] Make local branches behave like remote branches when --tracked
  2009-03-26 20:53     ` [PATCH 0/2] Make local branches behave like remote branches when --tracked Michael J Gruber
  2009-03-26 20:53       ` [PATCH 1/2] Test for local branches being followed with --track Michael J Gruber
@ 2009-03-26 20:57       ` Michael J Gruber
  1 sibling, 0 replies; 15+ messages in thread
From: Michael J Gruber @ 2009-03-26 20:57 UTC (permalink / raw)
  Cc: git, Daniel Barkalow, Johannes Schindelin, Junio C Hamano

Michael J Gruber venit, vidit, dixit 26.03.2009 21:53:
> [Sorry it took so long to finish... This is from my "sick bed", I hope
> it doesn't show ;)]
> 
> This mini series makes local branches behave the same as remote ones
> when they are used as --tracked branches. This means differences are
> reported by git status and git checkout, and also that the soon to be
> released tracking branch short cut (aka BEL) will work.
> 
> Michael J Gruber (2):
>   Test for local branches being followed with --track
>   Make local branches behave like remote branches when --tracked
> 
>  remote.c                 |    9 +++++----
>  t/t6040-tracking-info.sh |   10 +++++++++-
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 

Sorry, I meant to point out also that 2/2 changes the display format of
the branch: refs/ is removed rather than refs/remotes/, if present. This
makes for unique branch names ready to copy&paste (they were not
necessarily unique before).

Michael

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] Make local branches behave like remote branches when --tracked
  2009-03-26 20:53         ` [PATCH 2/2] Make local branches behave like remote branches when --tracked Michael J Gruber
@ 2009-03-27  8:08           ` Junio C Hamano
  2009-03-27  8:47             ` Michael J Gruber
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2009-03-27  8:08 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Daniel Barkalow, Johannes Schindelin

Michael J Gruber <git@drmicha.warpmail.net> writes:

> This makes sure that local branches, when followed using --track, behave
> the same as remote ones (e.g. differences being reported by git status
> and git checkout). This fixes 1 known failure.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
>  remote.c                 |    9 +++++----
>  t/t6040-tracking-info.sh |    2 +-
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 2b037f1..5d2d7a1 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1170,8 +1170,9 @@ struct branch *branch_get(const char *name)
>  			for (i = 0; i < ret->merge_nr; i++) {
>  				ret->merge[i] = xcalloc(1, sizeof(**ret->merge));
>  				ret->merge[i]->src = xstrdup(ret->merge_name[i]);
> -				remote_find_tracking(ret->remote,
> -						     ret->merge[i]);
> +				if(remote_find_tracking(ret->remote,
> +						     ret->merge[i]) && !strcmp(ret->remote_name, "."))
> +					ret->merge[i]->dst = xstrdup(ret->merge_name[i]);
>  			}
>  		}
>  	}

Yuck; please have a SP betweeen "if" and "(", and also have a decency to
break a long line at a more sensible place, like:

			if (remote_find_tracking(ret->remote, ret->merge[i])
			    && !strcmp(...))
                            	then do this;

A naïve question from me to this change is why this "fix-up" is done here.

The remote_find_tracking() function is given a half-filled refspec (this
caller fills the src side, and asks to find the dst side to the function).
After it fails to find a fetch refspec that copies remote refs to tracking
refs in the local repository that match the criteria, it returns -1 to
signal an error, otherwise it returns 0 after updating the other half of
the refspec.

After calling r-f-t, because this new code assumes that for the "." remote
(aka "local repository"), r-f-t lies and does not give back what it
expects, fixes what it got back from r-f-t.  Shouldn't we be fixing this
inside r-f-t?

> @@ -1449,8 +1450,8 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
>  		return 0;
>  
>  	base = branch->merge[0]->dst;
> -	if (!prefixcmp(base, "refs/remotes/")) {
> -		base += strlen("refs/remotes/");
> +	if (!prefixcmp(base, "refs/")) {
> +		base += strlen("refs/");

I am not sure if this is a good change.  The majority of the case would
be remotes/ and we would be better off not repeating them.  Can't you
limit the use of longer refs only when disambiguation is necessary?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] Make local branches behave like remote branches when --tracked
  2009-03-27  8:08           ` Junio C Hamano
@ 2009-03-27  8:47             ` Michael J Gruber
  2009-03-27 16:20               ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Michael J Gruber @ 2009-03-27  8:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Daniel Barkalow, Johannes Schindelin

Junio C Hamano venit, vidit, dixit 27.03.2009 09:08:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> This makes sure that local branches, when followed using --track, behave
>> the same as remote ones (e.g. differences being reported by git status
>> and git checkout). This fixes 1 known failure.
>>
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>> ---
>>  remote.c                 |    9 +++++----
>>  t/t6040-tracking-info.sh |    2 +-
>>  2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/remote.c b/remote.c
>> index 2b037f1..5d2d7a1 100644
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -1170,8 +1170,9 @@ struct branch *branch_get(const char *name)
>>  			for (i = 0; i < ret->merge_nr; i++) {
>>  				ret->merge[i] = xcalloc(1, sizeof(**ret->merge));
>>  				ret->merge[i]->src = xstrdup(ret->merge_name[i]);
>> -				remote_find_tracking(ret->remote,
>> -						     ret->merge[i]);
>> +				if(remote_find_tracking(ret->remote,
>> +						     ret->merge[i]) && !strcmp(ret->remote_name, "."))
>> +					ret->merge[i]->dst = xstrdup(ret->merge_name[i]);
>>  			}
>>  		}
>>  	}
> 
> Yuck; please have a SP betweeen "if" and "(", and also have a decency to
> break a long line at a more sensible place, like:
> 
> 			if (remote_find_tracking(ret->remote, ret->merge[i])
> 			    && !strcmp(...))
>                             	then do this;
> 

Sorry about the space. Regarding the break, you can see that the break
was like that before already, and I just followed suite, which I think
makes the diff more readable. But no problem changing that,

> A naïve question from me to this change is why this "fix-up" is done here.

It was the easiest and least intrusive way for me...

> 
> The remote_find_tracking() function is given a half-filled refspec (this
> caller fills the src side, and asks to find the dst side to the function).
> After it fails to find a fetch refspec that copies remote refs to tracking
> refs in the local repository that match the criteria, it returns -1 to
> signal an error, otherwise it returns 0 after updating the other half of
> the refspec.
> 
> After calling r-f-t, because this new code assumes that for the "." remote
> (aka "local repository"), r-f-t lies and does not give back what it
> expects, fixes what it got back from r-f-t.  Shouldn't we be fixing this
> inside r-f-t?

The technical reason is that there is no local remote, i.e. no remote
struct for '.', and I don't think we want it, because it would show up
in all places where the list of remotes is searched/displayed/...

With ret being the branch we talk about, r-f-t is passed ret->remote and
ret->merge[i] only. In the local case, r-f-t cannot use the remote
struct for '.' (there is none) to find what it needs, and it has no easy
access to ret->merge_names[i] which is that info.

branch_get(), on the other hand, has all needed info in place. So,
having r-f-t do it would require changing the parameters or adding a
remote struct for '.' and adjusting all callers correspondingly. Doing
it the way I did it is "minimally invasive" in that respect, with the
(small) downside that we may call r-f-t unnecessarily in the local case
- but we don't know before: If someone set up a remote config for '.'
then we have to go through r-f-t anayways.

> 
>> @@ -1449,8 +1450,8 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
>>  		return 0;
>>  
>>  	base = branch->merge[0]->dst;
>> -	if (!prefixcmp(base, "refs/remotes/")) {
>> -		base += strlen("refs/remotes/");
>> +	if (!prefixcmp(base, "refs/")) {
>> +		base += strlen("refs/");
> 
> I am not sure if this is a good change.  The majority of the case would
> be remotes/ and we would be better off not repeating them.  Can't you
> limit the use of longer refs only when disambiguation is necessary?
> 

The majority will be remotes, yes, but will the majority be unique? In
my case not.  Even when we knew that format_tracking_info() would have
to deal with remote branches only (before this series) there was a
(high) chance of outputting non-unique refs, even worse: if foo is
ambiguous because refs/heads/foo and refs/remotes/foo exist then
refs/heads/foo would win, i.e. we used to output the *wrong* ref. The
above disambiguates. But I'll see if I can simplify the output based on
the necessity of disambiguation.

Michael

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] Make local branches behave like remote branches when --tracked
  2009-03-27  8:47             ` Michael J Gruber
@ 2009-03-27 16:20               ` Junio C Hamano
  2009-03-27 16:52                 ` Michael J Gruber
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2009-03-27 16:20 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Daniel Barkalow, Johannes Schindelin

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Junio C Hamano venit, vidit, dixit 27.03.2009 09:08:
> ...
>> After calling r-f-t, because this new code assumes that for the "." remote
>> (aka "local repository"), r-f-t lies and does not give back what it
>> expects, fixes what it got back from r-f-t.  Shouldn't we be fixing this
>> inside r-f-t?
>
> The technical reason is that there is no local remote, i.e. no remote
> struct for '.', and I don't think we want it, because it would show up
> in all places where the list of remotes is searched/displayed/...
>
> With ret being the branch we talk about, r-f-t is passed ret->remote and
> ret->merge[i] only. In the local case, r-f-t cannot use the remote
> struct for '.' (there is none) to find what it needs, and it has no easy
> access to ret->merge_names[i] which is that info.
>
> branch_get(), on the other hand, has all needed info in place.

Thanks for a detailed explanation.  Would it deserve to be in the commit
log justification in a summarized form?

> ..., even worse: if foo is
> ambiguous because refs/heads/foo and refs/remotes/foo exist then
> refs/heads/foo would win, i.e. we used to output the *wrong* ref. The
> above disambiguates. But I'll see if I can simplify the output based on
> the necessity of disambiguation.

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] Make local branches behave like remote branches when --tracked
  2009-03-27 16:20               ` Junio C Hamano
@ 2009-03-27 16:52                 ` Michael J Gruber
  2009-04-01 21:42                   ` [PATCHv2 0/2] " Michael J Gruber
  0 siblings, 1 reply; 15+ messages in thread
From: Michael J Gruber @ 2009-03-27 16:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Daniel Barkalow, Johannes Schindelin

Junio C Hamano venit, vidit, dixit 27.03.2009 17:20:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> Junio C Hamano venit, vidit, dixit 27.03.2009 09:08:
>> ...
>>> After calling r-f-t, because this new code assumes that for the "." remote
>>> (aka "local repository"), r-f-t lies and does not give back what it
>>> expects, fixes what it got back from r-f-t.  Shouldn't we be fixing this
>>> inside r-f-t?
>>
>> The technical reason is that there is no local remote, i.e. no remote
>> struct for '.', and I don't think we want it, because it would show up
>> in all places where the list of remotes is searched/displayed/...
>>
>> With ret being the branch we talk about, r-f-t is passed ret->remote and
>> ret->merge[i] only. In the local case, r-f-t cannot use the remote
>> struct for '.' (there is none) to find what it needs, and it has no easy
>> access to ret->merge_names[i] which is that info.
>>
>> branch_get(), on the other hand, has all needed info in place.
> 
> Thanks for a detailed explanation.  Would it deserve to be in the commit
> log justification in a summarized form?

You tell me :)
I'm still unsure at times how detailed commit messages should be, but I
take it I should put a shortened version of the above in there.

> 
>> ..., even worse: if foo is
>> ambiguous because refs/heads/foo and refs/remotes/foo exist then
>> refs/heads/foo would win, i.e. we used to output the *wrong* ref. The
>> above disambiguates. But I'll see if I can simplify the output based on
>> the necessity of disambiguation.
> 
> Thanks.

I thought about doing the following:
- remove occurences of refs/remotes (like before the patch) and of
refs/heads. That way the output format stays the same, the amiguity
problem is not worsened
- think later/harder about doing this more cleverly. I guess I need to
check the output of dwim_ref, but have to figure out all the input
parameters first...

Michael

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv2 0/2] Make local branches behave like remote branches when --tracked
  2009-03-27 16:52                 ` Michael J Gruber
@ 2009-04-01 21:42                   ` Michael J Gruber
  2009-04-01 21:42                     ` [PATCHv2 1/2] Test for local branches being followed with --track Michael J Gruber
  0 siblings, 1 reply; 15+ messages in thread
From: Michael J Gruber @ 2009-04-01 21:42 UTC (permalink / raw)
  To: git; +Cc: Daniel Barkalow, Johannes Schindelin, Junio C Hamano


This mini series makes local branches behave the same as remote ones
when they are used as --tracked branches. This means differences are
reported by git status and git checkout, and also that the soon to be
released tracking branch short cut (aka BEL) will work.

v2 adds a more detailed commit message to 2/2 and fixes up formatting.
Also, the simplification of remote refs is now unchanged, and local refs
are simplified in the same way. This may lead to ambiguous refs just
like before this series. Unique simplification (which several places may
benefit from) is left for a future series.

Michael J Gruber (2):
  Test for local branches being followed with --track
  Make local branches behave like remote branches when --tracked

 remote.c                 |    7 +++++--
 t/t6040-tracking-info.sh |   10 +++++++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv2 1/2] Test for local branches being followed with --track
  2009-04-01 21:42                   ` [PATCHv2 0/2] " Michael J Gruber
@ 2009-04-01 21:42                     ` Michael J Gruber
  2009-04-01 21:42                       ` [PATCHv2 2/2] Make local branches behave like remote branches when --tracked Michael J Gruber
  0 siblings, 1 reply; 15+ messages in thread
From: Michael J Gruber @ 2009-04-01 21:42 UTC (permalink / raw)
  To: git; +Cc: Daniel Barkalow, Johannes Schindelin, Junio C Hamano

According to the documentation, it is perfectly okay to follow local
branches using the --track option. Introduce a test which checks whether
they behave the same. Currently 1 test fails.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/t6040-tracking-info.sh |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index ba90601..2a2b6b6 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -29,7 +29,9 @@ test_expect_success setup '
 		git checkout -b b4 origin &&
 		advance e &&
 		advance f
-	)
+	) &&
+	git checkout -b follower --track master &&
+	advance g
 '
 
 script='s/^..\(b.\)[	 0-9a-f]*\[\([^]]*\)\].*/\1 \2/p'
@@ -56,6 +58,12 @@ test_expect_success 'checkout' '
 	grep "have 1 and 1 different" actual
 '
 
+test_expect_failure 'checkout with local tracked branch' '
+	git checkout master &&
+	git checkout follower >actual
+	grep "is ahead of" actual
+'
+
 test_expect_success 'status' '
 	(
 		cd test &&
-- 
1.6.2.1.507.g0e68d

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv2 2/2] Make local branches behave like remote branches when --tracked
  2009-04-01 21:42                     ` [PATCHv2 1/2] Test for local branches being followed with --track Michael J Gruber
@ 2009-04-01 21:42                       ` Michael J Gruber
  0 siblings, 0 replies; 15+ messages in thread
From: Michael J Gruber @ 2009-04-01 21:42 UTC (permalink / raw)
  To: git; +Cc: Daniel Barkalow, Johannes Schindelin, Junio C Hamano

This makes sure that local branches, when followed using --track, behave
the same as remote ones (e.g. differences being reported by git status
and git checkout). This fixes 1 known failure.

The fix is done within branch_get(): The first natural candidate,
namely remote_find_tracking(), does not have all the necessary info
because in general there is no remote struct for '.', and we don't want
one because it would show up in other places as well.

branch_get(), on the other hand, has access to merge_names[] (in
addition to merge[]) and therefore can set up the followed branch
easily.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 remote.c                 |    7 +++++--
 t/t6040-tracking-info.sh |    2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/remote.c b/remote.c
index 2b037f1..d12140e 100644
--- a/remote.c
+++ b/remote.c
@@ -1170,8 +1170,9 @@ struct branch *branch_get(const char *name)
 			for (i = 0; i < ret->merge_nr; i++) {
 				ret->merge[i] = xcalloc(1, sizeof(**ret->merge));
 				ret->merge[i]->src = xstrdup(ret->merge_name[i]);
-				remote_find_tracking(ret->remote,
-						     ret->merge[i]);
+				if (remote_find_tracking(ret->remote, ret->merge[i])
+				    && !strcmp(ret->remote_name, "."))
+					ret->merge[i]->dst = xstrdup(ret->merge_name[i]);
 			}
 		}
 	}
@@ -1451,6 +1452,8 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 	base = branch->merge[0]->dst;
 	if (!prefixcmp(base, "refs/remotes/")) {
 		base += strlen("refs/remotes/");
+	} else if (!prefixcmp(base, "refs/heads/")) {
+		base += strlen("refs/heads/");
 	}
 	if (!num_theirs)
 		strbuf_addf(sb, "Your branch is ahead of '%s' "
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 2a2b6b6..3d6db4d 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -58,7 +58,7 @@ test_expect_success 'checkout' '
 	grep "have 1 and 1 different" actual
 '
 
-test_expect_failure 'checkout with local tracked branch' '
+test_expect_success 'checkout with local tracked branch' '
 	git checkout master &&
 	git checkout follower >actual
 	grep "is ahead of" actual
-- 
1.6.2.1.507.g0e68d

^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2009-04-01 21:44 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-20 14:22 Tracking of local branches Michael J Gruber
2009-03-20 16:13 ` Michael J Gruber
2009-03-20 16:46 ` Junio C Hamano
2009-03-20 18:10   ` Daniel Barkalow
2009-03-26 20:53     ` [PATCH 0/2] Make local branches behave like remote branches when --tracked Michael J Gruber
2009-03-26 20:53       ` [PATCH 1/2] Test for local branches being followed with --track Michael J Gruber
2009-03-26 20:53         ` [PATCH 2/2] Make local branches behave like remote branches when --tracked Michael J Gruber
2009-03-27  8:08           ` Junio C Hamano
2009-03-27  8:47             ` Michael J Gruber
2009-03-27 16:20               ` Junio C Hamano
2009-03-27 16:52                 ` Michael J Gruber
2009-04-01 21:42                   ` [PATCHv2 0/2] " Michael J Gruber
2009-04-01 21:42                     ` [PATCHv2 1/2] Test for local branches being followed with --track Michael J Gruber
2009-04-01 21:42                       ` [PATCHv2 2/2] Make local branches behave like remote branches when --tracked Michael J Gruber
2009-03-26 20:57       ` [PATCH 0/2] " Michael J Gruber

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.