Implementing branch attributes in git config

Implementing branch attributes in git config

[Pavel Roskin]

Hello!

I have tried to implement branch attributes (more specifically, the
default for git-fetch) in git config.  It turns out that the format of
the config file is to rigid.

Minuses, underscores, colons and dots are not allowed in the section
headers and keys.  I can understand that dots should be allowed either
in the section names or in the key names, but other limitations seem
totally unnecessary.

I think a good implementation should allow any characters in the keys,
even "=", because the key can be quoted.  Section names may disallow
square brackets and dots.

Such limitations make it unpractical to use branch names in section or
key names.  I'd like to have it fixed.

The only remaining place is values.  This means that there should be
multiple entries for the same key.  While this is allowed, it seems
quite fragile and inconvenient.

In particular, git-repo-config leaves the config file locked in the
regex is wrong:

$ git-repo-config branch.fetch "master:origin" +
Invalid pattern: +
$ git-repo-config branch.fetch "master:origin" +
could not lock config file

To fix it, just add "close(fd); unlink(lock_file);" after "Invalid
pattern" in config.c.

I don't quite understand what pattern is needed to add an entry.  "foo"
seems to work fine, I don't know why.

That problem with multiple values is that they are quite fragile and
require special options to access them.  Since regex is used, dots in
the branch names need to be escaped.  Probably more escapes are needed.

Anyway, here's the preliminary patch that implements default fetch
branches.  Unfortunately, it doesn't even come close to the goal of
having per-branch attributes due to the config file limitations.

To add an entry, use

git-repo-config branch.fetch "localbranch:remotebranch" foo


Read the default fetch branch from the config file

From: Pavel Roskin <proski@gnu.org>


---

 git-fetch.sh |   21 +++++++++++++++++----
 1 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/git-fetch.sh b/git-fetch.sh
index 280f62e..a5ea92b 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -61,12 +61,25 @@ do
 	shift
 done
 
+get_config_rem_branch() {
+	fetch_branch=origin
+	cur_branch=$(expr $(git-symbolic-ref HEAD) : 'refs/heads/\(.*\)')
+	test -z "$cur_branch" && return
+	cur_branch_esc=$(echo $cur_branch | sed 's/\./\\./')
+	branch_map=$(git-repo-config --get branch.fetch "^$cur_branch_esc:")
+	test -z "$branch_map" && return
+	rem_branch=$(expr "$branch_map" : "$cur_branch_esc:\(.*\)")
+	test -z "$rem_branch" && return
+	fetch_branch="$rem_branch"
+}
+
 case "$#" in
 0)
-	test -f "$GIT_DIR/branches/origin" ||
-		test -f "$GIT_DIR/remotes/origin" ||
-			die "Where do you want to fetch from today?"
-	set origin ;;
+	get_config_rem_branch
+	test -f "$GIT_DIR/branches/$fetch_branch" ||
+		test -f "$GIT_DIR/remotes/$fetch_branch" ||
+			die "No remote branch \"$fetch_branch\""
+	set "$fetch_branch" ;;
 esac
 
 remote_nick="$1"


-- 
Regards,
Pavel Roskin

Re: Implementing branch attributes in git config

[Junio C Hamano]

Pavel Roskin <proski@gnu.org> writes:

> In particular, git-repo-config leaves the config file locked in the
> regex is wrong:
>
> $ git-repo-config branch.fetch "master:origin" +
> Invalid pattern: +
> $ git-repo-config branch.fetch "master:origin" +
> could not lock config file
>
> To fix it, just add "close(fd); unlink(lock_file);" after "Invalid
> pattern" in config.c.

I'd give Johannes the first refusal right to deal with this and
not touch repo-config.c myself for now, since I suspect I
tempted him enough to restructure it ;-).

> I don't quite understand what pattern is needed to add an entry.  "foo"
> seems to work fine, I don't know why.

I think the value regexp is "replace the ones that match this",
and the convention he came up with is to use "^$" to append (see
some examples in t/t1300-repo-config.sh).

In any case, Documentation/git-repo-config.txt mentions
value_regex without explaining what the semantics is.  This
needs to be fixed, probably like the attached patch.

> That problem with multiple values is that they are quite fragile and
> require special options to access them.  Since regex is used, dots in
> the branch names need to be escaped.  Probably more escapes are needed.

I have a suspicion that using regex while is more powerful and
expressive might be a mistake and it would be easier for users
(both Porcelain and end-users) to use fnmatch() patterns.

> Such limitations make it unpractical to use branch names in section or
> key names.  I'd like to have it fixed.

I thought that is what the "for blah" convention is for (BTW
"for" is not a keyword, just a convention).

Also the syntax is a bit confusing.  I initially was confused,
after looking at your example:

	[branch]
        	fetch = "localbranch:remotebranch"

that the colon separated value was a usual refspec, but it
isn't.  The LHS is the name of the current branch, and RHS is
the name of the remotes/ file, which is not a remote _branch_ at
all.  Perhaps something like this is semantically clearer, aside
from names -- I am bad at coming up with them:

	[branch]
		defaultremote = origin for master
                defaultremote = private for test

to mean, "we use remotes/origin when on master branch by
default".  Also we would want to use remote.origin.{url,fetch}
configuration item as well, so the "file existence" check you do
in this part of the patch is wrong.

>  case "$#" in
>  0)
> -	test -f "$GIT_DIR/branches/origin" ||
> -		test -f "$GIT_DIR/remotes/origin" ||
> -			die "Where do you want to fetch from today?"
> -	set origin ;;
> +	get_config_rem_branch
> +	test -f "$GIT_DIR/branches/$fetch_branch" ||
> +		test -f "$GIT_DIR/remotes/$fetch_branch" ||
> +			die "No remote branch \"$fetch_branch\""
> +	set "$fetch_branch" ;;
>  esac

We should instead let the existence check do the right thing for
"$1" when it is used; after all the default "origin" may not
exist, and I suspect we do not probably check that (or if we do
already, the above check is totally unnecessary).

I haven't thought things through but I have a suspicion that
get_config_rem_branch might belong to git-parse-remote.sh not
git-fetch.sh.

-- >8 --
diff --git a/Documentation/git-repo-config.txt b/Documentation/git-repo-config.txt
index fd44f62..660c18f 100644
--- a/Documentation/git-repo-config.txt
+++ b/Documentation/git-repo-config.txt
@@ -23,10 +23,11 @@ You can query/set/replace/unset options 
 actually the section and the key separated by a dot, and the value will be
 escaped.
 
-If you want to set/unset an option which can occur on multiple lines, you
-should provide a POSIX regex for the value. If you want to handle the lines
-*not* matching the regex, just prepend a single exclamation mark in front
-(see EXAMPLES).
+If you want to set/unset an option which can occur on multiple
+lines, a POSIX regexp `value_regex` needs to be given.  Only the
+existing values that match the regexp are updated or unset.  If
+you want to handle the lines that do *not* match the regex, just
+prepend a single exclamation mark in front (see EXAMPLES).
 
 The type specifier can be either '--int' or '--bool', which will make
 'git-repo-config' ensure that the variable(s) are of the given type and

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Sun, 7 May 2006, Pavel Roskin wrote:
>
> I think a good implementation should allow any characters in the keys,
> even "=", because the key can be quoted.  Section names may disallow
> square brackets and dots.

That wouldn't help. The thing is designed to not only need no quoting 
(except for the _value_), it also is designed to have both section and key 
names ignore case. So you really aren't supposed to put things like branch 
names (which are potentially case-sensitive, depending on filesystem) in 
either.

And we depend on that (and I think it's important - users normally should 
_not_ care about capitalization)

So you'd need to literally create a different syntax if you want unquoted 
section naming.

> Such limitations make it unpractical to use branch names in section or
> key names.  I'd like to have it fixed.

Here's a possible syntax/patch. I actually think the quoting makes more 
sense in the section name, since you'll usually want several keys under 
each branch, so it makes sense to make the _branch_ be the section.

It basically makes it a special case if you have the section name be 
marked with quotes, like

	["XyZZy"]

and in that case it turns the _real_ section name into the string 
"branch.XyZZy", where the important part is that it does this without 
changing case or limiting the character set (but it will _not_ allow a 
newline) in any way.

So you could have something like

	["Origin"]
		URL = ...
		fetch = master

and it would just turn it into

	branch.Origin.url = ...
	branch.Origin.fetch = master

etc.

No, I'm not sure this is the best possible syntax. It's just a suggestion. 
And it's certainly simple enough.

The downside is that if you start using config files like this, you 
literally can't go back to older git versions. They'll refuse to touch 
such a config file (rather than just ignoring the new entries) and will 
exit with nasty messages. That might be unacceptable.

Instead of quoting with double-quotes, it might be ok to use some 
alternate syntax line "[:$branchname:]" which looks visually reasonable, 
and has the potential advantage that ':' is already an invalid character 
in a branch name, so you don't actually even need any quoting logic at all 
at that point.

I think the

	["branch"]
		...

syntax looks reasonably readable and clean.

		Linus

----
diff --git a/config.c b/config.c
index 41066e4..802e326 100644
--- a/config.c
+++ b/config.c
@@ -134,9 +134,44 @@ static int get_value(config_fn_t fn, cha
 	return fn(name, value);
 }
 
