Git 1.1.6.g4d44 make test FAILURE report

Running "make test" from an account with an empty gcos field fails test 6 of t1200-tutorial.sh.

If the gcos field is a single space character, "make test" fails test 3 of t3500-cherry.sh.

Thanks; good catch. I think we should export GIT_*_{NAME,EMAIL} variables explicitly for this particular problem. We probably do _not_ want to do the same for GIT_*_DATE though.

Also, git-var complains when used by an account with an empty gcos field; thereby, breaking all the non-C git commands even when the user is not committing.

If the _intent_ was to force commiters and author names in commits, why was the test not placed only in commit-tree.c?

git-var was more or less intentional. Scripts such as git-applypatch, git-commit and git-tag use the command to grab COMMITTER_IDENT to generate sign-off line and tagger information when asked, and commit-tree.c changes alone would not catch them.

A user eventually would make commit so it may not be a too bad to _strongly_ encourage setting up these environment variables, by being nasty ;-). I agree it would be _very_ annoying until you either fix your gecos and/or environment.

Ideas welcome.

It also breaks a lot of commands not related to making commits; git-fetch for one.

git-sh-setup one is easy to fix. We could say something equally silly like this instead:

index 1e638e4..157c7e4 100755
@@ -41,7 +41,11 @@ then
 	: ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"}
 
 	# Make sure we are in a valid repository of a vintage we understand.
-	GIT_DIR="$GIT_DIR" git-var GIT_AUTHOR_IDENT >/dev/null || exit
+	GIT_DIR="$GIT_DIR" git repo-config --get core.nosuch >/dev/null
+	if test $? == 128
+	then
+	    exit
+	fi
 else
 	GIT_DIR=$(git-rev-parse --git-dir) || exit
 fi

The only thing that code cares about is to use a relatively cheap command to examine "$GIT_DIR/config" to make sure that the respository format version recorded in the file is of old enough vintage we understand; the command to cause .git/config to be read does not have to be git-var.

So after nixing that one, the remaining users all look like they do want something reasonable from the user anyway, so for them git-var that fails when the user has not fixed the unusable gecos is a desirable thing.

git grep -n git-var '*.sh'

shows:

git-am, git-applypatch, git-format-patch::
use it to add sign-off by the user.

git-commit, git-tag::
use it to record the user identity.

git-pull::
this uses git-var-l which is unfortunate. We should be able to use git-repo-config instead. Patches welcome.

BTW, the reason I initially did this one and git-pull with git-var was there was no way to extract values from the config file easily from the command line back then. git-repo-config was initially called git-config-set and was about setting values.

IMHO the configuration mechanism, and git-var in particular, is quite broken, and nobody bothered to fix it.

(1) git-var -l can show all defined variables in the config file, but you cannot ask about specific variable it does not know about. You have to use git-repo-config for that:

$ git var -l | head -n 4 core.filemode=true core.gitproxy=none for kernel.org pull.twohead=resolve pull.octopus=octopus
$ git var core.filemode ;# it does not know
usage: git-var [-l | <variable>]
$ git repo-config core.filemode true

(2) git-repo-config does not have a way to list all the configuration items like "git-var -l".

(3) neither of these commands know list of all the possible configuration items, nor types of them, so core.filename can be spelled as "1" or "true" to mean the same thing to our C code, but repo-config faithfully returns how the value is literally spelled in the configuration file. The following two means the same thing to the C layer, so the calling script needs to further interpret the output from git-repo-config:

$ git repo-config core.filemode ;# [core] filemode=1 1
$ git repo-config core.filemode ;# [core] filemode=true true

(4) worse, boolean 'true' can be specified by just having the configuration item in the file, but repo-config dumps core on that:

$ git repo-config core.filemode ;# [core] filemode segmentation fault

Above problems are not the only problems with git-var and git-repo-config, but these are not entirely these programs' faults. They come from the way the configuration file mechanism works. To allow different classes of configuration items to be defined by commands that know about them, there is no central registry of all the supported configuration items in the code. For example, "diff.*" configuration items can be interpreted only by C level programs that call git_config(git_diff_config).

With the current structure, it is very hard for these commands to sensibly list all the configuration values (e.g. lack of "[core] filemode" in the configuration file ought to mean that the executable bit is unreliable i.e. core.filemode=false, but they cannot report it), or report the values taking into account their types. I do not think they even attempt to do so.

Dear diary, on Sat, Feb 11, 2006 at 05:43:21AM CET, I got a letter where Junio C Hamano <[email protected]> said that...

This patch provides a partial solution - if you query only for variables of the same type (or just a single variable), this adds type-checking and transformation to the given type.

It is basically what Cogito would like to see - centralized variables database in GIT won't help us, but we would like to have custom but still typed variables in the config file.

---

[PATCH] Add support for explicit type specifiers when calling git-repo-config

