[PATCH] mailinfo: accept >From in message header

[PATCH] mailinfo: accept >From in message header

[Michael S. Tsirkin]

Hi!
Mail I get sometimes has multiple From lines, like this:

>From Majordomo@vger.kernel.org  Thu Jul 27 16:39:36 2006
>From mtsirkin  Thu Jul 27 16:39:36 2006
Received: from yok.mtl.com [10.0.8.11]
....

which confuses git-mailinfo since that does not recognize >From
as a valid header line. The following patch makes git-applymbox
work for me:

---

Recognize >From XXX as a valid header line.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>

diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index ac53f76..2b6e9fa 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -446,7 +446,7 @@ static int read_one_header_line(char *li
 			break;
 	}
 	/* Count mbox From headers as headers */
-	if (!ofs && !memcmp(line, "From ", 5))
+	if (!ofs && (!memcmp(line, "From ", 5) || !memcmp(line, ">From ", 6)))
 		ofs = 1;
 	return ofs;
 }

-- 
MST

Re: [PATCH] mailinfo: accept >From in message header

[Junio C Hamano]

"Michael S. Tsirkin" <mst@mellanox.co.il> writes:

> From Majordomo@vger.kernel.org  Thu Jul 27 16:39:36 2006
>>From mtsirkin  Thu Jul 27 16:39:36 2006
> Received: from yok.mtl.com [10.0.8.11]
> ....
>
> which confuses git-mailinfo since that does not recognize >From
> as a valid header line. The following patch makes git-applymbox
> work for me:

Wouldn't that kind of breakage confuse git-mailsplit as well?

Re: [PATCH] mailinfo: accept >From in message header

[Michael S. Tsirkin]

Quoting r. Junio C Hamano <junkio@cox.net>:
> Subject: Re: [PATCH] mailinfo: accept >From in message header
> 
> "Michael S. Tsirkin" <mst@mellanox.co.il> writes:
> 
> > From Majordomo@vger.kernel.org  Thu Jul 27 16:39:36 2006
> >>From mtsirkin  Thu Jul 27 16:39:36 2006
> > Received: from yok.mtl.com [10.0.8.11]
> > ....
> >
> > which confuses git-mailinfo since that does not recognize >From
> > as a valid header line. The following patch makes git-applymbox
> > work for me:
> 
> Wouldn't that kind of breakage confuse git-mailsplit as well?
> 

Hmm - I normally don't use it - just pipe stiff from mutt into git-applymbox.
A quick test seems to indicate git-mailsplit works fine.

But why should it - mailsplit just needs ^From to match new mail,
correct? So the escaped >From is good for it.

-- 
MST

Multiple pulls from the same branch in .git/remotes/origin

[Michael S. Tsirkin]

Hi, all!
I have
> git --version
git version 1.4.2.rc3.g0d95
and I have put the following in .git/remotes/origin

>cat .git/remotes/origin
URL: ssh://mst@frodo/scm/git/linux-2.6
Pull: refs/heads/linus_master:refs/heads/origin
Pull: refs/heads/linus_master:refs/heads/linus_master_branch

now I get:

>git fetch origin
error: no such remote ref refs/heads/linus_master
Fetch failure: ssh://mst@frodo/scm/git/linux-2.6

However, if I remove the second line, like this:

>cat .git/remotes/origin
URL: ssh://mst@frodo/scm/git/linux-2.6
Pull: refs/heads/linus_master:refs/heads/origin

I get remote linus_master properly fetched into local origin.
Two questions:
1. Isn't is possible to pull from the same remote branch into multiple local
   branches?
2. Even if not, isn't the error message above a bit strange?
   After all, the remote branch *does* exist.

Thanks,

-- 
MST

Re: Multiple pulls from the same branch in .git/remotes/origin

[Junio C Hamano]

"Michael S. Tsirkin" <mst@mellanox.co.il> writes:

> I get remote linus_master properly fetched into local origin.
> Two questions:

> 1. Isn't is possible to pull from the same remote branch into multiple local
>    branches?