+static int get_branch_name(char *name)
+{
+	int baselen = 7;
+	int quote = 0;
+
+	memcpy(name, "branch.", 7);
+	for (;;) {
+		int c = get_next_char();
+		if (c == EOF)
+			return -1;
+		if (c == '\n')
+			return -1;
+		if (!quote) {
+			if (c == '"')
+				break;
+			if (c == ']')
+				return -1;
+			if (c == '\\') {
+				quote = 1;
+				continue;
+			}
+		}
+		name[baselen++] = c;
+		if (baselen > MAXNAME / 2)
+			return -1;
+	}
+	if (get_next_char() != ']')
+		return -1;
+	return baselen;
+}
+
 static int get_base_var(char *name)
 {
 	int baselen = 0;
+	int c = get_next_char();
+
+	if (c == '"')
+		return get_branch_name(name);
 
 	for (;;) {
 		int c = get_next_char();

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Sun, 7 May 2006, Linus Torvalds wrote:
>
> And it's certainly simple enough.

Well, simple enough to be buggy.

>  static int get_base_var(char *name)
>  {
>  	int baselen = 0;
> +	int c = get_next_char();
> +
> +	if (c == '"')
> +		return get_branch_name(name);
>  
>  	for (;;) {
>  		int c = get_next_char();

You also need to move the "int c = get_next_char()" in the for() loop down 
to the end of the loop (removing the "int", of course).

So here's the fixed thing in case people care (and this time I actually 
looked at whether it might even work, not just compile ;)

		Linus
---
diff --git a/config.c b/config.c
index 41066e4..69d451a 100644
--- a/config.c
+++ b/config.c
@@ -134,12 +134,46 @@ static int get_value(config_fn_t fn, cha
 	return fn(name, value);
 }
 
+static int get_branch_name(char *name)
+{
+	int baselen = 7;
+	int quote = 0;
+
+	memcpy(name, "branch.", 7);
+	for (;;) {
+		int c = get_next_char();
+		if (c == EOF)
+			return -1;
+		if (c == '\n')
+			return -1;
+		if (!quote) {
+			if (c == '"')
+				break;
+			if (c == ']')
+				return -1;
+			if (c == '\\') {
+				quote = 1;
+				continue;
+			}
+		}
+		name[baselen++] = c;
+		if (baselen > MAXNAME / 2)
+			return -1;
+	}
+	if (get_next_char() != ']')
+		return -1;
+	return baselen;
+}
+
 static int get_base_var(char *name)
 {
 	int baselen = 0;
+	int c = get_next_char();
+
+	if (c == '"')
+		return get_branch_name(name);
 
 	for (;;) {
-		int c = get_next_char();
 		if (c == EOF)
 			return -1;
 		if (c == ']')
@@ -149,6 +183,7 @@ static int get_base_var(char *name)
 		if (baselen > MAXNAME / 2)
 			return -1;
 		name[baselen++] = tolower(c);
+		c = get_next_char();
 	}
 }
 

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Sun, 7 May 2006, Linus Torvalds wrote:
>
> The downside is that if you start using config files like this, you 
> literally can't go back to older git versions. They'll refuse to touch 
> such a config file (rather than just ignoring the new entries) and will 
> exit with nasty messages. That might be unacceptable.

Side note: with this syntax, the _users_ will all just basically do

	if (!strncmp(name, "branch.", 7)) {
		branch = name + 7;
		dot = strchr(branch, '.');
		if (!dot)
			return -1;
		*dot++ = 0;
		.. we now have the branchname in "branc",
		    and the rest in "dot" ..

and if your branch names are purely alphabetical and lower-case, you can 
now write

	[branch.origin]
		remote = true
		url = git://git.kernel.org/...
		fetch = master

	[branch.master]
		pull = origin

and it will be parsed _exactly_ the same as

	["origin"]
		remote = true
		url = git://git.kernel.org/...
		fetch = master

	["master"]
		pull = origin

while the [branch.origin] syntax allows old versions of git to happily 
ignore it. So that would be a kind of cheesy work-around: the new 
double-quoted format is only _required_ for any branch-names that have 
special characters in it.

		Linus

Re: Implementing branch attributes in git config

[sean]

On Sun, 7 May 2006 17:05:26 -0700 (PDT)
Linus Torvalds <torvalds@osdl.org> wrote:

> So you could have something like
> 
> 	["Origin"]
> 		URL = ...
> 		fetch = master
> 
> and it would just turn it into
> 
> 	branch.Origin.url = ...
> 	branch.Origin.fetch = master
> 

Having magic sections that prepend "branch." seems a bit suspect;
why not just be explicit:

  [branch.Origin]
      URL = ...
      fetch = master

Wouldn't it be reasonable for git to impose modest restrictions on
branch names; such as restricting them to [a-zA-Z0-9] characters?
Then we just have to make section names case sensitve within the
config file; keys could still be case insensitive.

Actually it would be nice if we were consistent.  If case matters to
git then the config file should be case sensitive.  If case doesn't
matter to git, then it should consider "Branch", "branch" and "BrAnCh"
as the same in all contexts (eg. git branch -b BrAnCh).  It seems
silly for us to say people are too dumb to handle case sensitivity
in a config file, but it's perfectly acceptable everywhere else.

Sean

Re: Implementing branch attributes in git config

[Pavel Roskin]

Hello, Linus!

On Sun, 2006-05-07 at 17:05 -0700, Linus Torvalds wrote:

> The downside is that if you start using config files like this, you 
> literally can't go back to older git versions. They'll refuse to touch 
> such a config file (rather than just ignoring the new entries) and will 
> exit with nasty messages. That might be unacceptable.

You code faster that I write e-mails :-)

I like your approach, even though it breaks compatibility.  I understand
we are going to more .git/remotes to the config, so compatibility will
be broken anyways.

I'm only concerned that we would be hardcoding the word "branch".  We
could need fancy section names for other things in the future.

-- 
Regards,
Pavel Roskin

Re: Implementing branch attributes in git config

[Johannes Schindelin]

Hi,

On Sun, 7 May 2006, Junio C Hamano wrote:

> Pavel Roskin <proski@gnu.org> writes:
> 
> > In particular, git-repo-config leaves the config file locked in the
> > regex is wrong:
> >
> > $ git-repo-config branch.fetch "master:origin" +
> > Invalid pattern: +
> > $ git-repo-config branch.fetch "master:origin" +
> > could not lock config file
> >
> > To fix it, just add "close(fd); unlink(lock_file);" after "Invalid
> > pattern" in config.c.
> 
> I'd give Johannes the first refusal right to deal with this and
> not touch repo-config.c myself for now, since I suspect I
> tempted him enough to restructure it ;-).

Thanks. Yes, you tempted me real hard :-) Unfortunately, the restructuring 
will have to wait a little, because I really should work for my day job 
these days... :-(

> > I don't quite understand what pattern is needed to add an entry.  "foo"
> > seems to work fine, I don't know why.
> 
> I think the value regexp is "replace the ones that match this",
> and the convention he came up with is to use "^$" to append (see
> some examples in t/t1300-repo-config.sh).
> 
> In any case, Documentation/git-repo-config.txt mentions
> value_regex without explaining what the semantics is.  This
> needs to be fixed, probably like the attached patch.
> 
> > That problem with multiple values is that they are quite fragile and
> > require special options to access them.  Since regex is used, dots in
> > the branch names need to be escaped.  Probably more escapes are needed.
> 
> I have a suspicion that using regex while is more powerful and
> expressive might be a mistake and it would be easier for users
> (both Porcelain and end-users) to use fnmatch() patterns.

I did not know about fnmatch()... It is probably better than regular 
expressions. After all, what I use it for most often is

	git-repo-config --get-all remote.*url

which -- magically -- will continue to work with fnmatch()!

> 	[branch]
> 		defaultremote = origin for master
>                 defaultremote = private for test

FWIW I like that syntax much better. But then, somebody called me weird 
because of how I order the arguments of a comparison... tsk, tsk.

> @@ -23,10 +23,11 @@ You can query/set/replace/unset options 
>  actually the section and the key separated by a dot, and the value will be
>  escaped.
>  
> -If you want to set/unset an option which can occur on multiple lines, you
> -should provide a POSIX regex for the value. If you want to handle the lines
> -*not* matching the regex, just prepend a single exclamation mark in front
> -(see EXAMPLES).
> +If you want to set/unset an option which can occur on multiple
> +lines, a POSIX regexp `value_regex` needs to be given.  Only the
> +existing values that match the regexp are updated or unset.  If
> +you want to handle the lines that do *not* match the regex, just
> +prepend a single exclamation mark in front (see EXAMPLES).

I would actually prefer to go with your suggestion of using shell patterns 
instead of regular expressions. They are not needed, and most users tend 
to positively hate regular expressions.

Thoughts?

Ciao,
Dscho

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Sun, 7 May 2006, Pavel Roskin wrote:
> 
> I'm only concerned that we would be hardcoding the word "branch".  We
> could need fancy section names for other things in the future.

Fair enough. We could have used some fake section name that you can't 
generate any other way (in fact, "Branch.$branchname" would be that), but 
the upside of using "branch" is exactly that you _can_ generate it with 
the old-style syntax that is acceptable to older git versions too.

So the common case (all-lowercase, no special characters branch names) 
wouldn't need to break.

Now, backwards competibility for the .git/config file isn't likely a huge 
issue, but it does matter if you want to do things like "git bisect" to 
bisect a totally unrelated bug, and part of the bisection is actually to 
install the older git version that you're testing for the bug..

(Which is probably an insane thing to do anyway - you should be able to 
test any bugs _without_ actually having to install the git version you're 
testing. But whatever..)

		Linus

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Sun, 7 May 2006, sean wrote:
> 
> Having magic sections that prepend "branch." seems a bit suspect;
> why not just be explicit:
> 
>   [branch.Origin]
>       URL = ...
>       fetch = master

Exactly because section (and key) names are normally not case sensitive.

Even the documentation actually talks about "core.fileMode" and "[imap] 
Folders". 

		Linus

Re: Implementing branch attributes in git config

[Pavel Roskin]

On Sun, 2006-05-07 at 17:55 -0700, Linus Torvalds wrote:
> 
> On Sun, 7 May 2006, sean wrote:
> > 
> > Having magic sections that prepend "branch." seems a bit suspect;
> > why not just be explicit:
> > 
> >   [branch.Origin]
> >       URL = ...
> >       fetch = master
> 
> Exactly because section (and key) names are normally not case sensitive.
> 
> Even the documentation actually talks about "core.fileMode" and "[imap] 
> Folders". 

Make it ["branch.Origin"]

No hardcoded "branch" prepending needed.  The case sensitive name is
still protected by quotes.  This extends trivially to ["user.Linus"] or
["path./src/git.c"] or whatever.

-- 
Regards,
Pavel Roskin

Re: Implementing branch attributes in git config

[Johannes Schindelin]

Hi,

On Sun, 7 May 2006, Linus Torvalds wrote:

> [...]
> 
> and if your branch names are purely alphabetical and lower-case, you can 
> now write
> 
> 	[branch.origin]
> 		remote = true
> 		url = git://git.kernel.org/...
> 		fetch = master
> 
> 	[branch.master]
> 		pull = origin
> 
> and it will be parsed _exactly_ the same as
> 
> 	["origin"]
> 		remote = true
> 		url = git://git.kernel.org/...
> 		fetch = master
> 
> 	["master"]
> 		pull = origin
> 
> while the [branch.origin] syntax allows old versions of git to happily 
> ignore it. So that would be a kind of cheesy work-around: the new 
> double-quoted format is only _required_ for any branch-names that have 
> special characters in it.

Eek.

The ["blablabla"] syntax fails the is-it-obvious-what-this-does test. What 
*is* wrong with the " for " syntax? IIRC it was even proposed by you, and 
it happens to be backward compatible.

Ciao,
Dscho

Re: Implementing branch attributes in git config

[sean]

On Sun, 7 May 2006 17:55:25 -0700 (PDT)
Linus Torvalds <torvalds@osdl.org> wrote:

> On Sun, 7 May 2006, sean wrote:
> > 
> > Having magic sections that prepend "branch." seems a bit suspect;
> > why not just be explicit:
> > 
> >   [branch.Origin]
> >       URL = ...
> >       fetch = master
> 
> Exactly because section (and key) names are normally not case sensitive.

Restore case sensitivity to config file parsing and the problem largely goes
away.  Or go the other route and remove case sensitivity from all the other 
bits (branches names etc..).

Sean

Re: Implementing branch attributes in git config

[Pavel Roskin]

Hello, Johannes!

On Mon, 2006-05-08 at 03:05 +0200, Johannes Schindelin wrote:
> The ["blablabla"] syntax fails the is-it-obvious-what-this-does test. What 
> *is* wrong with the " for " syntax? IIRC it was even proposed by you, and 
> it happens to be backward compatible.

Not trying to answer for Linus, here's my take.

The "for" syntax is one more layer in the config hierarchy.  Adding
another layer is a more intrusive solution than relaxing the syntax of
the existing elements without changing their semantic.

git-repo-config is "for" agnostic.  It doesn't parse "for" (as far as I
know).

"for" can be confusing in some contexts, or may force inversion of the
hierarchy to make the config file more readable.  How would you
implement branch descriptions?  See this:

[branchdata]
description = "netdev" for "Network device development"

and this

[branchdata]
description = "Network device development" for "netdev"

The later is closer to English.  Or should we use the first approach and
"is" instead of "for"?

Now, how can I get a description for the "netdev" branch by one
git-repo-config command, without pipes?

-- 
Regards,
Pavel Roskin

Re: Implementing branch attributes in git config

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> On Sun, 7 May 2006, Pavel Roskin wrote:
>> 
>> I'm only concerned that we would be hardcoding the word "branch".  We
>> could need fancy section names for other things in the future.
>
> Fair enough. We could have used some fake section name that you can't 
> generate any other way (in fact, "Branch.$branchname" would be that), but 
> the upside of using "branch" is exactly that you _can_ generate it with 
> the old-style syntax that is acceptable to older git versions too.

Sorry, I do not follow the old-style syntax part.

How about keeping the default syntax as it is (tokens are case
insensitive and alnums only, dot separates tokens into
sections), and when a token that violates that rule needs to be
spelled out, require quoting, so:

	branch.foo	BranCh.FoO	branch.FOO

are the same (section "branch.foo"), and if I have js/fmt.patch
branch, I need to spell the configuration for that branch like
so:

	branch."js/fmt.patch"	or   "branch.js/fmt.patch"        

and the URL variable for that section is

	$ git repo-config '"branch.js/fmt.patch".url'

(BTW, you could even have a variable with dots in it by quoting
the variable name, like "branch.js/fmt.patch"."fetch.option"; I
do not know if it is worth it).

My repository is full of topic branches that are named xx/yyyy.
It is very handy to be able to say "show-branch --topics master
'heads/??/*' next" which would not show my other branches like
"test", "throwaway", "rework", "temp", etc.

Re: Implementing branch attributes in git config

[Johannes Schindelin]

Hi,

On Sun, 7 May 2006, Pavel Roskin wrote:

> On Mon, 2006-05-08 at 03:05 +0200, Johannes Schindelin wrote:
> > The ["blablabla"] syntax fails the is-it-obvious-what-this-does test. What 
> > *is* wrong with the " for " syntax? IIRC it was even proposed by you, and 
> > it happens to be backward compatible.
> 
> [branchdata]
> description = "netdev" for "Network device development"
> 
> and this
> 
> [branchdata]
> description = "Network device development" for "netdev"

The latter was how I recall the syntax proposed for proxies.

> Now, how can I get a description for the "netdev" branch by one
> git-repo-config command, without pipes?

	git-repo-config --get branchdata.description ' for netdev$'

Hth,
Dscho

Re: Implementing branch attributes in git config

[sean]

On Sun, 07 May 2006 18:27:32 -0700
Junio C Hamano <junkio@cox.net> wrote:


> How about keeping the default syntax as it is (tokens are case
> insensitive and alnums only, dot separates tokens into
> sections), and when a token that violates that rule needs to be
> spelled out, require quoting, so:
> 
> 	branch.foo	BranCh.FoO	branch.FOO
 
> are the same (section "branch.foo"),

Doesn't that mean you have to then prohibit creating mixed
case branches with "git branch" and "git checkout -b" ?

> and if I have js/fmt.patch
> branch, I need to spell the configuration for that branch like
> so:
> 
> 	branch."js/fmt.patch"	or   "branch.js/fmt.patch"        
> 
> and the URL variable for that section is
> 
> 	$ git repo-config '"branch.js/fmt.patch".url'

How about transforming slashes into dots?  so the above would 
be:

   [branch.js.fmt.patch]

      And could be accessed by either:

    $ git repo-config branch.js.fmt.patch
    $ git repo-config branch.js/fmt.patch

> (BTW, you could even have a variable with dots in it by quoting
> the variable name, like "branch.js/fmt.patch"."fetch.option"; I
> do not know if it is worth it).

Not worth it.  Branch names should be alnums and imho should be
case sensitive too.
 
> My repository is full of topic branches that are named xx/yyyy.
> It is very handy to be able to say "show-branch --topics master
> 'heads/??/*' next" which would not show my other branches like
> "test", "throwaway", "rework", "temp", etc.

Very nice.

Sean

Re: Implementing branch attributes in git config

[sean]

On Mon, 8 May 2006 03:45:47 +0200 (CEST)
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> Hi,
> 
> On Sun, 7 May 2006, sean wrote:
> 
> > Not worth it.  Branch names should be alnums and imho should be
> > case sensitive too.
> 
> Why should they be case sensitive? So you have a branch "origin" and 
> another named "Origin" and get totally confused?
> 

I don't care either way, just think we should be consistent.  Currently
we support case sensitivity in branch names and let people get 
totally confused.   But in practice people don't really get confused
by the linux filesystem which is case sensitive.

Sean

Re: Implementing branch attributes in git config

[Johannes Schindelin]

Hi,

On Sun, 7 May 2006, sean wrote:

> Not worth it.  Branch names should be alnums and imho should be
> case sensitive too.

Why should they be case sensitive? So you have a branch "origin" and 
another named "Origin" and get totally confused?

Ciao,
Dscho

Re: Implementing branch attributes in git config

[Pavel Roskin]

On Mon, 2006-05-08 at 03:27 +0200, Johannes Schindelin wrote:
> > Now, how can I get a description for the "netdev" branch by one
> > git-repo-config command, without pipes?
> 
> 	git-repo-config --get branchdata.description ' for netdev$'

No, it doesn't remove "for netdev".  What I really don't like is that
git-repo-config treats it as "not my problem".

git-repo-config places extremely tight limitations of the names of the
sections and the keys.  But sometimes a relationship between two loosely
defined strings needs to be presented.  It's a real need.  And
git-repo-config doesn't address this need.

I believe git-repo-config should allow direct retrieval of data from any
depth, and the syntax should be explicit rather than fuzzy.  A dot is
more explicit than "for", especially if the dot appears after a name
that may not contain dots.

Another question is how we want to group the data.  Do we want to have
all descriptions together or in separate sections?  Whatever the answer,
git-repo-config should provide means to extract all data in one command,
without need for postprocessing.

So I understand arguing where to place the branch name.  But what I
don't like is the desire to offload part of the processing on the
callers.

-- 
Regards,
Pavel Roskin

Re: Implementing branch attributes in git config

[Junio C Hamano]

sean <seanlkml@sympatico.ca> writes:

> On Sun, 07 May 2006 18:27:32 -0700
> Junio C Hamano <junkio@cox.net> wrote:
>
>
>> How about keeping the default syntax as it is (tokens are case
>> insensitive and alnums only, dot separates tokens into
>> sections), and when a token that violates that rule needs to be
>> spelled out, require quoting, so:
>> 
>> 	branch.foo	BranCh.FoO	branch.FOO
>  
>> are the same (section "branch.foo"),
>
> Doesn't that mean you have to then prohibit creating mixed
> case branches with "git branch" and "git checkout -b" ?

Not at all.  Whatever Porcelain that runs repo-config to record
the branch name needs to spell that branch name with proper
quoting, like:

>> 	branch."js/fmt.patch"	or   "branch.js/fmt.patch"        
>> 
>> and the URL variable for that section is
>> 
>> 	$ git repo-config '"branch.js/fmt.patch".url'
>
> How about transforming slashes into dots?  so the above would 
> be:
>
>    [branch.js.fmt.patch]

I _do_ want to keep my slashes intact and also dots; mangling
them is not very nice to me X-<.

Re: Implementing branch attributes in git config

[sean]

On Sun, 07 May 2006 19:29:50 -0700
Junio C Hamano <junkio@cox.net> wrote:


> Not at all.  Whatever Porcelain that runs repo-config to record
> the branch name needs to spell that branch name with proper
> quoting, like:

Okay.  It just seems nuts to require quoting because you happen
to use an uppercase character.  People are used to quoting 
special characters like * and $, not uppercase letters.

> I _do_ want to keep my slashes intact and also dots; mangling
> them is not very nice to me X-<.

You're right.

We should just relax the config file rules for legal characters,
in identifiers, at least [a-zA-Z0-9_/-].

Sean.

Re: Implementing branch attributes in git config

[Junio C Hamano]

Pavel Roskin <proski@gnu.org> writes:

> On Mon, 2006-05-08 at 03:27 +0200, Johannes Schindelin wrote:
>> > Now, how can I get a description for the "netdev" branch by one
>> > git-repo-config command, without pipes?
>> 
>> 	git-repo-config --get branchdata.description ' for netdev$'
>
> No, it doesn't remove "for netdev".  What I really don't like is that
> git-repo-config treats it as "not my problem".

Stating what you do not like about something is a good first
step to improve that something.  It should not be too hard to
extend the parser to grok:

	repo-config --get branchdata.description '\(.*\) for netdev$'

and when the value_regex has a capture return what matches
instead of the entire value.  I think that would do what you
want.

Re: Implementing branch attributes in git config

[Johannes Schindelin]

Hi,

On Mon, 8 May 2006, Junio C Hamano wrote:

> 	repo-config --get branchdata.description '\(.*\) for netdev$'

POSIX regexps do not want the backslashes...

Something like this?

---
[PATCH] repo-config: allow one group in value regexp

The regular expression for the value can now contain one group. In case
there is one, the output will be just that group, not the whole value.
Now you can say

        git-repo-config --get branch.defaultremote '(.*) for junio'

and for a config like this:

        [branch]
                defaultRemote = sushi for junio

the output will be "sushi".

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 repo-config.c |   30 ++++++++++++++++++++----------
 1 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/repo-config.c b/repo-config.c
index 63eda1b..9ac4c51 100644
--- a/repo-config.c
+++ b/repo-config.c
@@ -26,31 +26,41 @@ static int show_all_config(const char *k
 static int show_config(const char* key_, const char* value_)
 {
 	char value[256];
-	const char *vptr = value;
+	const char *vptr = value_;
 	int dup_error = 0;
 
 	if (value_ == NULL)
-		value_ = "";
+		vptr = value_ = "";
 
 	if (!use_key_regexp && strcmp(key_, key))
 		return 0;
 	if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0))
 		return 0;
-	if (regexp != NULL &&
-			 (do_not_match ^
-			  regexec(regexp, value_, 0, NULL, 0)))
-		return 0;
+	if (regexp != NULL) {
+		regmatch_t matches[2];
+		int len;
+
+		if (do_not_match ^ regexec(regexp, value_, 2, matches, 0))
+			return 0;
+		len = matches[1].rm_eo - matches[1].rm_so;
+		if (!do_not_match && len > 0) {
+			if (len > 255)
+				len = 255;
+			strncpy(value, value_ + matches[1].rm_so, len);
+			value[len] = 0;
+			vptr = value;
+		}
+	}
 
 	if (show_keys)
 		printf("%s ", key_);
 	if (seen && !do_all)
 		dup_error = 1;
-	if (type == T_INT)
+	if (type == T_INT) {
 		sprintf(value, "%d", git_config_int(key_, value_));
-	else if (type == T_BOOL)
+		vptr = value;
+	} else if (type == T_BOOL)
 		vptr = git_config_bool(key_, value_) ? "true" : "false";
-	else
-		vptr = value_;
 	seen++;
 	if (dup_error) {
 		error("More than one value for the key %s: %s",
-- 
1.3.1.g297e8-dirty

Re: Implementing branch attributes in git config

[Pavel Roskin]

On Mon, 2006-05-08 at 02:00 -0700, Junio C Hamano wrote:
> Stating what you do not like about something is a good first
> step to improve that something.  It should not be too hard to
> extend the parser to grok:
> 
> 	repo-config --get branchdata.description '\(.*\) for netdev$'
> 
> and when the value_regex has a capture return what matches
> instead of the entire value.  I think that would do what you
> want.

OK, that would be acceptable.

I still don't like the "for" conversion because it masquerades syntax
(note that text to right of "for" must be unique) as plain text, but
it's a matter of taste, so it's hard for me to argue about it.

-- 
Regards,
Pavel Roskin

Re: Implementing branch attributes in git config

[Daniel Barkalow]

On Sun, 7 May 2006, sean wrote:

> On Sun, 07 May 2006 19:29:50 -0700
> Junio C Hamano <junkio@cox.net> wrote:
> 
> 
> > Not at all.  Whatever Porcelain that runs repo-config to record
> > the branch name needs to spell that branch name with proper
> > quoting, like:
> 
> Okay.  It just seems nuts to require quoting because you happen
> to use an uppercase character.  People are used to quoting 
> special characters like * and $, not uppercase letters.

You could tell people always to use:

 [branch."name"]

even if the branch name is all lowercase anyway. They could even use:

 [Branch."MyMixedCaseBranch"]

Then when you refer to something case-sensitive with the possibility of 
funny characters, you put it in quotes, regardless of what it is.

For that matter, we could retain the quotes when we parse the file, and 
reject [branch.master] for lacking the quotes, so that people who are only 
exposed to branch names all in lowercase letters don't get habits that 
will fail when they have a v2.6.16.x branch.

I don't think that people are likely to use older versions of git on the 
same repository they've used newer versions on. (Clones of it, sure, but 
that doesn't matter here.) But we should, in any case, make the code 
ignore sections or lines with syntax errors, under the assumption that 
they're a later extension and possibly legal but not anything the code 
could be interested in getting from a parser that doesn't support them.

	-Daniel
*This .sig left intentionally blank*

Re: Implementing branch attributes in git config

[sean]

On Mon, 8 May 2006 19:20:17 -0400 (EDT)
Daniel Barkalow <barkalow@iabervon.org> wrote:

> You could tell people always to use:
> 
>  [branch."name"]
> 
> even if the branch name is all lowercase anyway. They could even use:
> 
>  [Branch."MyMixedCaseBranch"]
> 
> Then when you refer to something case-sensitive with the possibility of 
> funny characters, you put it in quotes, regardless of what it is.
> 
> For that matter, we could retain the quotes when we parse the file, and 
> reject [branch.master] for lacking the quotes, so that people who are only 
> exposed to branch names all in lowercase letters don't get habits that 
> will fail when they have a v2.6.16.x branch.
> 
> I don't think that people are likely to use older versions of git on the 
> same repository they've used newer versions on. (Clones of it, sure, but 
> that doesn't matter here.) But we should, in any case, make the code 
> ignore sections or lines with syntax errors, under the assumption that 
> they're a later extension and possibly legal but not anything the code 
> could be interested in getting from a parser that doesn't support them.


Unfortunately that's not the only place where you have to use the 
excessive quoting; you also have to do the same when using the git
repo-config command line.   All because we're clinging to case
insensitivity and a very restrictive set of characters for 
identifiers in the config file.  And just why are we clinging?

Believe when Linus offered the code originally he said that it was
easier to start out very restrictive and loosen up later than it was
to start loose and become restrictive later.  That makes a lot of
sense, but somewhere along the way we seem to have made a virtue
out of something that is actually getting in the way of natural
syntactic usage.  There's no good reason for git to break with 
traditional and common practice in this case.

Sean

Re: Implementing branch attributes in git config

[Johannes Schindelin]

Hi,

On Mon, 8 May 2006, Daniel Barkalow wrote:

> You could tell people always to use:
> 
>  [branch."name"]

I find this utterly ugly.

> I don't think that people are likely to use older versions of git on the 
> same repository they've used newer versions on. (Clones of it, sure, but 
> that doesn't matter here.) But we should, in any case, make the code 
> ignore sections or lines with syntax errors, under the assumption that 
> they're a later extension and possibly legal but not anything the code 
> could be interested in getting from a parser that doesn't support them.

I have to bisect git sometimes, just because I have some local changes. 
Older gits do barf with a fatal error when encountering a bad config.

Further, it is probably not a good idea to relax error-checking. It is too 
easy to overlook.

Ciao,
Dscho

Re: Implementing branch attributes in git config

[Johannes Schindelin]

Hi,

On Mon, 8 May 2006, sean wrote:

> There's no good reason for git to break with traditional and common 
> practice in this case.

It is well established common practice that ini files (and everything in 
config resembles an ini file) have case insensitive section headers as 
well as keys.

Ciao,
Dscho

Re: Implementing branch attributes in git config

[sean]

On Tue, 9 May 2006 01:44:31 +0200 (CEST)
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> Hi,
> 
> On Mon, 8 May 2006, sean wrote:
> 
> > There's no good reason for git to break with traditional and common 
> > practice in this case.
> 
> It is well established common practice that ini files (and everything in 
> config resembles an ini file) have case insensitive section headers as 
> well as keys.

Not in ini files that support section headers that represent filenames
and directories.  Exactly because those things _are_ case sensitive 
and include more characters than just alnums.  But we're not just 
talking about the config file syntax we're talking about how the 
user must ultimately refer to branches with uppercase character.  
Now everytime a person wants to add a branch attribute via repo-config
they have to remember that git thinks uppercase characters should
be quoted.  Doesn't that sound ridiculous to you?

The point is that we should make the config file and the repo-config
command easy for the _users_.   Instead we're going to make them use
some crazy extra syntax because we refuse to come to terms with the
limitations of the choices we've made so far.

One option, which I don't really like and comes with its own set of 
problems, would be to do something like:

[branch1]
    streetname = "p4/BrAnCH"
[branch2]
    streetname = "origin"

And then allow reference to it from git-repo-config by either branch#
or the value of street name.  While it would take some extra coding
but at least it lives within the current restrictions for key names.

It just seems that once you have to even consider options like this,
a big red flag should be raised about some of the assumptions we've
built into the system.

Sean

Re: Implementing branch attributes in git config

[Johannes Schindelin]

Hi,

On Mon, 8 May 2006, sean wrote:

> [branch1]
>     streetname = "p4/BrAnCH"
> [branch2]
>     streetname = "origin"

Conceptually, it feels just wrong to me that a section name should contain 
a filename. A section is something which contains data regarding a certain 
concept. Such as "core", which has values concerning the core of git.

In that sense, I stay with my preferance:

[branch]
	defaultRemote = bla for bloop
	defaultRemote = connery for sean

No need to extend the simple config file. And remember, it is so simple, 
because it is so strict.

Ciao,
Dscho

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Mon, 8 May 2006, sean wrote:
> 
> One option, which I don't really like and comes with its own set of 
> problems, would be to do something like:
> 
> [branch1]
>     streetname = "p4/BrAnCH"
> [branch2]
>     streetname = "origin"

You don't actually need that.

We could easily do

	[branch]
		name = "p4/BrAnCH"
		url = git://git.kernel.org/...
		pull = master

	;
	; Repeating the "[branch]" section here isn't
	;  needed, but doesn't hurt either, and is
	; more readable
	;
	[branch]
		name = "origin"
		url = ...
		pull = ...

because the config file is always parsed linearly, and just 
trigger on "branch.name", and keep that around when parsing 
anything else.

The problem with _that_ is that "git repo-config" can't add this kind of 
setup sanely: it doesn't understand that kind of statefulness.

The above would work without any changes what-so-ever to the config format 
and readers - and old versions of git wouldn't react badly either. But the 
writer would need to be taught about state.

		Linus

Re: Implementing branch attributes in git config

[sean]

On Mon, 8 May 2006 17:37:29 -0700 (PDT)
Linus Torvalds <torvalds@osdl.org> wrote:

> We could easily do
> 
> 	[branch]
> 		name = "p4/BrAnCH"
> 		url = git://git.kernel.org/...
> 		pull = master
> 
> 	;
> 	; Repeating the "[branch]" section here isn't
> 	;  needed, but doesn't hurt either, and is
> 	; more readable
> 	;
> 	[branch]
> 		name = "origin"
> 		url = ...
> 		pull = ...
> 
> because the config file is always parsed linearly, and just 
> trigger on "branch.name", and keep that around when parsing 
> anything else.

That was pretty much what I was suggesting although i overlooked 
the fact that you don't actually need to number the sections.

> The problem with _that_ is that "git repo-config" can't add this kind of 
> setup sanely: it doesn't understand that kind of statefulness.

Shouldn't be too hard to deal with.  So we would allow the following
command to make the url entry above:

    $ git repo-config branch.p4/BrAnCH.url git://git.kernel.org/...

> The above would work without any changes what-so-ever to the config format 
> and readers - and old versions of git wouldn't react badly either. But the 
> writer would need to be taught about state.

Well it seems like maybe the best compromise, and it avoids having to put
files and directories into section names which Johannes was objecting to.

Sean

Re: Implementing branch attributes in git config

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> On Mon, 8 May 2006, sean wrote:
>> 
>> One option, which I don't really like and comes with its own set of 
>> problems, would be to do something like:
>> 
>> [branch1]
>>     streetname = "p4/BrAnCH"
>> [branch2]
>>     streetname = "origin"
>
> You don't actually need that.
>
> We could easily do
>
> 	[branch]
> 		name = "p4/BrAnCH"
> 		url = git://git.kernel.org/...
> 		pull = master
>
> 	;
> 	; Repeating the "[branch]" section here isn't
> 	;  needed, but doesn't hurt either, and is
> 	; more readable
> 	;
> 	[branch]
> 		name = "origin"
> 		url = ...
> 		pull = ...
>
> because the config file is always parsed linearly, and just 
> trigger on "branch.name", and keep that around when parsing 
> anything else.
>
> The problem with _that_ is that "git repo-config" can't add this kind of 
> setup sanely: it doesn't understand that kind of statefulness.

Wait a minute...  Statefulness is not the issue, I think.

How would you tell your updated repo-config what to update and
what to look up?

	- I want the url for branch whose name is "origin"

	- I want to fetch their "for-linus" branch when fetching
          from the branch whose name is "jgarzik" from now on.

In these query and update, you _are_ naming the branch with
name="xxx"; you just made "name" a special attribute.

Now, how would that compare with:

        [branch.jgarzik]
                url = git://git.kernel.org/...
                fetch = for-linus

or
	[branch."JGarzik"]
                url = git://git.kernel.org/...
                fetch = for-linus

I would say if we are grouping things together, if we can give
name to each group, _and_ if we are going to refer to the group
with its name, we are better off making the groups into distinct
sections _and_ make the syntax obvious that what name refers to
the section.  I think [branch.jgarzik] syntax is more obvious
than your example where "name =" line is implicitly used as a
keyword to differenciate multiple occurrences of [branch]
sections.

Having said that, perhaps you have something more elaborate in mind,
e.g.

	repo-config --get branch.url where name = 'origin'
	repo-config --get branch.name where url like 'git://%'
	repo-config branch.url 'git://git.kernel.org/...' where name = foo

;-) ;-) ;-) ???  If that is what you are after, then I agree
your syntax is more generic and suitable.  But otherwise I fail
to see its point.