Currently, git-repo-config will just return the raw value of option as specified in the config file; this makes things difficult for scripts calling it, especially if the value is supposed to be boolean.

This patch makes it possible to ask git-repo-config to check if the option is of the given type (int or bool) and write out the value in its canonical form. If you do not pass --int or --bool, the behaviour stays unchanged and the raw value is emitted.

This also incidentally fixes the segfault when option with no value is encountered.

---
commit 8dcc626cd144b2c6eae2a299242bbbe905cb0059
tree 0d4dcc3a44eb318ef52c3d64dda11768745f7583
parent 29e55cd5ad9e17d2ff8a1a37b7ee45d18d1e59d6
author Petr Baudis <[email protected]> Sun, 12 Feb 2006 04:09:01 +0100 committer Petr Baudis <[email protected]> Sun, 12 Feb 2006 04:09:01 +0100

 Documentation/git-repo-config.txt |   18 ++++++--
 repo-config.c                     |   80 +++++++++++++++++++++++--------------
 2 files changed, 62 insertions(+), 36 deletions(-)

index 3069464..33fcde4 100644
@@ -8,12 +8,12 @@ git-repo-config - Get and set options in
 
 SYNOPSIS
 --------
-'git-repo-config' name [value [value_regex]]
-'git-repo-config' --replace-all name [value [value_regex]]
-'git-repo-config' --get name [value_regex]
-'git-repo-config' --get-all name [value_regex]
-'git-repo-config' --unset name [value_regex]
-'git-repo-config' --unset-all name [value_regex]
+'git-repo-config' [type] name [value [value_regex]]
+'git-repo-config' [type] --replace-all name [value [value_regex]]
+'git-repo-config' [type] --get name [value_regex]
+'git-repo-config' [type] --get-all name [value_regex]
+'git-repo-config' [type] --unset name [value_regex]
+'git-repo-config' [type] --unset-all name [value_regex]
 
 DESCRIPTION
 -----------

@@ -26,6 +26,12 @@ should provide a POSIX regex for the val
 *not* matching 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
+convert the value to the canonical form (simple decimal number for int,
+a "true" or "false" string for bool). If no type specifier is passed,
+no checks or transformations are performed on the value.
+
 This command will fail if
 
 . .git/config is invalid,

				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Of the 3 great composers Mozart tells us what it's like to be human,
Beethoven tells us what it's like to be Beethoven and Bach tells us
what it's like to be the universe.  -- Douglas Adams

Now you can say "git-repo-config -l" and it will output the same as "git-var -l", except that the keys are separated from the values by a space instead of an equal sign.

If you want to match only "core" lines, just do

	git-repo-config --get-regexp ^core

and if you want to be more specific, you can say

	git-repo-config --get-regexp "^core.*mode"

to match any keys in section [core] which end in "mode".

---

On Fri, 10 Feb 2006, Junio C Hamano wrote:

> Junio C Hamano <[email protected]> writes:
>
> > git-sh-setup one is easy to fix. We could say something equally > > silly like this instead:
>
> BTW, the reason I initially did this one and git-pull with > git-var was there was no way to extract values from the config > file easily from the command line back then. git-repo-config was > initially called git-config-set and was about setting values.

Happier?

 Documentation/git-repo-config.txt |   10 ++++-
 repo-config.c                     |   77 ++++++++++++++++++++++++++++---------
 2 files changed, 68 insertions(+), 19 deletions(-)

aef2834ec6dfbdcca79879b4b749bbe9d3c2b63f

index 3069464..b7e0c23 100644
@@ -8,10 +8,12 @@ git-repo-config - Get and set options in
 
 SYNOPSIS
 --------
+'git-repo-config' -l
 'git-repo-config' name [value [value_regex]]
 'git-repo-config' --replace-all name [value [value_regex]]
 'git-repo-config' --get name [value_regex]
 'git-repo-config' --get-all name [value_regex]
+'git-repo-config' --get-regexp name
 'git-repo-config' --unset name [value_regex]
 'git-repo-config' --unset-all name [value_regex]
 

@@ -39,9 +41,12 @@ This command will fail if
 OPTIONS
 -------
 
+-l::
+	List all key/value pairs.
+
 --replace-all::
 	Default behaviour is to replace at most one line. This replaces
-	all lines matching the key (and optionally the value_regex)
+	all lines matching the key (and optionally the value_regex).
 
 --get::
 	Get the value for a given key (optionally filtered by a regex

@@ -51,6 +56,9 @@ OPTIONS
 	Like get, but does not fail if the number of values for the key
 	is not exactly one.
 
+--get-regexp::
+	Like --get-all, but interprets the name as a regular expression.
+
 --unset::
 	Remove the line matching the key from .git/config.
 

1.1.4.g3d3a-dirty

Not really.

It still dumps core with:

	[core]
        	boolvarsaretrueiftheirnamesarelisted

The patch does not address any of the more important issues I listed with git-var and git-repo-config in that message.