Currently, no.  The underlying git-fetch-pack makes sure you do
not give the same branches twice on the command line.  I think
what the code actually tries to do is to make sure each of the
given refspecs is not ambiguous and matches something, but while
doing so it marks the remote ref that matched ineligible to
match again, which ends up showing the error message you saw.

It might not be too difficult to change it though.  This part of
the code is relatively old and I do not remember the details
offhand, other than I remember that the first time I saw it I
had the same confused "Huh?  we do not let a ref fetched twice?"
reaction ;-).

Re: Multiple pulls from the same branch in .git/remotes/origin

[Michael S. Tsirkin]

Quoting r. Junio C Hamano <junkio@cox.net>:
> Subject: Re: Multiple pulls from the same branch in .git/remotes/origin
> 
> "Michael S. Tsirkin" <mst@mellanox.co.il> writes:
> 
> > I get remote linus_master properly fetched into local origin.
> > Two questions:
> 
> > 1. Isn't is possible to pull from the same remote branch into multiple local
> >    branches?
> 
> Currently, no.  The underlying git-fetch-pack makes sure you do
> not give the same branches twice on the command line.  I think
> what the code actually tries to do is to make sure each of the
> given refspecs is not ambiguous and matches something, but while
> doing so it marks the remote ref that matched ineligible to
> match again, which ends up showing the error message you saw.
> 
> It might not be too difficult to change it though.  This part of
> the code is relatively old and I do not remember the details
> offhand, other than I remember that the first time I saw it I
> had the same confused "Huh?  we do not let a ref fetched twice?"
> reaction ;-).

At least, fix the error message?

-- 
MST

Re: Multiple pulls from the same branch in .git/remotes/origin

[Junio C Hamano]

"Michael S. Tsirkin" <mst@mellanox.co.il> writes:

>> It might not be too difficult to change it though.  This part of
>> the code is relatively old and I do not remember the details
>> offhand, other than I remember that the first time I saw it I
>> had the same confused "Huh?  we do not let a ref fetched twice?"
>> reaction ;-).
>
> At least, fix the error message?

That would touch the same vicinity of code so if I were to do
that myself I would rather see if the restriction can be
loosened properly first.

Re: Multiple pulls from the same branch in .git/remotes/origin

[Michael S. Tsirkin]

Quoting r. Junio C Hamano <junkio@cox.net>:
> Subject: Re: Multiple pulls from the same branch in .git/remotes/origin
> 
> "Michael S. Tsirkin" <mst@mellanox.co.il> writes:
> 
> >> It might not be too difficult to change it though.  This part of
> >> the code is relatively old and I do not remember the details
> >> offhand, other than I remember that the first time I saw it I
> >> had the same confused "Huh?  we do not let a ref fetched twice?"
> >> reaction ;-).
> >
> > At least, fix the error message?
> 
> That would touch the same vicinity of code so if I were to do
> that myself I would rather see if the restriction can be
> loosened properly first.

It seems that have to make path_match in connect.c return an array
of indexes for that to work. Right?

-- 
MST

Re: Multiple pulls from the same branch in .git/remotes/origin

[Junio C Hamano]

"Michael S. Tsirkin" <mst@mellanox.co.il> writes:

> It seems that have to make path_match in connect.c return an array
> of indexes for that to work. Right?

I do not recall why Linus placed that "*s = 0" in there.  That
is the one that makes the same ref not match more than once.

It _could_ be that the fix is just the matter of removing that
line, but it may have interesting consequences; I dunno offhand.

Re: Multiple pulls from the same branch in .git/remotes/origin

[Michael S. Tsirkin]

Quoting r. Junio C Hamano <junkio@cox.net>:
> Subject: Re: Multiple pulls from the same branch in .git/remotes/origin
> 
> "Michael S. Tsirkin" <mst@mellanox.co.il> writes:
> 
> > It seems that have to make path_match in connect.c return an array
> > of indexes for that to work. Right?
> 
> I do not recall why Linus placed that "*s = 0" in there.  That
> is the one that makes the same ref not match more than once.

Hmm, as I read it, this marks the *match* as already found.
This seems to be what makes the sanity check in main work:

                /* If the heads to pull were given, we should have
                 * consumed all of them by matching the remote.
                 * Otherwise, 'git-fetch remote no-such-ref' would
                 * silently succeed without issuing an error.
                 */

