threads / patch / 43222

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

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

## tl;dr

11 messages between Dec 4, 2006 and Dec 6, 2006. Diffs are folded; open one to read it.

replies: 10people: 5as markdown or json

Michael Loeffler· Dec 4, 2006, 19:38 UTC · lore

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

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?
Show changes to git-parse-remote.sh +3 −3
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
Jakub Narebski· Dec 4, 2006, 19:48 UTC · re: Michael Loeffler · lore
Michael Loeffler wrote:
Show 14 quoted lines
> 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
Michael Loeffler· Dec 6, 2006, 16:34 UTC · re: Jakub Narebski · lore

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/*).

Jakub Narebski· Dec 6, 2006, 16:58 UTC · re: Michael Loeffler · lore
Michael Loeffler wrote:
Show 8 quoted lines
> 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.
Show 9 quoted lines
> 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
Michael Loeffler· Dec 6, 2006, 18:16 UTC · re: Jakub Narebski · lore

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
Jakub Narebski· Dec 6, 2006, 18:27 UTC · re: Michael Loeffler · lore
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).
Show 7 quoted lines
> 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
Junio C Hamano· Dec 6, 2006, 18:39 UTC · re: Jakub Narebski · lore
Jakub Narebski <jnareb@gmail.com> writes:
Show 6 quoted lines
> 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.

Junio C Hamano· Dec 6, 2006, 18:37 UTC · re: Michael Loeffler · lore
Michael Loeffler <zvpunry@zvpunry.de> writes:
Show 11 quoted lines
>> 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.

Johannes Schindelin· Dec 6, 2006, 23:21 UTC · re: Jakub Narebski · lore
Hi,
On Wed, 6 Dec 2006, Jakub Narebski wrote:
Show 12 quoted lines
> 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,
Jakub Narebski· Dec 6, 2006, 23:38 UTC · re: Johannes Schindelin · lore
Johannes Schindelin wrote:
Show 20 quoted lines
> 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

← back to recent threads