On a related topic, I have always been torn about the "for"
convention.  While I think it was a cute hack, it would break
quite badly once we start doing anything complex.

	[branch]
        	url = git://git.kernel.org/... for jgarzik
                fetch = for-linus for jgarzik
                proxy = none

        	url = git://git.kernel.org/... for torvalds
                fetch = master for torvalds

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Mon, 8 May 2006, Junio C Hamano wrote:
> >
> > The problem with _that_ is that "git repo-config" can't add this kind of 
> > setup sanely: it doesn't understand that kind of statefulness.
> 
> Wait a minute...  Statefulness is not the issue, I think.

Well, it does end up being..

> How would you tell your updated repo-config what to update and
> what to look up?
> 
> 	- I want the url for branch whose name is "origin"
> 
> 	- I want to fetch their "for-linus" branch when fetching
>           from the branch whose name is "jgarzik" from now on.

Exactly, git repo-config would have to know about this magic thing, and 
have a special argument like

	--state=branch.name

that says that "state" is to be taken from the "branch.name" variable when 
seen.

Then, in addition to the regexp, you would have a way to trigger on the 
"state" variable.

> Now, how would that compare with:
> 
>         [branch.jgarzik]
>                 url = git://git.kernel.org/...
>                 fetch = for-linus
> 
> or
> 	[branch."JGarzik"]
>                 url = git://git.kernel.org/...
>                 fetch = for-linus