What stops it is return i + 1 in path_match, no?
I think the issue is that path_match will stop at first match and not
label multiple matches, so later sanity check in main errors out.

> It _could_ be that the fix is just the matter of removing that
> line, but it may have interesting consequences; I dunno offhand.
> 
> 

What I did is just scan the full list of refspecs instead of breaking
out at the first match.

-- 
MST

[PATCH] Multiple refs from the same remote in one git fetch

[Michael S. Tsirkin]

Quoting r. Junio C Hamano <junkio@cox.net>:
> > At least, fix the error message?
> 
> That would touch the same vicinity of code so if I were to do
> that myself I would rather see if the restriction can be
> loosened properly first.

The following seems to fix this issue: simply scan the full
refspec list for each remote instead of breaking on the first match.
I had to touch the git-fetch.sh script as well as it is scanning
the remote list, too. Works for me.

---

Fix git fetch for when multiple refspecs reference the same remote head.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>

diff --git a/Makefile b/Makefile
diff --git a/connect.c b/connect.c
index 4422a0d..3880191 100644
--- a/connect.c
+++ b/connect.c
@@ -115,6 +115,7 @@ int get_ack(int fd, unsigned char *resul
 int path_match(const char *path, int nr, char **match)
 {
 	int i;
+	int found = 0;
 	int pathlen = strlen(path);
 
 	for (i = 0; i < nr; i++) {
@@ -128,9 +129,9 @@ int path_match(const char *path, int nr,
 		if (pathlen > len && path[pathlen - len - 1] != '/')
 			continue;
 		*s = 0;
-		return (i + 1);
+		found = i + 1;
 	}
-	return 0;
+	return found;
 }
 
 struct refspec {
diff --git a/fetch-pack.c b/fetch-pack.c
diff --git a/git-fetch.sh b/git-fetch.sh
index c2eebee..c35bae6 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -373,26 +373,22 @@ fetch_main () {
 	      +$remote_name:*)
 		  single_force=t
 		  not_for_merge=
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      .+$remote_name:*)
 		  single_force=t
 		  not_for_merge=t
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      .$remote_name:*)
 		  not_for_merge=t
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      $remote_name:*)
 		  not_for_merge=
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      esac
-	  done
-	  local_name=$(expr "z$found" : 'z[^:]*:\(.*\)')
-	  append_fetch_head "$sha1" "$remote" \
+	      local_name=$(expr "z$found" : 'z[^:]*:\(.*\)')
+	      append_fetch_head "$sha1" "$remote" \
 		  "$remote_name" "$remote_nick" "$local_name" "$not_for_merge"
+	  done
       done
     ) || exit ;;
   esac

-- 
MST

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Junio C Hamano]

"Michael S. Tsirkin" <mst@mellanox.co.il> writes:

> The following seems to fix this issue: simply scan the full
> refspec list for each remote instead of breaking on the first match.
> I had to touch the git-fetch.sh script as well as it is scanning
> the remote list, too. Works for me.

Looks Ok, although I have to admit I just gave a cursory look.
Thanks.

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Michael S. Tsirkin]

Quoting r. Junio C Hamano <junkio@cox.net>:
> Subject: Re: [PATCH] Multiple refs from the same remote in one git fetch
> 
> "Michael S. Tsirkin" <mst@mellanox.co.il> writes:
> 
> > The following seems to fix this issue: simply scan the full
> > refspec list for each remote instead of breaking on the first match.
> > I had to touch the git-fetch.sh script as well as it is scanning
> > the remote list, too. Works for me.
> 
> Looks Ok, although I have to admit I just gave a cursory look.
> Thanks.
> 

Could this go into next then? 

-- 
MST

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Junio C Hamano]

"Michael S. Tsirkin" <mst@mellanox.co.il> writes:

> Quoting r. Junio C Hamano <junkio@cox.net>:
>> Subject: Re: [PATCH] Multiple refs from the same remote in one git fetch
>> 
>> "Michael S. Tsirkin" <mst@mellanox.co.il> writes:
>> 
>> > The following seems to fix this issue: simply scan the full
>> > refspec list for each remote instead of breaking on the first match.
>> > I had to touch the git-fetch.sh script as well as it is scanning
>> > the remote list, too. Works for me.
>> 
>> Looks Ok, although I have to admit I just gave a cursory look.
>> Thanks.
>> 
>
> Could this go into next then? 

No.  Spoke too fast.  Breaks t6200 test because it reports the
refs fetched in duplicates.

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Michael S. Tsirkin]

Quoting r. Junio C Hamano <junkio@cox.net>:
> >> Looks Ok, although I have to admit I just gave a cursory look.
> >> Thanks.
> >> 
> >
> > Could this go into next then? 
> 
> No.  Spoke too fast.  Breaks t6200 test because it reports the
> refs fetched in duplicates.
> 
> 

Right, The problem was with the way I coded the loop in git-fetch.sh.
Here's the fixed versin - seems to pass make test fine.

---

Fix git fetch for when multiple refspecs reference the same remote head.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>


diff --git a/connect.c b/connect.c
index 4422a0d..3880191 100644
--- a/connect.c
+++ b/connect.c
@@ -115,6 +115,7 @@ int get_ack(int fd, unsigned char *resul
 int path_match(const char *path, int nr, char **match)
 {
 	int i;
+	int found = 0;
 	int pathlen = strlen(path);
 
 	for (i = 0; i < nr; i++) {
@@ -128,9 +129,9 @@ int path_match(const char *path, int nr,
 		if (pathlen > len && path[pathlen - len - 1] != '/')
 			continue;
 		*s = 0;
-		return (i + 1);
+		found = i + 1;
 	}
-	return 0;
+	return found;
 }
 
 struct refspec {
diff --git a/git-fetch.sh b/git-fetch.sh
index c2eebee..80a7a5d 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -366,6 +366,7 @@ fetch_main () {
 		  exit 1 ;;
 	  esac
 	  found=
+	  found_any=
 	  single_force=
 	  for ref in $refs
 	  do
@@ -373,26 +374,34 @@ fetch_main () {
 	      +$remote_name:*)
 		  single_force=t
 		  not_for_merge=
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      .+$remote_name:*)
 		  single_force=t
 		  not_for_merge=t
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      .$remote_name:*)
 		  not_for_merge=t
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      $remote_name:*)
 		  not_for_merge=
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
+	      *)
+		  found=;;
 	      esac
+	      if test "$found" != ""
+	      then
+		  found_any=1
+		  local_name=$(expr "z$found" : 'z[^:]*:\(.*\)')
+		  append_fetch_head "$sha1" "$remote" "$remote_name" \
+		      "$remote_nick" "$local_name" "$not_for_merge"
+	      fi
 	  done
-	  local_name=$(expr "z$found" : 'z[^:]*:\(.*\)')
-	  append_fetch_head "$sha1" "$remote" \
-		  "$remote_name" "$remote_nick" "$local_name" "$not_for_merge"
+	  if test "$found_any" == ""
+	  then
+	      local_name=$(expr "z$found" : 'z[^:]*:\(.*\)')
+	      append_fetch_head "$sha1" "$remote" "$remote_name" \
+	          "$remote_nick" "$local_name" "$not_for_merge"
+	  fi
       done
     ) || exit ;;
   esac
diff --git a/t/Makefile b/t/Makefile

-- 
MST

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Junio C Hamano]

"Michael S. Tsirkin" <mst@mellanox.co.il> writes:

>> > Could this go into next then? 
>> 
>> No.  Spoke too fast.  Breaks t6200 test because it reports the
>> refs fetched in duplicates.
>
> Right, The problem was with the way I coded the loop in git-fetch.sh.
> Here's the fixed versin - seems to pass make test fine.

But what are you doing for single_force and not_for_merge when
there are more than one matches in the patch?  They seem to get
set to a random value depending on whatever happens to match
last, which does not feel quite right.

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Michael S. Tsirkin]

