[PATCH 2/3] git-fetch: do not use "*" for fetching multiple refs

[PATCH 2/3] git-fetch: do not use "*" for fetching multiple refs

[Michael Loeffler]

The trailing / is enough to decide if this should map everything under
refs/heads/ to refs/somewhere/.

The "*" should be reserved for the use as regex operator.

Signed-off-by: Michael Loeffler <zvpunry@zvpunry.de>
---
I want to use regular expressions to match remote refs, so I try to
implement this. But the current globfetch syntax needs the '*'.

Maybe it is not to late to change the syntax to this:
Pull: refs/heads/:refs/remotes/origin/

What do you think?


diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index da064a5..38af4cb 100755
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -101,13 +101,13 @@ expand_refs_wildcard () {
 	do
 		lref=${ref#'+'}
 		# a non glob pattern is given back as-is.
-		expr "z$lref" : 'zrefs/.*/\*:refs/.*/\*$' >/dev/null || {
+		expr "z$lref" : 'zrefs/.*/:refs/.*/$' >/dev/null || {
 			echo "$ref"
 			continue
 		}
 
-		from=`expr "z$lref" : 'z\(refs/.*/\)\*:refs/.*/\*$'`
-		to=`expr "z$lref" : 'zrefs/.*/\*:\(refs/.*/\)\*$'`
+		from=`expr "z$lref" : 'z\(refs/.*/\):refs/.*/$'`
+		to=`expr "z$lref" : 'zrefs/.*/:\(refs/.*/\)$'`
 		local_force=
 		test "z$lref" = "z$ref" || local_force='+'
 		echo "$ls_remote_result" |
-- 
1.4.4

Re: [PATCH 2/3] git-fetch: do not use "*" for fetching multiple refs

[Jakub Narebski]

Michael Loeffler wrote:

> The trailing / is enough to decide if this should map everything under
> refs/heads/ to refs/somewhere/.
> 
> The "*" should be reserved for the use as regex operator.
> 
> Signed-off-by: Michael Loeffler <zvpunry@zvpunry.de>
> ---
> I want to use regular expressions to match remote refs, so I try to
> implement this. But the current globfetch syntax needs the '*'.
> 
> Maybe it is not to late to change the syntax to this:
> Pull: refs/heads/:refs/remotes/origin/
> 
> What do you think?

I'm not sure if regexp support is truly better than the usual path globbing,
as in fnmatch / glob.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH 2/3] git-fetch: do not use "*" for fetching multiple refs

[Michael Loeffler]

I used the wrong email-account for this patch, please reply to
zvpunry@zvpunry.de. Sorry.

bye

Re: [PATCH 2/3] git-fetch: do not use "*" for fetching multiple refs

[Michael Loeffler]

Am Montag, den 04.12.2006, 20:48 +0100 schrieb Jakub Narebski:
...
> I'm not sure if regexp support is truly better than the usual path globbing,
> as in fnmatch / glob.
The current code does not do a real glob, this was the reason for me to
think about regex support, I thought it is easy to use sed for this. Now
I know it better.

I want it a bit portable, but sed on other systems (like macos or
solaris) does not support extended REs, and the basic REs do not support
the | operator (but this works on systems with glibc with \|).

Maybe we should support something like this:
Pull: refs/heads/v*:refs/remotes/origin/

I still don't like the * on the destination ref, it looks a bit strange
(like cp Downloads/*.mp3 Music/*).

Re: [PATCH 2/3] git-fetch: do not use "*" for fetching multiple refs

[Jakub Narebski]

Michael Loeffler wrote:

> Am Montag, den 04.12.2006, 20:48 +0100 schrieb Jakub Narebski:
> ...
>> I'm not sure if regexp support is truly better than the usual path globbing,
>> as in fnmatch / glob.
>
> The current code does not do a real glob, this was the reason for me to
> think about regex support, I thought it is easy to use sed for this. Now
> I know it better.

We could use perl for that, but embedded perl is a bit horrible.

> I want it a bit portable, but sed on other systems (like macos or
> solaris) does not support extended REs, and the basic REs do not support
> the | operator (but this works on systems with glibc with \|).
> 
> Maybe we should support something like this:
> Pull: refs/heads/v*:refs/remotes/origin/
> 
> I still don't like the * on the destination ref, it looks a bit strange
> (like cp Downloads/*.mp3 Music/*).

'*' in destination part would mean $n / \n (n-th match for *).
And you need some way to mark if it is prefix match, or whole path match.
Ending prefix match with '/' is one way of doing this... Unless it would
be prefix match always, but I think this leads way to confusion.

Just a thought.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH 2/3] git-fetch: do not use "*" for fetching multiple refs

[Michael Loeffler]

Am Mittwoch, den 06.12.2006, 17:58 +0100 schrieb Jakub Narebski:
...
> We could use perl for that, but embedded perl is a bit horrible.
I had the same idea after the sed problems with macos/solaris, but
embedded perl is really a bit horrible.

...
> '*' in destination part would mean $n / \n (n-th match for *).
> And you need some way to mark if it is prefix match, or whole path match.
> Ending prefix match with '/' is one way of doing this... Unless it would
> be prefix match always, but I think this leads way to confusion.
Then we could just use (.*) and \1..9 and use extended REs. The only
problem is this stupid sed thing, only GNU-sed has the -r
option to use extended REs.

> Just a thought.
I would prefer the following ways to do this globfetch stuff:

1.) The original refspec:
    Pull: refs/heads/master:refs/remotes/origin/master

2.) The one with "prefix match":
    Pull: refs/heads/:refs/remotes/origin/

3.) The one with extended regex:
    Pull: refs/heads/(.*):refs/remotes/origin/\1

Re: [PATCH 2/3] git-fetch: do not use "*" for fetching multiple refs

[Jakub Narebski]

Michael Loeffler wrote:

>> We could use perl for that, but embedded perl is a bit horrible.
>
> I had the same idea after the sed problems with macos/solaris, but
> embedded perl is really a bit horrible.

Or you can rewrite git-fetch in Perl (or as built-in in C).

> I would prefer the following ways to do this globfetch stuff:
> 
> 1.) The original refspec:
>     Pull: refs/heads/master:refs/remotes/origin/master
> 
> 2.) The one with "prefix match":
>     Pull: refs/heads/:refs/remotes/origin/

I just worry what would happen when someone would write e.g.
      Pull: refs/heads/:refs/heads/origin-
 
> 3.) The one with extended regex:
>     Pull: refs/heads/(.*):refs/remotes/origin/\1

3.) The one with shell-like (fnmatch / glob) globbing
      Pull: refs/heads/*:refs/remotes/origin/*

By the way, with globbing we really need some other way than
first Pull: line to select remote head to merge on "git pull".
For example "Merge:" line / remote.<name>.merge config var.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH 2/3] git-fetch: do not use "*" for fetching multiple refs

[Junio C Hamano]

Michael Loeffler <zvpunry@zvpunry.de> writes:

>> Just a thought.
> I would prefer the following ways to do this globfetch stuff:
>
> 1.) The original refspec:
>     Pull: refs/heads/master:refs/remotes/origin/master
>
> 2.) The one with "prefix match":
>     Pull: refs/heads/:refs/remotes/origin/
>
> 3.) The one with extended regex:
>     Pull: refs/heads/(.*):refs/remotes/origin/\1

Please, don't do regex when talking about paths.  Uniformly
using fnmatch/glob is less confusing.  I do not see anything
wrong with Andy's refspec glob we already have.  Although I
agree that the second asterisk in "src/*:dst/*" has a certain
"Huh?" factor to UNIX-trained eyes, I think it is quite obvious
even to new people what it does.

Also, while I agree that (2) is logical and less typing, I would
avoid cases where foo and foo/ behave differently when "foo"
itself is a directory/tree like thing.  Doing otherwise easily
invites mistakes.

Re: [PATCH 2/3] git-fetch: do not use "*" for fetching multiple refs

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> 3.) The one with shell-like (fnmatch / glob) globbing
>       Pull: refs/heads/*:refs/remotes/origin/*
>
> By the way, with globbing we really need some other way than
> first Pull: line to select remote head to merge on "git pull".
> For example "Merge:" line / remote.<name>.merge config var.

Why?

        URL: some-where
        Pull: refs/heads/master:refs/remotes/origin/master
        Pull: refs/heads/*:refs/remotes/origin/*

works just fine.

But we should encourage people to use config to define default
merge source per-branch.

Re: [PATCH 2/3] git-fetch: do not use "*" for fetching multiple refs

[Johannes Schindelin]

Hi,

On Wed, 6 Dec 2006, Jakub Narebski wrote:

> Michael Loeffler wrote:
> 
> > Am Montag, den 04.12.2006, 20:48 +0100 schrieb Jakub Narebski:
> > ...
> >> I'm not sure if regexp support is truly better than the usual path globbing,
> >> as in fnmatch / glob.
> >
> > The current code does not do a real glob, this was the reason for me to
> > think about regex support, I thought it is easy to use sed for this. Now
> > I know it better.
> 
> We could use perl for that, but embedded perl is a bit horrible.

Not to talk about portable, and as we saw, dependent on the C compiler 
(you would have to make git compile with the same C compiler that perl was 
compiled with).

So, please look into other options first.

Ciao,

Re: [PATCH 2/3] git-fetch: do not use "*" for fetching multiple refs

[Jakub Narebski]

Johannes Schindelin wrote:

> On Wed, 6 Dec 2006, Jakub Narebski wrote:
> 
>> Michael Loeffler wrote:
>> 
>>> Am Montag, den 04.12.2006, 20:48 +0100 schrieb Jakub Narebski:
>>> ...
>>>> I'm not sure if regexp support is truly better than the usual path globbing,
>>>> as in fnmatch / glob.
>>>
>>> The current code does not do a real glob, this was the reason for me to
>>> think about regex support, I thought it is easy to use sed for this. Now
>>> I know it better.
>> 
>> We could use perl for that, but embedded perl is a bit horrible.
> 
> Not to talk about portable, and as we saw, dependent on the C compiler 
> (you would have to make git compile with the same C compiler that perl was 
> compiled with).
> 
> So, please look into other options first.

No, not embedded in C, but embedded in shell script.
Use perl -ip instead of sed.

-- 
Jakub Narebski