It would be _able_ to do all the same things, but thanks to statefulness 
you'd be able to keep the section (and key) names the way they are.

> On a related topic, I have always been torn about the "for"
> convention.

I agree. And I think it's actually very much the same thing. It adds 
state, but it adds it to each _value_, instead of adding it once "before" 
the values.

		Linus

Re: Implementing branch attributes in git config

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> On Mon, 8 May 2006, Junio C Hamano wrote:
>> 
>> Wait a minute...  Statefulness is not the issue, I think.
>
> Well, it does end up being..

Not really, you ended up making it so, perhaps, but I do not
think it needs to be.

You dodged my comments on the SQL-like queries ;-).  I was half
(perhaps 3/4) joking, but some people actually might like to be
able to say:

	git repo-config --get-all branch.name where url like 'git://%'

to list all the repositories reachable via git-native protocol.

Oops, by the way, why does a [branch] have url as its attribute?
I think this was a bad example -- we are talking about [remote]
here.  But the main point is about syntax, so that is OK.

> Exactly, git repo-config would have to know about this magic thing, and 
> have a special argument like
>
> 	--state=branch.name
>
> that says that "state" is to be taken from the "branch.name" variable when 
> seen.
>
> Then, in addition to the regexp, you would have a way to trigger on the 
> "state" variable.

I am reluctant to buy that argument (I see it is an easy way
from the implementation point of view) -- it appears to me that
it would invite this easy user error.


	[branch]
        	name = linus
        	url = git://git.kernel.org/../torvalds/linux-2.6

	[branch]
        	url = git://git.kernel.org/../jgarzik/libata-dev
        	name = libata

> It would be _able_ to do all the same things, but thanks to statefulness 
> you'd be able to keep the section (and key) names the way they are.

Yes, but that statefulness is inviting user errors, and you need
to update repo-config and config parser anyway, so I still do
not see what the advantage is.

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Mon, 8 May 2006, Junio C Hamano wrote:
> 
> You dodged my comments on the SQL-like queries ;-).  I was half
> (perhaps 3/4) joking, but some people actually might like to be
> able to say:
> 
> 	git repo-config --get-all branch.name where url like 'git://%'

I think databases tend to be a huge mistake. Show me a SQL database where 
users can edit the data by hand, and it's all readable, and maybe I'll 
change my mind.

The monotone guys have almost everything in a database, and from what I 
can tell it results in (a) you can do some really funky queries and (b) 
it's confusing and slow as hell.

Now, the speed issue doesn't matter for a config file, but there the 
editability very much does.

> Oops, by the way, why does a [branch] have url as its attribute?
> I think this was a bad example -- we are talking about [remote]
> here.  But the main point is about syntax, so that is OK.

Yeah, "remote" is clearly better than "url".

> I am reluctant to buy that argument (I see it is an easy way
> from the implementation point of view) -- it appears to me that
> it would invite this easy user error.
> 
> 	[branch]
>         	name = linus
>         	url = git://git.kernel.org/../torvalds/linux-2.6
> 
> 	[branch]
>         	url = git://git.kernel.org/../jgarzik/libata-dev
>         	name = libata

Yes, that would be a silent and confusing error.

		Linus

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Mon, 8 May 2006, Junio C Hamano wrote:
> 
> Yes, but that statefulness is inviting user errors, and you need
> to update repo-config and config parser anyway, so I still do
> not see what the advantage is.

Btw, I keep coming back to the same

	["jc/show-branch-dense"]
		remote = git://...

branch specifier syntax. It just seems very intuitive and is easy to 
parse. 

The only real downside ends up being the non-forwards-compatibility thing. 
But trying to be forwards-compatible for old git versions with this thing 
would seem to be a major pain for rather slim gain.

			Linus

Re: Implementing branch attributes in git config

[sean]

On Mon, 8 May 2006 18:57:08 -0700 (PDT)
Linus Torvalds <torvalds@osdl.org> wrote:

> Btw, I keep coming back to the same
> 
> 	["jc/show-branch-dense"]
> 		remote = git://...
> 
> branch specifier syntax. It just seems very intuitive and is easy to 
> parse. 

We already need such section headers for remotes, and for branches.. 
both which may well need to be quoted as above.. how to distinguish
between them?   How to handle the next case that comes along where we
want these special header semantics?

We really need:

   [branch.Whatever]

and:

   [remote.Whatever]

As in the case of "origin" where we have a remote and a branch
named that.


> The only real downside ends up being the non-forwards-compatibility thing. 
> But trying to be forwards-compatible for old git versions with this thing 
> would seem to be a major pain for rather slim gain.

What's the advantage of section quotation marks over just allowing these
characters in regular section names?  To be specific, what is wrong with:

   [jc/show-branch-dense]
       remote = git://...

If we just relax the legal characters in identifiers to include slash and dash?
It doesn't seem to be any different in amount of code needed to achieve, and it
is just as intuitive (perhaps more so) and no worse on the forward-compatibility
thing.

If we continue with case insensitive section names, it's quite possible for 
the user to mess up while hand editing or forgetting proper "git" quoting
rules on the command line and end up with silent breakage:

$ git repo-config "Branch".url = git://...  
        (updates section ["Branch"])

And then the next time forget the quotes and use:

$ git repo-config Branch.url = git://...    
         (updates section [branch])

And all of a sudden the user is updating _different_ sections with 
unpredictable results.  This is just awkward; why not just admit that 
section names should always be case sensitive if we're going to put
filenames inside them?

Sean

Re: Implementing branch attributes in git config

[sean]

On Mon, 8 May 2006 20:08:41 -0700 (PDT)
Linus Torvalds <torvalds@osdl.org> wrote:

> On Mon, 8 May 2006, sean wrote:
> > 
> > What's the advantage of section quotation marks over just allowing these
> > characters in regular section names?  To be specific, what is wrong with:
> > 
> >    [jc/show-branch-dense]
> 
> This would _suck_
> 
> What if you have a branch called "core"? Not all all unlikely. 
> 
> Think about what a section like
> 
> 	[core]
> 
> really means.


Yeah, but the part of my message you didn't quote made it quite clear i know
about this problem, what i would really propose is:

[core]
 ...
[branch.core]
 ...
[remote.core]
 ...

etc...
 
> Plus I really want to not be case sensitive by default. Case sensitivity 
> really is _not_ normal for this kind of config file syntax.

But it's not just the config file, it's also how it ends up being used
on the command line..  you have to admit silent differences between
these two command lines is _not_ desirable:

    $ git repo-config "Branch".url  
    $ git repo-config Branch.url

That can't be something you want to see either.

Sean

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Mon, 8 May 2006, sean wrote:
> 
> What's the advantage of section quotation marks over just allowing these
> characters in regular section names?  To be specific, what is wrong with:
> 
>    [jc/show-branch-dense]

This would _suck_

What if you have a branch called "core"? Not all all unlikely. 

Think about what a section like

	[core]

really means.

Plus I really want to not be case sensitive by default. Case sensitivity 
really is _not_ normal for this kind of config file syntax.

			Linus

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Mon, 8 May 2006, sean wrote:
>
> [core]
>  ...
> [branch.core]
>  ...
> [remote.core]
>  ...

Ok. In that case, I would suggest a much more readable format:

	[core]
		...

	[branch "core"]
		...

	[remote "core"]
		...

and yes, enforce the <space>+<quoted name> format. We'd turn it internally 
into some random internal string format (probably replacing the space with 
a dot, and removing the quotes, so it would become "remote.core.<key>").

> But it's not just the config file, it's also how it ends up being used
> on the command line.. 

Actually, the command line migth as well allow any strange thing, and 
_add_ the quotes internally. So you could do something that does

	git repo-config set Remote.Core.Pull master

and we could just let that generate

	[remote "Core"]
		pull = master

or whatever.

I care about the _file_ being human-editable, but that means that I think 
it's perfectly fine to have some smarts in the tools that help us do so, 
and let them recognize the magic "remote" and "branch" prefixes.

		Linus

Re: Implementing branch attributes in git config

[Pavel Roskin]

Hello!

I feel so bad that I sparked this discussion about config files and
couldn't participate in it in real time.  I'd like to summarize my
thoughts on the subject - maybe they will help us come to an agreement.

User convenience trumps backward compatibility.

Case in-sensitivity is almost a foreign concept for POSIX.  There is no
expectation (except among newbies) that bash would run grep if it's
asked to run Grep.  Why would git-repo-config need to foster such
expectations, and do so inconsistently, e.g. for key names but not for
values?

The config files use escaping by backslash, which is easier to work with
than quoting.  Quoting should be introduced if backslash escaping
doesn't work, and I think backslash escaping in fine.

Users who edit the config file manually and mindlessly get what they
deserve.  Users who misspell "master" as "Master" get what they deserve.
Occasional typos could be caught and reported if practical, but hand
holding shouldn't be a design goal.

Either we need the third layer in key hierarchy, and that layer should
support user defined strings, or we need to relax one of the layers to
user define strings.  User defined means that it can include spaces,
slashes, dots and many other characters.  Whenever a character is not
allowed, we should have a good reason.

An example of two-layer approach:

[branchdescriptions]
	master = My master branch
	netdev-master = Patches for netdev
[branchremotes]
	master = origin
	netdev-master = netdev

All other examples quoted here are examples of three-layer approach.
Either the extra key is inserted into the section name (Linus) or into
the value (Dscho).  It can also be inserted into the existing key.

If we want to group all branch properties for each branch, we have to go
three-layer.  If we don't want that, the above example should accepted
as the simplest approach.

Adding an additional key layer to the existing keys is syntactically
nice, but buys us very little in terms of ability to group branch data:

[branchdata]
	remote[master] = origin
	remote[netdev-master] = master
	description[master] = My master branch
	description[netdev-master] = Patches for netdev

Adding an additional key layer to the value is inherently fragile.  The
value has free format, and so is the new key.  It also has the same
problem that the data for different branches is mixed together.

Adding an additional key layer to the section name looks strange
syntactically, but it's the approach that gives us immediate grouping of
all branch data for every branch.

My personal preference is the Linus' proposal, but with backslash
escaping instead of quoting, with explicit "branch." in the section
names, and with case sensitive sections and keys.

-- 
Regards,
Pavel Roskin

Re: Implementing branch attributes in git config

[Jakub Narebski]

Linus Torvalds wrote:

> I would suggest a much more readable format:
> 
> [core]
> ...
> 
> [branch "core"]
> ...
> 
> [remote "core"]
> ...
> 
> and yes, enforce the <space>+<quoted name> format. We'd turn it internally
> into some random internal string format (probably replacing the space with
> a dot, and removing the quotes, so it would become "remote.core.<key>").

I'm all for it. Nice compromise of [branch."miXedCaps"] and ["miXedCaps"],
human readable end editable, and easy parsable.

-- 
Jakub Narebski
Warsaw, Poland

Re: Implementing branch attributes in git config

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