Quoting r. Junio C Hamano <junkio@cox.net>:
> Subject: Re: [PATCH] Multiple refs from the same remote in one git fetch
> 
> "Michael S. Tsirkin" <mst@mellanox.co.il> writes:
> 
> >> > Could this go into next then? 
> >> 
> >> No.  Spoke too fast.  Breaks t6200 test because it reports the
> >> refs fetched in duplicates.
> >
> > Right, The problem was with the way I coded the loop in git-fetch.sh.
> > Here's the fixed versin - seems to pass make test fine.
> 
> But what are you doing for single_force and not_for_merge when
> there are more than one matches in the patch?  They seem to get
> set to a random value depending on whatever happens to match
> last, which does not feel quite right.
> 

I think it was only true for single_force - but here's a patch to fix it and
also make this explicit.

BTW, does it still look like it's worth it the effort to lift the restriction,
or does fixing the error message to something like
"no such remote or duplicate ref %s"
make more sense to you?

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>

diff --git a/connect.c b/connect.c
index 4422a0d..3880191 100644
--- a/connect.c
+++ b/connect.c
@@ -115,6 +115,7 @@ int get_ack(int fd, unsigned char *resul
 int path_match(const char *path, int nr, char **match)
 {
 	int i;
+	int found = 0;
 	int pathlen = strlen(path);
 
 	for (i = 0; i < nr; i++) {
@@ -128,9 +129,9 @@ int path_match(const char *path, int nr,
 		if (pathlen > len && path[pathlen - len - 1] != '/')
 			continue;
 		*s = 0;
-		return (i + 1);
+		found = i + 1;
 	}
-	return 0;
+	return found;
 }
 
 struct refspec {
diff --git a/git-fetch.sh b/git-fetch.sh
index c2eebee..328168b 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -366,33 +366,44 @@ fetch_main () {
 		  exit 1 ;;
 	  esac
 	  found=
-	  single_force=
+	  found_any=
 	  for ref in $refs
 	  do
 	      case "$ref" in
 	      +$remote_name:*)
 		  single_force=t
 		  not_for_merge=
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      .+$remote_name:*)
 		  single_force=t
 		  not_for_merge=t
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      .$remote_name:*)
+		  single_force=
 		  not_for_merge=t
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      $remote_name:*)
+		  single_force=
+		  not_for_merge=
+		  found="$ref";;
+	      *)
+		  single_force=
 		  not_for_merge=
-		  found="$ref"
-		  break ;;
+		  found=;;
 	      esac
+	      if test "$found" != ""
+	      then
+		  found_any=1
+		  local_name=$(expr "z$found" : 'z[^:]*:\(.*\)')
+		  append_fetch_head "$sha1" "$remote" "$remote_name" \
+		      "$remote_nick" "$local_name" "$not_for_merge"
+	      fi
 	  done
-	  local_name=$(expr "z$found" : 'z[^:]*:\(.*\)')
-	  append_fetch_head "$sha1" "$remote" \
-		  "$remote_name" "$remote_nick" "$local_name" "$not_for_merge"
+	  if test "$found_any" == ""
+	  then
+	      append_fetch_head "$sha1" "$remote" "$remote_name" \
+	          "$remote_nick" "" ""
+	  fi
       done
     ) || exit ;;
   esac
diff --git a/t/Makefile b/t/Makefile

-- 
MST

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Junio C Hamano]

"Michael S. Tsirkin" <mst@mellanox.co.il> writes:

> BTW, does it still look like it's worth it the effort to lift the restriction,
> or does fixing the error message to something like
> "no such remote or duplicate ref %s"
> make more sense to you?

I have been hesitant to claim that it does not make any sense to
use more than one tracking branch for the same remote branch,
because the only reason I might say so is because I haven't
thought of a good usage pattern to do so.

But apparently you do use more than one local branch to keep
track of one remote branch.  How do you use it for?  Do you feel
it is a good feature to be able to do that, or do you think it
is just a mistake and more sensible error message is what we
would really want?

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Michael S. Tsirkin]

