[PATCH] git-remote: support remotes with a dot in the name

[PATCH] git-remote: support remotes with a dot in the name

[Pavel Roskin]

Ignore configuration data other that "url" and "fetch" for the remote. 
We cannot process it to extract the remote name from it reliably. 
Besides, a remote without "url" is currently invalid, so we are not
missing anything.

Signed-off-by: Pavel Roskin <proski@gnu.org>
---

 git-remote.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-remote.perl b/git-remote.perl
index 6e473ec..97b5f6c 100755
--- a/git-remote.perl
+++ b/git-remote.perl
@@ -67,7 +67,7 @@ sub list_remote {
 		$git->command(qw(config --get-regexp), '^remote\.');
 	};
 	for (@remotes) {
-		if (/^remote\.([^.]*)\.(\S*)\s+(.*)$/) {
+		if (/^remote\.(\S*)\.(fetch|url)\s+(.*)$/) {
 			add_remote_config(\%seen, $1, $2, $3);
 		}
 	}

Re: [PATCH] git-remote: support remotes with a dot in the name

[Junio C Hamano]

Pavel Roskin <proski@gnu.org> writes:

> Ignore configuration data other that "url" and "fetch" for the remote. 
> We cannot process it to extract the remote name from it reliably. 
> Besides, a remote without "url" is currently invalid, so we are not
> missing anything.
>
> Signed-off-by: Pavel Roskin <proski@gnu.org>

I do not think we ever officially "supported" remotes with a dot
in their names since $GIT_DIR/remotes/ or $GIT_DIR/branches
days.  

>  	for (@remotes) {
> -		if (/^remote\.([^.]*)\.(\S*)\s+(.*)$/) {
> +		if (/^remote\.(\S*)\.(fetch|url)\s+(.*)$/) {
>  			add_remote_config(\%seen, $1, $2, $3);
>  		}

I do not strongly oppose to allowing it now, but I suspect this
is probably less impact:

	if (/^remote\.(\S+?)\.([^.\s]+)\s+(.*)$/) {
        	...
	}

With this, we disallow whitespaces in remote names, but we leave
the door open for supporting variables other than fetch and url
by accepting the third token that matches any sequence of
non-dot, non-whitespace letters.

Re: [PATCH] git-remote: support remotes with a dot in the name

[Pavel Roskin]

Quoting Junio C Hamano <junkio@cox.net>:

> I do not think we ever officially "supported" remotes with a dot
> in their names since $GIT_DIR/remotes/ or $GIT_DIR/branches
> days.

On the other hand, I've been tracking "wireless-2.6" for months without even
realizing that the name could be a problem.  It is only when I tried the new
"git-remote update" that I noticed that it tries to update "wireless-2" and
fails.

> I do not strongly oppose to allowing it now, but I suspect this
> is probably less impact:
>
> 	if (/^remote\.(\S+?)\.([^.\s]+)\s+(.*)$/) {
>         	...
> 	}
>
> With this, we disallow whitespaces in remote names, but we leave
> the door open for supporting variables other than fetch and url
> by accepting the third token that matches any sequence of
> non-dot, non-whitespace letters.

It would be great as long as we don't use names with more than one dot after the
remote name (e.g. remote.wireless-2.6.url.push), but if you think it's unlikely,
I agree that your code is better.

--
Regards,
Pavel Roskin

Re: [PATCH] git-remote: support remotes with a dot in the name

[Junio C Hamano]

Pavel Roskin <proski@gnu.org> writes:

> On the other hand, I've been tracking "wireless-2.6" for months without even
> realizing that the name could be a problem.

Ah, sorry.  You are absolutely right.  Using wireless-2.6 to
name wireless-2.6 repository (or linux-2.6 to name linux-2.6
repository) makes perfect sense.

>> I do not strongly oppose to allowing it now, but I suspect this
>> is probably less impact:
>>
>> 	if (/^remote\.(\S+?)\.([^.\s]+)\s+(.*)$/) {
>>         	...
>> 	}
>>
>> With this, we disallow whitespaces in remote names, but we leave
>> the door open for supporting variables other than fetch and url
>> by accepting the third token that matches any sequence of
>> non-dot, non-whitespace letters.
>
> It would be great as long as we don't use names with more than
> one dot after the remote name
> (e.g. remote.wireless-2.6.url.push),...

Do you mean:

	[remote "wireless-2.6.url"]
		url = wire.less:/repo/sito/ry.git
        	fetch = +refs/heads/*:refs/remotes/wireless-2.6.url/*

If so I think my replacement would match it.  It will be
returned from "git config --get-regexp '^remote\.'"  like this:

remote.wireless-2.6.url.url wire.less:/repo/sito/ry.git
remote.wireless-2.6.url.fetch +refs/heads/*:refs/remotes/wireless-2.6.url/*

and in:

 	/^remote\.(\S+?)\.([^.\s]+)\s+(.*)$/

$1 would match shortest non-whitespace sequence after "remote.",
$2 would match longuest non-dot, non-whitespace sequence before
a sequence of whitespaces, and
$3 would match everything after that sequence of whitespaces.

So, $1 = "wireless-2.6.url", $2 = "url", $3 = "wire.less:/repo/sito/ry.git"
or $1 = "wireless-2.6.url", $2 = "fetch", $3 = "+refs/heads/...."

But my Perl is rusty, so please double check it.

Re: [PATCH] git-remote: support remotes with a dot in the name

[Pavel Roskin]

Quoting Junio C Hamano <junkio@cox.net>:

> Do you mean:
>
> 	[remote "wireless-2.6.url"]
> 		url = wire.less:/repo/sito/ry.git
>         	fetch = +refs/heads/*:refs/remotes/wireless-2.6.url/*

I was thinking of something like

[remote "wireless-2.6"]
url = http://foo/bar
url.push = ssh://foo/bar

But I think it's quite unlikely to be named like that.

> But my Perl is rusty, so please double check it.

The "(\S*?)" construct looks weird (you probably meant to use "(\S*)" for the
remote name), but the rest is probably OK.  I'll send the "double-checked"
patch tomorrow unless you beat me at that.

--
Regards,
Pavel Roskin

Re: [PATCH] git-remote: support remotes with a dot in the name

[Shawn O. Pearce]

Pavel Roskin <proski@gnu.org> wrote:
> I was thinking of something like
> 
> [remote "wireless-2.6"]
> url = http://foo/bar
> url.push = ssh://foo/bar

The key `url.push` is not a valid string in a config file.
 
> > But my Perl is rusty, so please double check it.

FWIW Junio's pattern looks OK to me.

> The "(\S*?)" construct looks weird (you probably meant to use "(\S*)" for the
> remote name), but the rest is probably OK.  I'll send the "double-checked"
> patch tomorrow unless you beat me at that.

The use of (\S+?) here is right.  We want to be non-greedy in our
matching of the remote name, as we don't want to overslurp and grab
through the key name and part of the key value into the remote name
by mistake, especially if the key value contained spaces.

-- 
Shawn.

Re: [PATCH] git-remote: support remotes with a dot in the name

[Junio C Hamano]

Pavel Roskin <proski@gnu.org> writes:

> I was thinking of something like
>
> [remote "wireless-2.6"]
> url = http://foo/bar
> url.push = ssh://foo/bar

Ahh.

I was not taking the above use case into account when I wrote
that regexp, because I do not think we ever supported (nor were
planning to ever support) names with dot at the third level.

> But I think it's quite unlikely to be named like that.

The second level ("wireless-2.6") is designed to be more lenient
to accept wider "user level names" for branches, remotes, etc.,
but the third level is for variable names the programs use, and
it is not the question of likely/unlikely, but more about how we
(as the git system builders, not the end users) want to name our
variables.  So I do not think it is not such a big deal if we do
not support "url.push" variable.

>> But my Perl is rusty, so please double check it.
>
> The "(\S*?)" construct looks weird (you probably meant to use "(\S*)" for the
> remote name), but the rest is probably OK.  I'll send the "double-checked"
> patch tomorrow unless you beat me at that.

Actually I did mean to write "*?"; wasn't it how minimal match
is written?

Re: [PATCH] git-remote: support remotes with a dot in the name

[Theodore Tso]

On Tue, Feb 20, 2007 at 10:18:55PM -0800, Junio C Hamano wrote:
> Do you mean:
> 
> 	[remote "wireless-2.6.url"]
> 		url = wire.less:/repo/sito/ry.git
>         	fetch = +refs/heads/*:refs/remotes/wireless-2.6.url/*

Or even more likely:

	[remote "stable-2.6.19"]
		url = git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.19.y.git
		fetch = +refs/heads/*:refs/remotes/stable-2.6.19/*

> If so I think my replacement would match it.  It will be
> returned from "git config --get-regexp '^remote\.'"  like this:
> 
> remote.wireless-2.6.url.url wire.less:/repo/sito/ry.git
> remote.wireless-2.6.url.fetch +refs/heads/*:refs/remotes/wireless-2.6.url/*
> 
> and in:
> 
>  	/^remote\.(\S+?)\.([^.\s]+)\s+(.*)$/
> 
> $1 would match shortest non-whitespace sequence after "remote.",
> $2 would match longuest non-dot, non-whitespace sequence before
> a sequence of whitespaces, and
> $3 would match everything after that sequence of whitespaces.
> 
> So, $1 = "wireless-2.6.url", $2 = "url", $3 = "wire.less:/repo/sito/ry.git"
> or $1 = "wireless-2.6.url", $2 = "fetch", $3 = "+refs/heads/...."
> 
> But my Perl is rusty, so please double check it.

Yep, looks good to me.

						- Ted