>> 	[branch]
>>         	name = linus
>>         	url = git://git.kernel.org/../torvalds/linux-2.6
>> 
>> 	[branch]
>>         	url = git://git.kernel.org/../jgarzik/libata-dev
>>         	name = libata
>
> Yes, that would be a silent and confusing error.

Although I haved raised objections, I actually started to like
the idea of using multiple [branch] (or wasn't it [remote] given
the example variables we have been using?) sections.

We should first depart from the Windoze .INI mindset.  While I
do not think users expect case insensisitivity, only because the
section headers are marked with [brackets], if that syntax
somehow makes people expect such, maybe we should stop using
[bracket] as section markers.

Whatever marker we end up using, I'd suggest somewhat different
approach.

 - Treat each part that are grouped under [bracketted-string]
   marker as a bag of variable=value pairs.  Loosely speaking,
   the bracketted-string defines a schema -- what kind of
   variables are expected to be there in that seciton.  For
   example, a section for things we traditionally had in remotes
   file would contain fields (variables) such as url, fetch, and
   push (we might add proxy to this list).  And we call this
   "bag of variable=value" a section.

 - There can be multiple sections in a config file that uses the
   same schema.  The example at the beginning of this message
   is perfectly valid.  It defines two sections of type
   [branch], each of which has two variables (name and url) in
   it.

Unlike your earlier suggestion, the second [branch] is not just
for readability; it is mandatory, because we are talking about
two different [branch]es (eh, that's [remote]s, really), it
needs to be there to separate two instances.

The above would break the existing repo-config command, but
let's forget about it for now.  I think we are breaking
forward/backward compatibility in any proposals brought up so
far anyway.

We would need user interface level commands to

	add a new section
        delete a section

We would need a way to identify a secion, perhaps using a value
of arbitrary key (e.g. "where name=blah").  Creating a section
could be implicit, just like the current repo-config.

        add a variable=value to a section
        delete a variable=value from a section
	retrieve variables' values from a section
	list value of a variable from all sections of a kind.

Probably need to support the same variable name appearing more
than once, just like the current multi-value support.

The current multi-value stuff assumes that multi-values are
exceptions, and rare.  While I do not necessarily agree with
that, for now let's assume that is true.

Creating a new section with given variables:

	$ cfg --set section var value var value ...
   (eg) $ cfg --set branch name master merge origin pull linus

    Here, 'var' and 'value' are case sensitive; if they have
    syntactical metacharacters (WS, =, quotes, etc), they need
    to be quoted when cfg command writes to the file (i.e. the
    user do not have to quote more than necessary to protect
    them from the shell).

Updating an existing section's variables, or create a new one:

	$ cfg --replace section.var value where var0 = val0
   (eg) $ cfg --replace remote.url git://... where name = linus

    Look for a "remote" section that has a variable "name" with
    value "linus" in it, and replace its "url" variable with
    "git://...".  If there is no "remote" section with such a
    name, create it.  For the key matching syntax, I do not
    insist on using "where" (I merely used it for continuity
    from the previous discussion).  For the comparison operator,
    in addition to the '=' shown above, we would probably want
    to have regexp match (perhaps ':' to emulate "expr") as well.

Retrieving a variable:

	$ cfg --get section.var [where var0 = val0]
   (eg) $ cfg --get remote.url where name = linus

List sections:

	$ cfg --list section.var
   (eg) $ cfg --list remote.name

So, an equivalent of "grep -H URL: .git/remotes/*" becomes something like:

	for name in `cfg --list remote.name`
        do
             	url=`cfg --get remote.url where name = "$name"`
                echo "$name: URL: $url"
	done

Re: Implementing branch attributes in git config

[Johannes Schindelin]

Hi,

On Tue, 9 May 2006, Jakub Narebski wrote:

> Linus Torvalds wrote:
> 
> > I would suggest a much more readable format:
> > 
> > [core]
> > ...
> > 
> > [branch "core"]
> > ...
> > 
> > [remote "core"]
> > ...
> > 
> > and yes, enforce the <space>+<quoted name> format. We'd turn it internally
> > into some random internal string format (probably replacing the space with
> > a dot, and removing the quotes, so it would become "remote.core.<key>").
> 
> I'm all for it. Nice compromise of [branch."miXedCaps"] and ["miXedCaps"],
> human readable end editable, and easy parsable.

Okay, to summarize what people proposed (and that I remember):

1) [branch."AnY+String"]

2) multiple [branch]

3) magic <space>+<quoted>

4) [branch.just/allow-slashes/and-dashes]

5) the " for " notation

Of all these, only (5) is backwards compatible, but it is also the only 
one where you have to type the branch name over and over again.

However, the old gits do not really know what to do with the [branch] 
section anyway, so you could consider (2) (and (4), if you do not have 
branch names with slashes and/or dashes) backwards-compatible, because git 
will continue to work -- ignoring the funny entries.

(1) and (3) definitely would make an old git choke.

Now, for the ease of use:

(1), (3) and (4) are in the same league of easiness (except maybe that you 
have to keep in mind to extra-quote in shell scripts with (1) and (3)), 
(2) is especially good for people with a database mindset, and (5) is 
annoying as hell.

Now, for the ease of implementation:

(1) and (3) are in the same league, they have to change the way the config 
is parsed as well as make downcasing conditional in repo-config. (2) is 
obviously hardest of all. (4) is very easy (one line in config.c), and (5) 
easiest (nothing to do).

Now, for the versatility, i.e. what you can express with the syntax:

The same for all (except for (4) which has very weak restrictions on the 
branch name).

Oh, I completely forgot about another proposal: (6) subkeys (something 
like "url[branchname] = blablabla"). It has about the same effects as (1) 
or (3).

Another thing: I completely ignored the case sensitivity. Because it is 
irrelevant. Why? Because you do not have two branches which are only 
different by case-ness. It is confusing, and that's why. And you don't 
need to handle the case specially, because the comparison is done by 
downcasing anyway.

Obviously, I deem (4) the best solution ATM, because it has all the 
expressability needed, while being the simplest.

Ciao,
Dscho

Re: Implementing branch attributes in git config

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 816 bytes --]

hoi :)

On Mon, May 08, 2006 at 08:08:41PM -0700, Linus Torvalds wrote:
> > What's the advantage of section quotation marks over just allowing these
> > characters in regular section names?  To be specific, what is wrong with:
> > 
> >    [jc/show-branch-dense]
> 
> This would _suck_
> 
> What if you have a branch called "core"? Not all all unlikely. 
> 
> Think about what a section like
> 
> 	[core]
> 
> really means.
> 
> Plus I really want to not be case sensitive by default. Case sensitivity 
> really is _not_ normal for this kind of config file syntax.

So why is everybody trying to munch all branch related data into
one .ini style config file?

why not simply use the mechanisms we use elsewhere and build something
like our remotes or the new HEAD file?

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Implementing branch attributes in git config

[Johannes Schindelin]

Hi,

On Tue, 9 May 2006, Martin Waitz wrote:

> So why is everybody trying to munch all branch related data into
> one .ini style config file?
> 
> why not simply use the mechanisms we use elsewhere and build something
> like our remotes or the new HEAD file?

Because it is good to have one consistent tool to query/update what makes 
up the configuration. Of course, we really could go about it like M$ who 
invent a hundred an twenty three ways to do the same thing, all with their 
own set of bugs.

I admit that repo-config has not been as stable as it could have been. 
That was my fault, certainly. But with the help of the list, it has become 
more stable.

Now, if we decide upon a totally different config format, okay, that's 
what it takes. But please let's not have several different formats 
*again*.

Ciao,
Dscho

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Tue, 9 May 2006, Johannes Schindelin wrote:
> 
> Okay, to summarize what people proposed (and that I remember):
> 
> 1) [branch."AnY+String"]

If we really change the syntax, I would oppose the ".". I realize I may 
have used it myself, and I think it would be good _internally_, but I 
think the syntax would be

	[branch "Any+String"]

which looks a lot more readable. Ie just a space (or, perhaps, "any 
combination of spaces and tabs").

> 4) [branch.just/allow-slashes/and-dashes]

Same here. If we break old parsers anyway, the "." has no redeeming 
value. You'd use it when _scripting_ stuff, but not anywhere else.

So in both cases, the above would turn into the _variable_ called 
"branch.Any+String/and-dashes.<key>" internally, and things that used "git 
repo-config" would say it that way, but the config file format should be 
human-readable.

We already do that human-readability thing. We say

	[core]
		name = Linus  Torvalds
		email = random

but we turn it internally into

	core.name  ->	"Linus Torvalds"
	core.email ->	"random"

and that's also the format you use for "git repo-config". Similarly, 
having

	[branch "master"]
		remote = git://....

in the config file should - for exactly the same reasons - be turned 
_internally_ into the association

	branch.master.remote    ->    git://...

and for exactly the same reason you'd just use

	git repo-config set branch.master.remote "git://..."

on the command line.

IOW, we _already_ do not match the internal and command line with the 
actal human-readable syntax.

			Linus

Re: Implementing branch attributes in git config

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Okay, to summarize what people proposed (and that I remember):
>
> 1) [branch."AnY+String"]
>
> 2) multiple [branch]
>
> 3) magic <space>+<quoted>
>
> 4) [branch.just/allow-slashes/and-dashes]
>
> 5) the " for " notation

Thanks for a nice summary.

> Now, for the ease of use:
>
> (1), (3) and (4) are in the same league of easiness (except maybe that you 
> have to keep in mind to extra-quote in shell scripts with (1) and (3)), 
> (2) is especially good for people with a database mindset, and (5) is 
> annoying as hell.

One thing you might want to consider is variable types and
default values (eh, that makes two).

When Linus first introduced the config mechanism, he made it so
that a loosely coupled set of programs can take the "Why should
I care about other programs configuration" attitude, and
actively encouraged to do so by allowing custom config parsers
inherit from the default parser, like the way git_diff_config()
falls back on git_default_config() when it does not recognize
the variable.

It is quite a good design for most uses, except that it made it
inconvenient to implement things like "git-repo-config -l" and
"git-var -l".  The point of this design _is_ that they cannot
know what the set of possible variables, their types and the
default values when missing are, so by design the script that
used "git-var -l | grep" to read the configuration needed to
know that a boolean can be denoted by existence, value set to
zero or non-zero integer, or string "true"/"false" (i.e.
"filemode", "filemode=1", and "filemode = true" mean the same
thing; BTW I think we would probably want to add "yes"/"no"
here).

Later it was made easier to use by Pasky with "repo-config --type"
option.  The caller supplies the name of the variable and the
type and repo-config gets the value -- the caller knows what it
wants to use, so having it to know what type of things it is
interested in is not so bad, so the type problem was practically
solved.  But it still feels somewhat hacky.

With (2) and (5), we have a bound set of "se.ct.ion.variable";
we could enumerate the variables we care about, define what they
mean and what their types and default values are (we need to do
that for Documentation/config.txt anyway).  With many parts of
the standalone git plumbing programs migrating into builtins, I
think it is not a bad idea to have a central table of all the
configuration variables that the core knows about.  Porcelains
and scripts could define customized tables that describe the
sections/variables they also want to see and act on in the
configuration file, and call git-repo-config with that table as
an optional parameter.

> Now, for the ease of implementation:
>
> (1) and (3) are in the same league, they have to change the way the config 
> is parsed as well as make downcasing conditional in repo-config. (2) is 
> obviously hardest of all. (4) is very easy (one line in config.c), and (5) 
> easiest (nothing to do).
>
> Now, for the versatility, i.e. what you can express with the syntax:
>...
> Obviously, I deem (4) the best solution ATM, because it has all the 
> expressability needed, while being the simplest.

If we are shooting for "let's not do this again", I do not think
(4) without some quoting convention is good enough.  Today, we
are talking about branch names so we could give them artificial
limits, which could be weaker than what we already have on the
branch names, but we would later regret that, when we start
wanting to have other names in the configuration (e.g. people's
names).

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Tue, 9 May 2006, Junio C Hamano wrote:
> 
> If we are shooting for "let's not do this again", I do not think
> (4) without some quoting convention is good enough.  Today, we
> are talking about branch names so we could give them artificial
> limits, which could be weaker than what we already have on the
> branch names, but we would later regret that, when we start
> wanting to have other names in the configuration (e.g. people's
> names).

Here's my suggestion as a patch.

NOTE! This patch could be applied right now, and to all the branches (to
make 1.x, 1.2.x and 1.3.x all support the _syntax_). Even if nothing 
actually uses it.

It just makes the syntax be

	[section<space>+"<randomstring>"]

where the only rule for "randomstring" is that it can't contain a newline, 
and if you really want to insert a double-quote, you do it with \".

It turns that into the section name "secion.randomstring".

So you could use this for things like

	[email "torvalds@osdl.org"]
		name = Linus Torvalds

if you wanted to do the "email->name" conversion as part of the config 
file format (I'm not claiming that is sensible, I'm just giving it as an 
insane example). That would show up as the association

	email.torvalds@osdl.org.name -> Linus Torvalds

which is easy to parse (the "." in the email _looks_ ambiguous, but it 
isn't: you know that there will always be a single key-name, so you find 
the key name with "strrchr(name, '.')" and things are entirely 
unambiguous).

Now, it doesn't do any repo-config stuff, since the whole point of this is 
to make this a legal syntax, not actually start _using_ it yet. If people 
think this is an acceptable syntax, then pls apply to everything, and 
worry about usage later.

		Linus

---
diff --git a/config.c b/config.c
index 0f518c9..f3b74e0 100644
--- a/config.c
+++ b/config.c
@@ -134,6 +134,41 @@ static int get_value(config_fn_t fn, cha
 	return fn(name, value);
 }
 