Quoting r. Junio C Hamano <junkio@cox.net>:
> Subject: Re: [PATCH] Multiple refs from the same remote in one git fetch
> 
> "Michael S. Tsirkin" <mst@mellanox.co.il> writes:
> 
> > BTW, does it still look like it's worth it the effort to lift the restriction,
> > or does fixing the error message to something like
> > "no such remote or duplicate ref %s"
> > make more sense to you?
> 
> I have been hesitant to claim that it does not make any sense to
> use more than one tracking branch for the same remote branch,
> because the only reason I might say so is because I haven't
> thought of a good usage pattern to do so.
> 
> But apparently you do use more than one local branch to keep
> track of one remote branch.  How do you use it for?  Do you feel
> it is a good feature to be able to do that, or do you think it
> is just a mistake and more sensible error message is what we
> would really want?
> 

Well, what I was *trying* to do is simply add a more descriptive name for
the linus master branch to my existing tree.
So it seemed like an obvious idea to add

Pull: master:origin
Pull: master:linus_master

On a more theoretical level, in a shared repository development style, one might
imagine several people who want the branch to be called differently.
Another reason might be scripts using specific branch names where you
might want to be free to decide where a specific branch name points to.

Yea, I still feel it would be a good feature to have - I just wanted to
check there's no opposition to this.

-- 
MST

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Junio C Hamano]

"Michael S. Tsirkin" <mst@mellanox.co.il> writes:

> On a more theoretical level, in a shared repository
> development style, one might imagine several people who want
> the branch to be called differently.

A shared repository in git context usually means the repository
"everybody pushes into and pulls _from_".  We are talking about
pulling into a repository -- and even if you are using a shared
repository you do not share a repository you pull into.

A shared repository style" does not mean anarchy; helping to
name the same thing with multiple branch names at the central
site is not something we would want to encourage anyway.

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Michael S. Tsirkin]

Quoting r. Junio C Hamano <junkio@cox.net>:
> A shared repository style" does not mean anarchy; helping to
> name the same thing with multiple branch names at the central
> site is not something we would want to encourage anyway.
> 

OK. Do other reasons sound sufficiently convincing to you?

-- 
MST

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Jakub Narebski]

Michael S. Tsirkin wrote:

> Quoting r. Junio C Hamano <junkio@cox.net>:
[...]
>> I have been hesitant to claim that it does not make any sense to
>> use more than one tracking branch for the same remote branch,
>> because the only reason I might say so is because I haven't
>> thought of a good usage pattern to do so.
>> 
>> But apparently you do use more than one local branch to keep
>> track of one remote branch.  How do you use it for?  Do you feel
>> it is a good feature to be able to do that, or do you think it
>> is just a mistake and more sensible error message is what we
>> would really want?
>> 
> 
> Well, what I was *trying* to do is simply add a more descriptive name for
> the linus master branch to my existing tree.
> So it seemed like an obvious idea to add
> 
> Pull: master:origin
> Pull: master:linus_master

Couldn't you do this via symlinks or symrefs? 

BTW. Do we support symrefs other than HEAD, and does reflog works with
symref heads, and symlinked heads?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Michael S. Tsirkin]

Quoting r. Jakub Narebski <jnareb@gmail.com>:
> Subject: Re: [PATCH] Multiple refs from the same remote in one git fetch
> 
> Michael S. Tsirkin wrote:
> 
> > Quoting r. Junio C Hamano <junkio@cox.net>:
> [...]
> >> I have been hesitant to claim that it does not make any sense to
> >> use more than one tracking branch for the same remote branch,
> >> because the only reason I might say so is because I haven't
> >> thought of a good usage pattern to do so.
> >> 
> >> But apparently you do use more than one local branch to keep
> >> track of one remote branch.  How do you use it for?  Do you feel
> >> it is a good feature to be able to do that, or do you think it
> >> is just a mistake and more sensible error message is what we
> >> would really want?
> >> 
> > 
> > Well, what I was *trying* to do is simply add a more descriptive name for
> > the linus master branch to my existing tree.
> > So it seemed like an obvious idea to add
> > 
> > Pull: master:origin
> > Pull: master:linus_master
> 
> Couldn't you do this via symlinks or symrefs? 
> 
> BTW. Do we support symrefs other than HEAD, and does reflog works with
> symref heads, and symlinked heads?

How?

-- 
MST

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Jakub Narebski]

Michael S. Tsirkin wrote:

>>> Well, what I was *trying* to do is simply add a more descriptive name
>>> for the linus master branch to my existing tree.
>>>
>>> So it seemed like an obvious idea to add
>>> 
>>> Pull: master:origin
>>> Pull: master:linus_master
>> 
>> Couldn't you do this via symlinks or symrefs? 
>> 
>> BTW. Do we support symrefs other than HEAD, and does reflog works with
>> symref heads, and symlinked heads?
> 
> How?

Have only "Pull: master:linus_master" as the pull line, and make origin
symlink (ln -s $GIT_DIR/refs/heads/linus_master $GIT_DIR/refs/heads/origin)
or symref (echo "ref: refs/heads/linus_master" > $GIT_DIR/refs/heads/origin)
or vice versa.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] Multiple refs from the same remote in one git fetch

[Michael S. Tsirkin]

Quoting r. Jakub Narebski <jnareb@gmail.com>:
> Have only "Pull: master:linus_master" as the pull line, and make origin
> symlink (ln -s $GIT_DIR/refs/heads/linus_master $GIT_DIR/refs/heads/origin)
> or symref (echo "ref: refs/heads/linus_master" > $GIT_DIR/refs/heads/origin)
> or vice versa.

Thanks.

-- 
MST

[PATCH repost] Multiple refs from the same remote in one git fetch

[Michael S. Tsirkin]

Hi, Junio!
Is the following likely to get merged eventually?
It was pointed out that I can work-around the limitation
by using softlinks, so it's not a show-stopper for me.

---

Fix git fetch for when multiple refspecs reference the same remote head.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>

diff --git a/connect.c b/connect.c
index 4422a0d..3880191 100644
--- a/connect.c
+++ b/connect.c
@@ -115,6 +115,7 @@ int get_ack(int fd, unsigned char *resul
 int path_match(const char *path, int nr, char **match)
 {
 	int i;
+	int found = 0;
 	int pathlen = strlen(path);
 
 	for (i = 0; i < nr; i++) {
@@ -128,9 +129,9 @@ int path_match(const char *path, int nr,
 		if (pathlen > len && path[pathlen - len - 1] != '/')
 			continue;
 		*s = 0;
-		return (i + 1);
+		found = i + 1;
 	}
-	return 0;
+	return found;
 }
 
 struct refspec {
diff --git a/git-fetch.sh b/git-fetch.sh
index c2eebee..328168b 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -366,33 +366,44 @@ fetch_main () {
 		  exit 1 ;;
 	  esac
 	  found=
-	  single_force=
+	  found_any=
 	  for ref in $refs
 	  do
 	      case "$ref" in
 	      +$remote_name:*)
 		  single_force=t
 		  not_for_merge=
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      .+$remote_name:*)
 		  single_force=t
 		  not_for_merge=t
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      .$remote_name:*)
+		  single_force=
 		  not_for_merge=t
-		  found="$ref"
-		  break ;;
+		  found="$ref";;
 	      $remote_name:*)
+		  single_force=
+		  not_for_merge=
+		  found="$ref";;
+	      *)
+		  single_force=
 		  not_for_merge=
-		  found="$ref"
-		  break ;;
+		  found=;;
 	      esac
+	      if test "$found" != ""
+	      then
+		  found_any=1
+		  local_name=$(expr "z$found" : 'z[^:]*:\(.*\)')
+		  append_fetch_head "$sha1" "$remote" "$remote_name" \
+		      "$remote_nick" "$local_name" "$not_for_merge"
+	      fi
 	  done
-	  local_name=$(expr "z$found" : 'z[^:]*:\(.*\)')
-	  append_fetch_head "$sha1" "$remote" \
-		  "$remote_name" "$remote_nick" "$local_name" "$not_for_merge"
+	  if test "$found_any" == ""
+	  then
+	      append_fetch_head "$sha1" "$remote" "$remote_name" \
+	          "$remote_nick" "" ""
+	  fi
       done
     ) || exit ;;
   esac

-- 
MST

Re: [PATCH repost] Multiple refs from the same remote in one git fetch

[Junio C Hamano]

Although this round does not seem to break tests like before, I
still do not see a valid use case for it.  So...