+static int get_extended_base_var(char *name, int baselen, int c)
+{
+	do {
+		if (c == '\n')
+			return -1;
+		c = get_next_char();
+	} while (isspace(c));
+
+	/* We require the format to be '[base "extension"]' */
+	if (c != '"')
+		return -1;
+	name[baselen++] = '.';
+
+	for (;;) {
+		int c = get_next_char();
+		if (c == '\n')
+			return -1;
+		if (c == '"')
+			break;
+		if (c == '\\') {
+			c = get_next_char();
+			if (c == '\n')
+				return -1;
+		}
+		name[baselen++] = c;
+		if (baselen > MAXNAME / 2)
+			return -1;
+	}
+
+	/* Final ']' */
+	if (get_next_char() != ']')
+		return -1;
+	return baselen;
+}
+
 static int get_base_var(char *name)
 {
 	int baselen = 0;
@@ -144,6 +179,8 @@ static int get_base_var(char *name)
 			return -1;
 		if (c == ']')
 			return baselen;
+		if (isspace(c))
+			return get_extended_base_var(name, baselen, c);
 		if (!isalnum(c) && c != '.')
 			return -1;
 		if (baselen > MAXNAME / 2)

Re: Implementing branch attributes in git config

[sean]

On Tue, 9 May 2006 12:24:02 -0700 (PDT)
Linus Torvalds <torvalds@osdl.org> wrote:

> NOTE! This patch could be applied right now, and to all the branches (to
> make 1.x, 1.2.x and 1.3.x all support the _syntax_). Even if nothing 
> actually uses it.
> 
> It just makes the syntax be
> 
> 	[section<space>+"<randomstring>"]
> 
> where the only rule for "randomstring" is that it can't contain a newline, 
> and if you really want to insert a double-quote, you do it with \".

Lightly tested here.  Looks good.

Sean

Re: Implementing branch attributes in git config

[sean]

On Tue, 9 May 2006 15:44:59 -0400
sean <seanlkml@sympatico.ca> wrote:

> On Tue, 9 May 2006 12:24:02 -0700 (PDT)
> Linus Torvalds <torvalds@osdl.org> wrote:
> 
> > NOTE! This patch could be applied right now, and to all the branches (to
> > make 1.x, 1.2.x and 1.3.x all support the _syntax_). Even if nothing 
> > actually uses it.
> > 
> > It just makes the syntax be
> > 
> > 	[section<space>+"<randomstring>"]
> > 
> > where the only rule for "randomstring" is that it can't contain a newline, 
> > and if you really want to insert a double-quote, you do it with \".
> 
> Lightly tested here.  Looks good.
> 

Linus,

I really tried to like your patch ;o)  But it breaks the repo-config command
line and causes mixing of some branches using old style [branch.xyz] and new
style [branch "XYZ"] which just doesn't seem to be the right thing to do.

The following patch produced for sake of discussion just allows section
headers to contain whatever characters are needed to get the job done.

git-repo-config works properly with no further need of change.  Section
headers become case sensitive but key identifiers within sections do not.
AFAIK there should be no backward compatibility issues with regard to
case sensitivity.  All tests pass after having been fixed up to remove
the assumption that section names are case insensitive.

The syntax is:

  [<random string>]

Here's how your example would look in this style:

  [email.torvalds@osdl.org]
        name = Linus Torvalds

And there's no strange syntax needed with repo-config to set and get it:

  $ git repo-config email.torvalds@osdl.org.name
  Linus Torvalds

and just to show that key names are still case insensitive:

  $ git repo-config email.torvalds@osdl.org.NAME
  Linus Torvalds

Setting new sections is unambiguous from the command line and
doesn't have to decide whether to use the [branch "<string>"] or
[branch.section.name] format.

  $ git repo-config branch.branch.x y
  $ git repo-config branch.WonkKY.x y
  $ git repo-config --get-regexp branch.\*
  branch.branch.x y
  branch.WonkKY.x y

  [email.torvalds@osdl.org]
        name = Linus Torvalds
  [branch.branch]
        x = y
  [branch.WonkKY]
        x = y

Sean
---

 config.c               |   11 +++++++----
 repo-config.c          |    8 ++++----
 t/t1300-repo-config.sh |   38 ++++++++++++++++++++++----------------
 3 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/config.c b/config.c
index 0f518c9..5d19ae9 100644
--- a/config.c
+++ b/config.c
@@ -144,11 +144,14 @@ static int get_base_var(char *name)
 			return -1;
 		if (c == ']')
 			return baselen;
-		if (!isalnum(c) && c != '.')
-			return -1;
+		if (c == '\\') {
+			c = get_next_char();
+			if (c == '\n')
+				return -1;
+		}
 		if (baselen > MAXNAME / 2)
 			return -1;
-		name[baselen++] = tolower(c);
+		name[baselen++] = c;
 	}
 }
 
@@ -455,7 +458,7 @@ int git_config_set_multivar(const char* 
 			ret = 1;
 			goto out_free;
 		} else
-			store.key[i] = tolower(key[i]);
+			store.key[i] = key[i];
 	store.key[i] = 0;
 
 	/*
diff --git a/repo-config.c b/repo-config.c
index 63eda1b..ba5fbd6 100644
--- a/repo-config.c
+++ b/repo-config.c
@@ -65,11 +65,11 @@ static int show_config(const char* key_,
 static int get_value(const char* key_, const char* regex_)
 {
 	int i;
+	char *tl;
 
-	key = malloc(strlen(key_)+1);
-	for (i = 0; key_[i]; i++)
-		key[i] = tolower(key_[i]);
-	key[i] = 0;
+	key = strdup(key_);
+	for (tl=key+strlen(key)-1; tl >= key && *tl != '.'; --tl)
+		*tl = tolower(*tl);
 
 	if (use_key_regexp) {
 		key_regexp = (regex_t*)malloc(sizeof(regex_t));
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 7090ea9..f341206 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -23,6 +23,7 @@ git-repo-config Core.Movie BadPhysics
 cat > expect << EOF
 [core]
 	penguin = little blue
+[Core]
 	Movie = BadPhysics
 EOF
 
@@ -33,6 +34,7 @@ git-repo-config Cores.WhatEver Second
 cat > expect << EOF
 [core]
 	penguin = little blue
+[Core]
 	Movie = BadPhysics
 [Cores]
 	WhatEver = Second
@@ -45,10 +47,12 @@ git-repo-config CORE.UPPERCASE true
 cat > expect << EOF
 [core]
 	penguin = little blue
+[Core]
 	Movie = BadPhysics
-	UPPERCASE = true
 [Cores]
 	WhatEver = Second
+[CORE]
+	UPPERCASE = true
 EOF
 
 test_expect_success 'similar section' 'cmp .git/config expect'
@@ -62,11 +66,13 @@ test_expect_success 'replace with non-ma
 cat > expect << EOF
 [core]
 	penguin = very blue
-	Movie = BadPhysics
-	UPPERCASE = true
 	penguin = kingpin
+[Core]
+	Movie = BadPhysics
 [Cores]
 	WhatEver = Second
+[CORE]
+	UPPERCASE = true
 EOF
 
 test_expect_success 'non-match result' 'cmp .git/config expect'
@@ -130,7 +136,7 @@ EOF
 
 test_expect_success 'really mean test' 'cmp .git/config expect'
 
-git-repo-config nextsection.nonewline wow
+git-repo-config nextSection.nonewline wow
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -160,7 +166,7 @@ EOF
 
 test_expect_success 'unset' 'cmp .git/config expect'
 
-git-repo-config nextsection.NoNewLine "wow2 for me" "for me$"
+git-repo-config nextSection.NoNewLine "wow2 for me" "for me$"
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -176,18 +182,18 @@ EOF
 test_expect_success 'multivar' 'cmp .git/config expect'
 
 test_expect_success 'non-match' \
-	'git-repo-config --get nextsection.nonewline !for'
+	'git-repo-config --get nextSection.nonewline !for'
 
 test_expect_success 'non-match value' \
-	'test wow = $(git-repo-config --get nextsection.nonewline !for)'
+	'test wow = $(git-repo-config --get nextSection.nonewline !for)'
 
 test_expect_failure 'ambiguous get' \
-	'git-repo-config --get nextsection.nonewline'
+	'git-repo-config --get nextSection.nonewline'
 
 test_expect_success 'get multivar' \
-	'git-repo-config --get-all nextsection.nonewline'
+	'git-repo-config --get-all nextSection.nonewline'
 
-git-repo-config nextsection.nonewline "wow3" "wow$"
+git-repo-config nextSection.nonewline "wow3" "wow$"
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -202,15 +208,15 @@ EOF
 
 test_expect_success 'multivar replace' 'cmp .git/config expect'
 
-test_expect_failure 'ambiguous value' 'git-repo-config nextsection.nonewline'
+test_expect_failure 'ambiguous value' 'git-repo-config nextSection.nonewline'
 
 test_expect_failure 'ambiguous unset' \
-	'git-repo-config --unset nextsection.nonewline'
+	'git-repo-config --unset nextSection.nonewline'
 
 test_expect_failure 'invalid unset' \
-	'git-repo-config --unset somesection.nonewline'
+	'git-repo-config --unset someSection.nonewline'
 
-git-repo-config --unset nextsection.nonewline "wow3$"
+git-repo-config --unset nextSection.nonewline "wow3$"
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -249,7 +255,7 @@ test_expect_success 'hierarchical sectio
 
 cat > expect << EOF
 beta.noindent=sillyValue
-nextsection.nonewline=wow2 for me
+nextSection.nonewline=wow2 for me
 123456.a123=987
 1.2.3.alpha=beta
 EOF
@@ -259,7 +265,7 @@ test_expect_success 'working --list' \
 
 cat > expect << EOF
 beta.noindent sillyValue
-nextsection.nonewline wow2 for me
+nextSection.nonewline wow2 for me
 EOF
 
 test_expect_success '--get-regexp' \

Re: Implementing branch attributes in git config

[Junio C Hamano]

sean <seanlkml@sympatico.ca> writes:

> The syntax is:
>
>   [<random string>]

Does this mean you can have anything other than LF and ']'?

> Here's how your example would look in this style:
>
>   [email.torvalds@osdl.org]
>         name = Linus Torvalds

How does a program (not a script, but git_config() users) get
that value and parse it?  

Re: Implementing branch attributes in git config

[sean]

On Tue, 09 May 2006 15:42:25 -0700
Junio C Hamano <junkio@cox.net> wrote:

> sean <seanlkml@sympatico.ca> writes:
> 
> > The syntax is:
> >
> >   [<random string>]
> 
> Does this mean you can have anything other than LF and ']'?

Anything but LF; how's this for ugly:

    ["hello Worl\]d \\backslash]

> > Here's how your example would look in this style:
> >
> >   [email.torvalds@osdl.org]
> >         name = Linus Torvalds
> 
> How does a program (not a script, but git_config() users) get
> that value and parse it?  

The same way they do now.  For instance git-repo-config processes
the config file using the same get_config() + callback as usual.  The
only issue is that they should no longer cast everything to lower first.

Sean

Re: Implementing branch attributes in git config

[sean]

On Tue, 9 May 2006 18:45:19 -0400
sean <seanlkml@sympatico.ca> wrote:

> > How does a program (not a script, but git_config() users) get
> > that value and parse it?  
> 
> The same way they do now.  For instance git-repo-config processes
> the config file using the same get_config() + callback as usual.  The
> only issue is that they should no longer cast everything to lower first.

Junio,

Sorry I see what you're driving at; how does a program break the section
name into it's constituent pieces.  I glossed over this issue because
it's exactly the same between Linus' proposal and mine.   The answer is,
they really can't, with either proposal.

All you can count on (by convention) is that there is an initial segment
that is terminated by a period; and a final segment that starts with a
period, and everything in between is an opaque unit.

    section.<random string>.keyname   value

Although the initial "section." isn't currently enforced (but easily
could be).  Actually i didn't yank out the bit from config.c that
validates the keyname, so without an additional patch the only way
to enter the extended names is by manual editing of the .git/config.

Sean

Re: Implementing branch attributes in git config

[Pavel Roskin]

On Tue, 2006-05-09 at 18:45 -0400, sean wrote:
> On Tue, 09 May 2006 15:42:25 -0700
> Junio C Hamano <junkio@cox.net> wrote:
> > Does this mean you can have anything other than LF and ']'?
> 
> Anything but LF; how's this for ugly:
> 
>     ["hello Worl\]d \\backslash]

Actually, LF is already handled just fine in the value part:

[proski@dv .git]$ git-repo-config s1.k1 $'v1\nv2'
[proski@dv .git]$ grep [sk]1 config 
[s1]
        k1 = v1\nv2

Note that quoting doesn't solve this problem (unless multi-line section
headers are allowed), but backslash escaping does.

But I guess everybody prefers quotes for their friendly look.

-- 
Regards,
Pavel Roskin

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Tue, 9 May 2006, sean wrote:
>
> I really tried to like your patch ;o)  But it breaks the repo-config command
> line and causes mixing of some branches using old style [branch.xyz] and new
> style [branch "XYZ"] which just doesn't seem to be the right thing to do.

Well, obviously it's _simpler_ to have the machine-readable format as-is, 
and say "don't try to make it human-readable". But the repo-config patch 
to make it human-readable isn't that hard.

And it's _not_ that hard to make repo-config do the right thing.

Here's a pretty lightly tested patch (on top of my previous one) that does 
exactly that.

(The first part just fixes an indentation bug)

		Linus
---
diff --git a/config.c b/config.c
index f3b74e0..12c51b1 100644
--- a/config.c
+++ b/config.c
@@ -372,10 +372,12 @@ static int store_aux(const char* key, co
 			store.offset[store.seen] = ftell(config_file);
 			store.state = KEY_SEEN;
 			store.seen++;
-		} else if (strrchr(key, '.') - key == store.baselen &&
+		} else {
+			if (strrchr(key, '.') - key == store.baselen &&
 			      !strncmp(key, store.key, store.baselen)) {
 					store.state = SECTION_SEEN;
 					store.offset[store.seen] = ftell(config_file);
+			}
 		}
 	}
 	return 0;
@@ -383,8 +385,30 @@ static int store_aux(const char* key, co
 
 static void store_write_section(int fd, const char* key)
 {
+	const char *dot = strchr(key, '.');
+	int len1 = store.baselen, len2 = -1;
+
+	dot = strchr(key, '.');
+	if (dot) {
+		int dotlen = dot - key;
+		if (dotlen < len1) {
+			len2 = len1 - dotlen - 1;
+			len1 = dotlen;
+		}
+	}
+			
 	write(fd, "[", 1);
-	write(fd, key, store.baselen);
+	write(fd, key, len1);
+	if (len2 >= 0) {
+		write(fd, " \"", 2);
+		while (--len2 >= 0) {
+			unsigned char c = *++dot;
+			if (c == '"')
+				write(fd, "\\", 1);
+			write(fd, &c, 1);
+		}
+		write(fd, "\"", 1);
+	}
 	write(fd, "]\n", 2);
 }
 
@@ -458,7 +482,7 @@ int git_config_set(const char* key, cons
 int git_config_set_multivar(const char* key, const char* value,
 	const char* value_regex, int multi_replace)
 {
-	int i;
+	int i, dot;
 	int fd = -1, in_fd;
 	int ret;
 	char* config_filename = strdup(git_path("config"));
@@ -483,16 +507,23 @@ int git_config_set_multivar(const char* 
 	 * Validate the key and while at it, lower case it for matching.
 	 */
 	store.key = (char*)malloc(strlen(key)+1);
-	for (i = 0; key[i]; i++)
-		if (i != store.baselen &&
-				((!isalnum(key[i]) && key[i] != '.') ||
-				 (i == store.baselen+1 && !isalpha(key[i])))) {
-			fprintf(stderr, "invalid key: %s\n", key);
-			free(store.key);
-			ret = 1;
-			goto out_free;
-		} else
-			store.key[i] = tolower(key[i]);
+	dot = 0;
+	for (i = 0; key[i]; i++) {
+		unsigned char c = key[i];
+		if (c == '.')
+			dot = 1;
+		/* Leave the extended basename untouched.. */
+		if (!dot || i > store.baselen) {
+			if (!isalnum(c) || (i == store.baselen+1 && !isalpha(c))) {
+				fprintf(stderr, "invalid key: %s\n", key);
+				free(store.key);
+				ret = 1;
+				goto out_free;
+			}
+			c = tolower(c);
+		}
+		store.key[i] = c;
+	}
 	store.key[i] = 0;
 
 	/*

Re: Implementing branch attributes in git config

[sean]

On Tue, 9 May 2006 17:17:58 -0700 (PDT)
Linus Torvalds <torvalds@osdl.org> wrote:

> And it's _not_ that hard to make repo-config do the right thing.
> 
> Here's a pretty lightly tested patch (on top of my previous one) that does 
> exactly that.

So every mutli-part section is going to be of the form:

  [section "big long opaque string"]

It seems to handle everything, you have me convinced.
 
Sean

Re: Implementing branch attributes in git config

[sean]

On Tue, 9 May 2006 17:17:58 -0700 (PDT)
Linus Torvalds <torvalds@osdl.org> wrote:

> Here's a pretty lightly tested patch (on top of my previous one) that does 
> exactly that.

This patch or something like it is needed for repo-config in order to query
the new case sensitive portion of section names.   This is on top of your two
patches:

diff --git a/repo-config.c b/repo-config.c
index 63eda1b..9a9194f 100644
--- a/repo-config.c
+++ b/repo-config.c
@@ -65,11 +65,14 @@ static int show_config(const char* key_,
 static int get_value(const char* key_, const char* regex_)
 {
 	int i;
+	char *tl;
+
+	key = strdup(key_);
+	for (tl=key+strlen(key)-1; tl >= key && *tl != '.'; --tl)
+		*tl = tolower(*tl);
+	for (tl=key; *tl && *tl != '.'; ++tl)
+		*tl = tolower(*tl);
 
-	key = malloc(strlen(key_)+1);
-	for (i = 0; key_[i]; i++)
-		key[i] = tolower(key_[i]);
-	key[i] = 0;
 
 	if (use_key_regexp) {
 		key_regexp = (regex_t*)malloc(sizeof(regex_t));

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Tue, 9 May 2006, sean wrote:
> On Tue, 9 May 2006 17:17:58 -0700 (PDT)
> Linus Torvalds <torvalds@osdl.org> wrote:
> 
> > And it's _not_ that hard to make repo-config do the right thing.
> > 
> > Here's a pretty lightly tested patch (on top of my previous one) that does 
> > exactly that.
> 
> So every mutli-part section is going to be of the form:
> 
>   [section "big long opaque string"]

That's what my stupid patch does now. It seems to work well for all cases, 
but if we were to care, we could have some special heuristics for 
different section names (ie "if subsection is all lower-case 
alphanumerics, and the section name is one of the following, use the 
old-fashioned format").

I don't see _why_ we'd ever do that, but we certainly _could_, if it were 
to make more sense that way for some section name.

However, if you already use a syntax like

	[section.subsection]
		key = 1

and then do

	git-repo-config --replace-all section.subsection.new 2

it will actually keep the old section header, so you'll end up with

	[section.subsection]
		key = 1
		new = 2

but if you create a _new_ subsection (and since subsections are now case 
sensitive, this example is a "new" subsection):

	git-repo-config --replace-all section.SubSection.new 3

you will now have

	[section.subsection]
		key = 1
		new = 2
	[section "SubSection"]
		new = 3

(ie notice how it did _not_ replace the old "section.subsection.new", 
because of how this is a _different_ subsection due to the subsectin 
rules, and notice how it will always create the new subsection with 
quotes).

So you _can_ continue to use the old subsection format, and it will work 
the way it always did, except for the fact that it would now be deprecated 
(if there were any multi-level users, which I don't think there are), and 
it is now case-sensitive (which makes sense in the new format with "" 
around it, but is illogical in the old deprecated one).

> It seems to handle everything, you have me convinced.

Hey, it was fun, and the only ugly part was the write-out of the quoted 
format.

And it should be perfectly easy to use. Modulo double-quotes in branch 
names, you can do trivial things like

	git repo-config "branch.$branchname.remote" "git://git.kernel.org/..."

and it will do the obvious thing.

My one complaint is that I think we should add an empty line for the case 
where we add a new sub-section to the end of a file. That's not a new 
problem, but that was really the only visually ugly part I noticed during 
testing.

You _can_ be user-friendly and machine-parseable at the same time!

			Linus

Re: Implementing branch attributes in git config

[Martin Langhoff]

On 5/10/06, Linus Torvalds <torvalds@osdl.org> wrote:
> You _can_ be user-friendly and machine-parseable at the same time!

Good one. I'm following this thread with interest, but it feels we've
been attacked by the 'bike shed bug' in the act of redesigning
Windows.ini.

As an end-user, I have personally stayed away from the increasingly
complex scheme for remotes waiting for it to settle, and stuck with
the old-styled .git/branches stuff which is slam-dunk simple and it
just works.

The normal non-branch config options don't need any of this fancy
stuff. And I think the branches is reasonably well managed as files as
is done in .git/remotes which is trivial to work on with standard
shell commands. What I mean is that I can grep them trivially to ask
"how many remotes pull from server X" or from repo Y. Or via rsync.

Also -- repo config is tricky in the sense of scope. I want all my
"dev" repos of different projects on my laptop to have mostly the same
config but radically different remotes listings.

So... call me old-styled... but I'm happy with one-file-per-remote.
Was it broken to start with? Should we restart the track renames
flameway instead?

cheers,



martin

Re: Implementing branch attributes in git config

[Johannes Schindelin]

Hi,

On Wed, 10 May 2006, Martin Langhoff wrote:

> So... call me old-styled... but I'm happy with one-file-per-remote.
> Was it broken to start with?

Depends on how you look at it. A bunch of files is okay for quick-n-dirty, 
where you do not care about locking or consistency or extensibility.

Ciao,
Dscho

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Wed, 10 May 2006, Martin Langhoff wrote:
> 
> Good one. I'm following this thread with interest, but it feels we've
> been attacked by the 'bike shed bug' in the act of redesigning
> Windows.ini.

Sure. It clearly _is_ a bike shed, which is why I posted patches: I think 
the way to resolve bike sheds is to "just do it". In the kernel, the 
general rule ends up being "he who writes the code gets to choose how it 
gets done", because it turns out that there are a lot more people willing 
to _argue_ about code than there are people willing to _write_ it, and 
thus that "real code wins" rule actually works very well in practice.

I don't think I've ever really seen an argument where you ended up having 
seriously competing patches. Yes, you can have patches to do things 
different ways, but once you have that, it tends to be pretty easy for the 
maintainer to just draw a line in the sand. And once one patch has been 
accepted, it's all over.

So the real problem with "bike sheds" is actually when you have a culture 
of arguing, and not enough of a culture of "just do it".

If you have a "just do it" culture, bike sheds are often good things. If 
it really _is_ that easy, a bike shed is a perfect opportunity for 
somebody who isn't all that deeply into a project to make a contribution 
and start feeling more comfy about it. It obviously didn't happen here, 
but it's definitely true that a lot of people in the kernel get introduced 
to doing patches through various "trivial" things.

So don't knock the bike sheds. It's a BSD term, and I think there's a 
_reason_ why it's a BSD term. Those people seem to sometimes like to argue 
about theoretical things (or about anything else, for that matter) more 
than actually getting the damn job done.

> As an end-user, I have personally stayed away from the increasingly
> complex scheme for remotes waiting for it to settle, and stuck with
> the old-styled .git/branches stuff which is slam-dunk simple and it
> just works.

It does work, and I think it's nice in many ways. It was certainly a good 
way to prototype things.

At the same time, especially with moving things more to C (or almost any 
other language, for that matter - shell is really pretty special in making 
it _easier_ to have things in individual files), it's in many ways nicer 
to have a more structured representation that has a nice fixed interface 
and a library and known access methods behind it.

And I'm personally actually pretty fed up with the .git/branches/ and 
.git/remotes/ thing, partly because I can never remember which file is 
which. I had to look at the code of git-parse-remote.sh to remind me which 
had what semantics. We could remove the old one entirely, of course (and 
no, I don't remember which is which now either), and avoid that particular 
problem, but it kind of soured me on it. 

And if we truly have separate files, we should go all the way, and have 
the good old "one file, one value" rule. Which we don't, and which I think 
everybody admits would be horrible for this case for users (even if it 
might be very nice for scripting).

		Linus

Re: Implementing branch attributes in git config

[Jakub Narebski]

Linus Torvalds wrote:


> On Wed, 10 May 2006, Martin Langhoff wrote:
>> 
>> Good one. I'm following this thread with interest, but it feels we've
>> been attacked by the 'bike shed bug' in the act of redesigning
>> Windows.ini.
[...]
>> As an end-user, I have personally stayed away from the increasingly
>> complex scheme for remotes waiting for it to settle, and stuck with
>> the old-styled .git/branches stuff which is slam-dunk simple and it
>> just works.

> And I'm personally actually pretty fed up with the .git/branches/ and
> .git/remotes/ thing, partly because I can never remember which file is
> which. I had to look at the code of git-parse-remote.sh to remind me which
> had what semantics. We could remove the old one entirely, of course (and
> no, I don't remember which is which now either), and avoid that particular
> problem, but it kind of soured me on it.
> 
> And if we truly have separate files, we should go all the way, and have
> the good old "one file, one value" rule. Which we don't, and which I think
> everybody admits would be horrible for this case for users (even if it
> might be very nice for scripting).

On one hand the remotes/ (or older branches/) is similar to refs/heads and
refs/tags that it contains shortcut names for pulling and pushing. On the
other hand configuration should belong to configuration.

Besides, AFAICT we did not have the place to have branch specific
configuration (like description, default place to pull from + marking
branch as immutable, default place to push to, etc.) and if I understand
correctly branches/ was used for something else. refs/heads/`name` stored
branches, including temporary branches which did not need configuration.

I guess that for the time being we can have remotes both in remotes/ and in
config file, plus script to freely transform between them (unless some
config would be unattainable by remotes/ file).

-- 
Jakub Narebski
Warsaw, Poland

Re: Implementing branch attributes in git config

[Martin Langhoff]

On 5/11/06, Linus Torvalds <torvalds@osdl.org> wrote:
> Sure. It clearly _is_ a bike shed, which is why I posted patches: I think
> the way to resolve bike sheds is to "just do it". In the kernel, the

there's no disputing that!

> So don't knock the bike sheds. It's a BSD term, and I think there's a
> _reason_ why it's a BSD term. Those people seem to sometimes like to

Apologies -- I didn't want to know it, but I do wonder what the gain
behind the change is. It seems to me that it would be a bad idea to
store refs in the config file and, in my mind at least, branch info is
closer to refs.

> > As an end-user, I have personally stayed away from the increasingly
> > complex scheme for remotes waiting for it to settle, and stuck with
> > the old-styled .git/branches stuff which is slam-dunk simple and it
> > just works.
>
> It does work, and I think it's nice in many ways. It was certainly a good
> way to prototype things.
>
> At the same time, especially with moving things more to C (or almost any
> other language, for that matter - shell is really pretty special in making
> it _easier_ to have things in individual files), it's in many ways nicer
> to have a more structured representation that has a nice fixed interface
> and a library and known access methods behind it.

But it is a bit of a loss for perl/shell porcelains, and for users
that abuse the contents of .git directly on a regular basis...

there's nothing to stop us from having a structured representation of
what's in .git/branches/

> And I'm personally actually pretty fed up with the .git/branches/ and
> .git/remotes/ thing, partly because I can never remember which file is
> which. I had to look at the code of git-parse-remote.sh to remind me

Agreed, it's a mess, but I suspect it's still there to support cogito.
In keeping with the 'talk code' ethos, I'll work on adding support for
.git/remotes in cogito.

> And if we truly have separate files, we should go all the way, and have
> the good old "one file, one value" rule.

What about one semantics, one file? So far we have had 3 semantics:
git config, remotes, refs. And git config has been mostly
project-indepentent. In fact, I have been copying it across my
checkouts of different, unrelated projects. You just don't do that
with remotes or refs.

cheers,


martin

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Thu, 11 May 2006, Martin Langhoff wrote:
> 
> Apologies -- I didn't want to know it, but I do wonder what the gain
> behind the change is.

I think we can do better in a few pretty important regards:

 - having the information in one place. I agree that the multi-file 
   approach works fine for shell scripts (although I disagree that the new 
   one would be harder - you just use git-repo-config instead), but I 
   think it's quite confusing from a new user perspective.

   I bet that even without any tools, new users can be told to just open 
   ".git/config", and guess how hard a time they would have to add a new 
   branch, if they already had one that said

	[branch "origin"]
		remote = git://git.kernel.org/pub/scm/git/git.git
		branch master

   which would tell you that the local branch "origin" is really branch
   "master" at that remote git repository.

   Yeah, I'm not sure what the actual config rules would be, but think it 
   would be a hell of a lot more intuitive than what we have now. 

   What we have now _works_. It works really well. No question about that. 
   It's just pretty hard to explain. The above syntax wouldn't even need 
   any explanation. You could just tell people to look into their config 
   files.

 - I think we'll have a much easier time (from a purely technical angle) 
   to add special attributes to the local branches. Add a "read-only" 
   specifier? It's _obvious_:

	[branch "origin"] 
		remote = git://git.kernel.org/pub/scm/git/git.git
		branch master
		readonly

   and it's absolutely trivial to parse. And part of the important thing 
   is that this all makes 100% sense EVEN IF IT'S NOT A REMOTE REPO!

   So imagine that it's a purely local branch, but you want to protect it. 
   Solution?

	[branch "July Snapshot"]
		readonly

   and you're done. In contrast, even if you ended up just extending the 
   file format for the .git/remotes/July\ Snapshot file, and just added a 
   "readonly" line to it, it wouldn't make _sense_. Whaa? "remotes"? In 
   contrast, in the .git/config file, it makes a ton of sense, and in fact 
   it's totally obvious.

   (Actually, we should probably have the .git/config file syntax separate 
   local branches like "master" from remote branches like "origin", so it 
   might be more like

	[remote "origin"]
		url = git://git.kernel.org/pub/scm/git/git.git

    which just tells that the _word_ "origin" corresponds to a 
    shorthand for a particular remote repository

	[branch "origin"]
		remote = origin
		branch = master

   or something to show that your _local_ branch named "origin" 
   corresponds to a particular remote (which could be a shorthand like the 
   above, or just spelled out), and a particular branch _at_ that remote 
   repository)

   Anyway, the point is, I think our current .git/remotes/xyzzy files 
   actually mix two different concepts, and they also end up doing it 
   pretty badly. They _work_, but because of the mix-ups, they aren't all 
   that they could be, and it's fundamentally impossible to make them so, 
   because the mixup really is that "origin" means TWO DIFFERENT THINGS 
   (the local branch, and the remote that it corresponds to)

 - Finally, I think it opens the possibility for some other things. For 
   example, once you accept that different branches might want attributes 
   like "readonly", you realize that some other attributes also make 
   sense. Like adding the default pull source per local branch, etc.

Again, I'm not saying that we can't work with the .git/remotes/ files. But 
I think it gets increasingly ugly, and the confusion gets increasingly 
worse.

> But it is a bit of a loss for perl/shell porcelains, and for users
> that abuse the contents of .git directly on a regular basis...

I really disagree. 

The .git/config file is _easier_ to edit by hand than the remotes. It's 
easier to copy-paste within one file than it is to work with two different 
files (and let's face it, copy-paste is usually what at least I would do 
for something like this). And it's _easier_ to just always open one file, 
and search within that one, than try to remember what file it was.

Now, C programs can very easily use the config library, and shell programs 
can equally easily query the variables with "git repo-config". I really 
doubt it's very hard for perl either, but I'm not a perl person, so maybe 
I don't see why this is hard.

		Linus

Re: Implementing branch attributes in git config

[Linus Torvalds]

On Wed, 10 May 2006, Linus Torvalds wrote:
>
>  - having the information in one place. I agree that the multi-file 
>    approach works fine for shell scripts (although I disagree that the new 
>    one would be harder - you just use git-repo-config instead), but I 
>    think it's quite confusing from a new user perspective.

Btw, I seriously believe that git has come to the point where we've licked 
the real technical issues. Stability hasn't been a concern for the last 
year - and even something as seriously as a repacking bug causing a 
SIGSEGV (yesterday) was actually basically designed to not be able to 
cause problems. The repack failed, and nothing happened to the old data. 
It was scary, but it wasn't "bad".

The last performance problem was a stupid one-liner, where one of the 
shell scripts didn't use the "--aggressive" flag for doing the trivial 
three-way merge, so it ended up forking and executing the "merge-one-file" 
shell script for 4500+ files for one unfortunate project that had a 
strange workflow. Adding the "--aggressive" flag took a 5-minute (where 
all of the time was spent in a shell script basically doing nothing) thing 
down to under a second.

So git should kick butt in performance, scale very well, and seems to take 
less disk space than just about anybody else. 

So what do people actually _complain_ about? 

I don't think we've seen a serious complaint lately that hasn't been about 
nice user interface and/or documentation. Anybody? 

So as far as I can tell, the #1 issue is that "new user" experience. You 
can pretty much forget about anything else. Working with git in a 
distributed manner is really easy and efficient, but from the comments 
I've seen, it's not always easy and obvious how to get to that point. 

Creating a remote repository and filling it. And being able to understand 
what the local vs remote branches actually _mean_. And I think our current 
.git/remotes/ thing is a part of that. It's not exactly user _hostile_, 
but it's very much "implementation friendly, and doesn't care about the 
user". So I think .git/config can help us there.

I also think we could do with a few scripts to just do setup of a remote 
repo:

	git remote clone <remoteaddress>
	git remote branch <remoteaddress> [-D]
	git remote fsck <remoteaddress>
	git remote repack <remoteaddress> -a -d

which would all basically boil down to "ssh to the remote address, cd into 
that directory, and do the named git command there" (well, not clone: 
doing a remote clone involves doing a mkdir/git-init-db/git-receive-pack 
remotely, and doing a git-send-pack locally, so some of them would be 
about doing things _both_ locally and remotely).

And documentation.

Now, I don't do documentation, and I really think somebody else could do 
the whole "git remote <cmd>" thing too. It _should_ really be pretty 
trivial. My real point is that almost none of this is about technology, 
and it's much more about trying to put a whole lot of lipstick on this 
pig. We have _got_ the technology already, and I think most people will 
agree git is doing pretty damn well there.

Because I really think the pig is quite charming, just sometimes you see 
some of its boorish sides right now..

(Or should that be "boarish", when we talk about pigs? ;)

		Linus

Re: Implementing branch attributes in git config

[Martin Langhoff]

On 5/11/06, Linus Torvalds <torvalds@osdl.org> wrote:
>         [branch "origin"]
>                 remote = git://git.kernel.org/pub/scm/git/git.git
>                 branch master

This is confusing at first read -- is it branch origin or branch master?

>    Anyway, the point is, I think our current .git/remotes/xyzzy files
>    actually mix two different concepts, and they also end up doing it
>    pretty badly. They _work_, but because of the mix-ups, they aren't all
>    that they could be, and it's fundamentally impossible to make them so,
>    because the mixup really is that "origin" means TWO DIFFERENT THINGS
>    (the local branch, and the remote that it corresponds to)

As you say, this needs to be explained/exposed better to the user.
Now, how about having one .git/config and one .git/branches file?
Different semantics, etc.

> The .git/config file is _easier_ to edit by hand than the remotes. It's
> easier to copy-paste within one file than it is to work with two

Agreed, but I suspect repo config and branches config travel at
different speeds. Maybe what this means is that if this happens, we'll
start seeing a need for ~/.git/config and /etc/git/config to set
defaults (merge.summary=1 for all my repos, core.sharedrepository=1
for all the repos on this server) where I now I just mostly copy
.git/config around.

Does that make sense?

> I don't see why this is hard.

Must be me... it's not the Perl part... I just do a lot of grep |
xargs | sed stuff in my daily git usage ;-)

cheers,


martin

Re: Implementing branch attributes in git config

[Nicolas Pitre]

On Wed, 10 May 2006, Linus Torvalds wrote:

> 	[branch "origin"]
> 		remote = git://git.kernel.org/pub/scm/git/git.git
> 		branch master

I totally agree with the principle, but I find the above really 
confusing.  Which "branch" means what?  At least if it was "remote_url" 
and "remote_branch" then there wouldn't be any possibility for 
confusion.


Nicolas

Re: Implementing branch attributes in git config

[Jakub Narebski]

Nicolas Pitre wrote:

> On Wed, 10 May 2006, Linus Torvalds wrote:
> 
>> [branch "origin"]
>> remote = git://git.kernel.org/pub/scm/git/git.git
>> branch master
> 
> I totally agree with the principle, but I find the above really
> confusing.  Which "branch" means what?  At least if it was "remote_url"
> and "remote_branch" then there wouldn't be any possibility for
> confusion.

I'm not sure if remotes shortcuts and configuration (which branches should
be pulled, and to which branches) should be in branches configuration. It
is somewhat confusing. Branches configuration might be used for default
pulling, e.g.


[remote "kernel.org"]
        url = git://git.kernel.org/pub/scm/git/git.git
        pull = master:origin
        ...
        pull = +pu:pu

[branch "origin"]
        pull = kernel.org
        readonly

[branch "pu"]
        pull = kernel.org
        readonly
        fast-forward = no

-- 
Jakub Narebski
Warsaw, Poland

Re: Implementing branch attributes in git config

[Jeff King]

On Wed, May 10, 2006 at 05:11:17PM -0700, Linus Torvalds wrote:

> I also think we could do with a few scripts to just do setup of a remote 
> repo:
> 
> 	git remote clone <remoteaddress>
> 	git remote branch <remoteaddress> [-D]
> 	git remote fsck <remoteaddress>
> 	git remote repack <remoteaddress> -a -d

Here's a 'git remote' that handles the easy commands. It makes things
like 'git remote origin repack -a -d' do what you expect. The biggest
problems are:
  - it only works for ssh remotes
  - it assumes your remote path is a git dir (do we have a usual way of
    deciding between $path and $path/.git?)
  - ssh'ing will mangle your shell quoting in the command arguments
  - the url parsing is somewhat ad-hoc (do we have a usual way of
    parsing urls for shell scripts?)

---
Add braindead git-remote script.

This script is a convenience wrapper for performing remote commands on a
repository using ssh.

---

 Makefile      |    2 +-
 git-remote.sh |   19 +++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletions(-)
 create mode 100644 git-remote.sh

8810ae2524d3339b8a8341b34b2d3f14ddb9c899
diff --git a/Makefile b/Makefile
index 37fbe78..58eddd8 100644
--- a/Makefile
+++ b/Makefile
@@ -125,7 +125,7 @@ SCRIPT_SH = \
 	git-applymbox.sh git-applypatch.sh git-am.sh \
 	git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
 	git-merge-resolve.sh git-merge-ours.sh git-grep.sh \
-	git-lost-found.sh
+	git-lost-found.sh git-remote.sh
 
 SCRIPT_PERL = \
 	git-archimport.perl git-cvsimport.perl git-relink.perl \
diff --git a/git-remote.sh b/git-remote.sh
new file mode 100644
index 0000000..04b1ce9
--- /dev/null
+++ b/git-remote.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+USAGE='<remote> <command> [options]'
+. git-sh-setup
+. git-parse-remote
+
+case "$#" in
+  0|1) usage ;;
+esac
+
+remote=`get_remote_url "$1"` shift;
+case "$remote" in
+  ssh://*|git+ssh://*|ssh+git://*)
+    host=`echo "$remote" | sed 's!^[^/]*://\([^/]*\).*!\1!'`
+    path=`echo "$remote" | sed 's!^[^/]*://[^/]*\(.*\)!\1!'`
+    exec ssh -n $host "GIT_DIR=$path git $@"
+    ;;
+  *) die "unhandled protocol: $remote" ;;
+esac
-- 
1.3.1

Re: Implementing branch attributes in git config

[Johannes Schindelin]

Hi,

On Thu, 11 May 2006, Martin Langhoff wrote:

> [...] where I now I just mostly copy .git/config around.

Why not just edit the template?

Ciao,
Dscho

Re: Implementing branch attributes in git config

[Jeff King]

On Thu, May 11, 2006 at 05:51:58AM -0400, Jeff King wrote:

> +remote=`get_remote_url "$1"` shift;

Oops, this should be:
  remote=`get_remote_url "$1"`; shift
but it actually works fine under dash (a bug in dash?)

-Peff

Re: Implementing branch attributes in git config

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> On Tue, 9 May 2006, Junio C Hamano wrote:
>> 
>> If we are shooting for "let's not do this again", I do not think
>> (4) without some quoting convention is good enough.  Today, we
>> are talking about branch names so we could give them artificial
>> limits, which could be weaker than what we already have on the
>> branch names, but we would later regret that, when we start
>> wanting to have other names in the configuration (e.g. people's
>> names).
>
> Here's my suggestion as a patch.
>
> NOTE! This patch could be applied right now, and to all the branches (to
> make 1.x, 1.2.x and 1.3.x all support the _syntax_). Even if nothing 
> actually uses it.

Linus,

I've adjusted this patch, your follow-up patch, and Sean's
"extended section part is case sensitive" patch, along with the
test tweak to "maint" branch.  I also prepared them to be
mergeable to the "master" branch.  Tentatively it is in "next"
for testing.

I'm ready to push out "maint" (not tagged as 1.3.3 yet) and
"next", but have not done so.

I understand the plan is to have 1.3.3 out from this "maint",
and also merge this in "master" about the same time, but I am
expecting I will be offline for the rest of the week most of the
time, so it would happen over the weekend at the earliest.

Does that sound good to you?

I do not think we would need v1.1.7 nor v1.2.7 for this.  People
who have stayed at v1.1.6 or v1.2.6 would need to update if they
are going to use newer git in their repo anyway, and I do not
think of a reason not to update to 1.3.3 but update to 1.1.7 or
1.2.7 in order to stay at the feature level of 1.1.X or 1.2.X
series.  We haven't made incompatible changes as far as I
remember.

Here is what the (adjusted) test case in "next" looks like.
Corresponding one in "maint" lack --list so does not have the
last hunk.  The "maint" and "next" branches I have locally both
passes the test.

-- >8 --

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 7090ea9..8260d57 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -229,7 +229,7 @@ test_expect_failure 'invalid key' 'git-r
 test_expect_success 'correct key' 'git-repo-config 123456.a123 987'
 
 test_expect_success 'hierarchical section' \
-	'git-repo-config 1.2.3.alpha beta'
+	'git-repo-config Version.1.2.3eX.Alpha beta'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -241,8 +241,8 @@ # empty line
 	NoNewLine = wow2 for me
 [123456]
 	a123 = 987
-[1.2.3]
-	alpha = beta
+[Version "1.2.3eX"]
+	Alpha = beta
 EOF
 
 test_expect_success 'hierarchical section value' 'cmp .git/config expect'
@@ -251,7 +251,7 @@ cat > expect << EOF
 beta.noindent=sillyValue
 nextsection.nonewline=wow2 for me
 123456.a123=987
-1.2.3.alpha=beta
+version.1.2.3eX.alpha=beta
 EOF
 
 test_expect_success 'working --list' \