Re: Seeing various mode changes on cygwin

Daniel Barkalow [off-list ref] writes:

Perhaps have a bit in the index mode for the file to say that the mode in 
the filesystem is unreliable, which gets set if a stat of the 
newly-written file doesn't match the mode it was supposed to have, or if 
git chmod is used to change it; then, if the bit is set, ignore the mode 
in the filesystem and just use the mode in the index.

In effect, you are making the "per-repo configuration" Linus
mentioned a non configuration but a property recorded in the
index file.  I think this is a clever solution which is very
helpful to the end user.  I have to think about this a bit, but
my gut feeling tells me that it is the right direction if it
works.

I do not think you have to necessarily record it in the "index
mode" -- which implies this is per path -- nor even in the index
file itself.  We might even be able to get away with doing this
check at git-init-db time just once, and record it in a file,
say ".git/fs-mode-unreliable".

On Mon, 10 Oct 2005, Junio C Hamano wrote:

Daniel Barkalow [off-list ref] writes:

quoted

Perhaps have a bit in the index mode for the file to say that the mode in 
the filesystem is unreliable, which gets set if a stat of the 
newly-written file doesn't match the mode it was supposed to have, or if 
git chmod is used to change it; then, if the bit is set, ignore the mode 
in the filesystem and just use the mode in the index.

In effect, you are making the "per-repo configuration" Linus
mentioned a non configuration but a property recorded in the
index file.  I think this is a clever solution which is very
helpful to the end user.  I have to think about this a bit, but
my gut feeling tells me that it is the right direction if it
works.

I do not think you have to necessarily record it in the "index
mode" -- which implies this is per path -- nor even in the index
file itself.  We might even be able to get away with doing this
check at git-init-db time just once, and record it in a file,
say ".git/fs-mode-unreliable".

Actually, you're right; it is information about the behavior of the 
working tree, and is also needed if you want to compare the working tree 
against a tree object, in which case you aren't using the index at all.

	-Daniel
*This .sig left intentionally blank*

On Mon, 10 Oct 2005, Daniel Barkalow wrote:

Actually, you're right; it is information about the behavior of the 
working tree, and is also needed if you want to compare the working tree 
against a tree object, in which case you aren't using the index at all.

Git _always_ uses the index for working tree operations.

It may take the actual file _data_ from the working tree, but it will take 
the list of files from the index, so it's certainly possible to link the 
index to the working tree.

That said, I don't think it's necessarily a good idea. You can have 
temporary indexes for various operations that ignore the main one (ie any 
random

	GIT_INDEX_FILE=tmp git-read-tree ...

will create a new index).

So I think it's much better to have a config file.

I'll write something up. Make it extensible while at it.

		Linus

This is a first cut at a very simple parser for a git config file.

The format of the file is a simple ini-file like thing, with simple 
variable/value pairs. You can (and should) make the variables have a 
simple single-level scope, ie a valid file looks something like this:

	#
	# This is the config file, and
	# a '#' or ';' character indicates
	# a comment
	#

	; core variables
	[core]
		; Don't trust file modes
		filemode = false

	; Our diff algorithm 
	[diff]
		external = "/usr/local/bin/gnu-diff -u"
		renames = true

which parses into two variables: "core.filemode" is associated with the 
string "false", and "diff.external" gets the appropriate quoted value.

Right now we only react to one variable: "core.filemode" is a boolean that 
decides if we should care about the 0100 (user-execute) bit of the stat 
information. Even that is just a parsing demonstration - this doesn't 
actually implement that st_mode compare logic itself.

Different programs can react to different config options, although they 
should always fall back to calling "git_default_config()" on any config 
option name that they don't recognize.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
----

Ok, so it's stupid. But quite frankly, I think the Windows ini-file format 
is a hell of a lot more readable than something over-engineered like XML 
files or other crap.

The interface is really easy to use, imho. You can do things like

	static int enable_renames = 0;

	static int my_options(const char *var, const char *value)
	{
		if (!strcmp("diff.renames", var)) {
			enable_renames = git_config_bool(var, value);
			return 0;
		}

		/*
		 * Put other local option parsing for this program
		 * here .. 
		 */

		/* Fall back on the default ones */
		return git_default_config(var, value);
	}

and then in the "main()" routine you just do

		git_config(my_options);

at the top (or, more precisely, just after the "git_setup_directory()" if 
you have one).

And as usual, it's not like this has gotten a whole lot of testing.

Flames, comments, whatever? The code is actually written so that the 
config file parsing should really be pretty neutral. It doesn't even have 
any git-specific in it, except for the naming and the actual initial 
"fopen()" pathname used, I think.

---

diff --git a/Makefile b/Makefile
index a201187..e8b46f1 100644
--- a/Makefile
+++ b/Makefile

@@ -158,7 +158,7 @@ LIB_OBJS = \
 	object.o pack-check.o patch-delta.o path.o pkt-line.o \
 	quote.o read-cache.o refs.o run-command.o \
 	server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \
-	tag.o tree.o usage.o $(DIFF_OBJS)
+	tag.o tree.o usage.o config.o $(DIFF_OBJS)
 
 LIBS = $(LIB_FILE)
 LIBS += -lz

diff --git a/cache.h b/cache.h
index 5987d4c..0571282 100644
--- a/cache.h
+++ b/cache.h

@@ -178,6 +178,8 @@ extern int hold_index_file_for_update(st
 extern int commit_index_file(struct cache_file *);
 extern void rollback_index_file(struct cache_file *);
 
+extern int trust_executable_bit;
+
 #define MTIME_CHANGED	0x0001
 #define CTIME_CHANGED	0x0002
 #define OWNER_CHANGED	0x0004

@@ -372,4 +374,10 @@ extern int gitfakemunmap(void *start, si
 
 #endif
 
+typedef int (*config_fn_t)(const char *, const char *);
+extern int git_default_config(const char *, const char *);
+extern int git_config(config_fn_t fn);
+extern int git_config_int(const char *, const char *);
+extern int git_config_bool(const char *, const char *);
+
 #endif /* CACHE_H */

diff --git a/config.c b/config.c
new file mode 100644
index 0000000..f3c4fa4
--- /dev/null
+++ b/config.c

@@ -0,0 +1,222 @@
+#include <ctype.h>
+
+#include "cache.h"
+
+#define MAXNAME (256)
+
+static FILE *config_file;
+static int config_linenr;
+static int get_next_char(void)
+{
+	int c;
+	FILE *f;
+
+	c = '\n';
+	if ((f = config_file) != NULL) {
+		c = fgetc(f);
+		if (c == '\n')
+			config_linenr++;
+		if (c == EOF) {
+			config_file = NULL;
+			c = '\n';
+		}
+	}
+	return c;
+}
+
+static char *parse_value(void)
+{
+	static char value[1024];
+	int quote = 0, comment = 0, len = 0, space = 0;
+
+	for (;;) {
+		int c = get_next_char();
+		if (len >= sizeof(value))
+			return NULL;
+		if (c == '\n') {
+			if (quote)
+				return NULL;
+			value[len] = 0;
+			return value;
+		}
+		if (comment)
+			continue;
+		if (isspace(c) && !quote) {
+			space = 1;
+			continue;
+		}
+		if (space) {
+			if (len)
+				value[len++] = ' ';
+			space = 0;
+		}
+		if (c == '\\') {
+			c = get_next_char();
+			switch (c) {
+			case '\n':
+				continue;
+			case 't':
+				c = '\t';
+				break;
+			case 'b':
+				c = '\b';
+				break;
+			case 'n':
+				c = '\n';
+				break;
+			return NULL;
+			}
+			value[len++] = c;
+			continue;
+		}
+		if (c == '"') {
+			quote = 1-quote;
+			continue;
+		}
+		if (!quote) {
+			if (c == ';' || c == '#') {
+				comment = 1;
+				continue;
+			}
+		}
+		value[len++] = c;
+	}
+}
+
+static int get_value(config_fn_t fn, char *name, unsigned int len)
+{
+	int c;
+	char *value;
+
+	/* Get the full name */
+	for (;;) {
+		c = get_next_char();
+		if (c == EOF)
+			break;
+		if (!isalnum(c))
+			break;
+		name[len++] = tolower(c);
+		if (len >= MAXNAME)
+			return -1;
+	}
+	name[len] = 0;
+	while (c == ' ' || c == '\t')
+		c = get_next_char();
+
+	value = NULL;
+	if (c != '\n') {
+		if (c != '=')
+			return -1;
+		value = parse_value();
+		if (!value)
+			return -1;
+	}
+	return fn(name, value);
+}
+
+static int get_base_var(char *name)
+{
+	int baselen = 0;
+
+	for (;;) {
+		int c = get_next_char();
+		if (c == EOF)
+			return -1;
+		if (c == ']')
+			return baselen;
+		if (!isalnum(c))
+			return -1;
+		if (baselen > MAXNAME / 2)
+			return -1;
+		name[baselen++] = tolower(c);
+	}
+}
+
+static int git_parse_file(config_fn_t fn)
+{
+	int comment = 0;
+	int baselen = 0;
+	static char var[MAXNAME];
+
+	for (;;) {
+		int c = get_next_char();
+		if (c == '\n') {
+			/* EOF? */
+			if (!config_file)
+				return 0;
+			comment = 0;
+			continue;
+		}
+		if (comment || isspace(c))
+			continue;
+		if (c == '#' || c == ';') {
+			comment = 1;
+			continue;
+		}
+		if (c == '[') {
+			baselen = get_base_var(var);
+			if (baselen <= 0)
+				break;
+			var[baselen++] = '.';
+			var[baselen] = 0;
+			continue;
+		}
+		if (!isalpha(c))
+			break;
+		var[baselen] = c;
+		if (get_value(fn, var, baselen+1) < 0)
+			break;
+	}
+	die("bad config file line %d", config_linenr);
+}
+
+int git_config_int(const char *name, const char *value)
+{
+	if (value && *value) {
+		char *end;
+		int val = strtol(value, &end, 0);
+		if (!*end)
+			return val;
+	}
+	die("bad config value for '%s'", name);
+}
+
+int git_config_bool(const char *name, const char *value)
+{
+	if (!value)
+		return 1;
+	if (!*value)
+		return 0;
+	if (!strcasecmp(value, "true"))
+		return 1;
+	if (!strcasecmp(value, "false"))
+		return 0;
+	return git_config_int(name, value) != 0;
+}
+
+int git_default_config(const char *var, const char *value)
+{
+	/* This needs a better name */
+	if (!strcmp(var, "core.filemode")) {
+		trust_executable_bit = git_config_bool(var, value);
+		return 0;
+	}
+
+	/* Add other config variables here.. */
+	return 0;
+}
+
+int git_config(config_fn_t fn)
+{
+	int ret;
+	FILE *f = fopen(git_path("config"), "r");
+
+	ret = -1;
+	if (f) {
+		config_file = f;
+		config_linenr = 1;
+		ret = git_parse_file(fn);
+		fclose(f);
+	}
+	return ret;
+}

diff --git a/diff-files.c b/diff-files.c
index 5e59832..96d2c7f 100644
--- a/diff-files.c
+++ b/diff-files.c

@@ -38,6 +38,7 @@ int main(int argc, const char **argv)
 	const char *prefix = setup_git_directory();
 	int entries, i;
 
+	git_config(git_default_config);
 	diff_setup(&diff_options);
 	while (1 < argc && argv[1][0] == '-') {
 		if (!strcmp(argv[1], "-q"))

diff --git a/diff-tree.c b/diff-tree.c
index b2d74eb..2203fa5 100644
--- a/diff-tree.c
+++ b/diff-tree.c

@@ -408,6 +408,7 @@ int main(int argc, const char **argv)
 	unsigned char sha1[2][20];
 	const char *prefix = setup_git_directory();
 
+	git_config(git_default_config);
 	nr_sha1 = 0;
 	diff_setup(&diff_options);
 

diff --git a/read-cache.c b/read-cache.c
index d2aebdd..c7f3b26 100644
--- a/read-cache.c
+++ b/read-cache.c

@@ -5,6 +5,7 @@
  */
 #include "cache.h"
 
+int trust_executable_bit = 1;
 struct cache_entry **active_cache = NULL;
 unsigned int active_nr = 0, active_alloc = 0, active_cache_changed = 0;
 

On Mon, 10 Oct 2005, Linus Torvalds wrote:

	; core variables
	[core]
		; Don't trust file modes
		filemode = false

	; Our diff algorithm 
	[diff]
		external = "/usr/local/bin/gnu-diff -u"
		renames = true

which parses into two variables: "core.filemode" is associated with the 
string "false", and "diff.external" gets the appropriate quoted value.

_Three_ variables. Duh. I added the "renames" thing later, as I was 
looking at what kinds of default flags the "git-diff-xyz" family might be 
interested in having.

		Linus

Linus Torvalds wrote:

On Mon, 10 Oct 2005, Linus Torvalds wrote:

quoted

; core variables
[core]
	; Don't trust file modes
	filemode = false

; Our diff algorithm 
[diff]
	external = "/usr/local/bin/gnu-diff -u"
	renames = true

which parses into two variables: "core.filemode" is associated with the 
string "false", and "diff.external" gets the appropriate quoted value.

 
_Three_ variables. Duh. I added the "renames" thing later, as I was 
looking at what kinds of default flags the "git-diff-xyz" family might be 
interested in having.

A suggestion: if you make your basic types (integer, string, boolean) 
recognizable by the parser, you can present them in that way.  This 
pretty much means strings will have to always be double-quoted, but that 
avoids a bunch of ambiguities.

	-hpa

On Mon, 10 Oct 2005, H. Peter Anvin wrote:

A suggestion: if you make your basic types (integer, string, boolean)
recognizable by the parser, you can present them in that way.  This pretty
much means strings will have to always be double-quoted, but that avoids a
bunch of ambiguities.

Well, the parser is already pretty unambiguous, purely by virtue of being 
stupid as hell.

In particular, the only thing quoting does is to actually make whitespace 
meaningful, and as a way to allow comments inside strings (otherwise a "#" 
or ";" is always a "comment starts here" marker).

Outside of quotes, any whitespace will just collapse to a single space 
(and be removed from beginning and end).

And quite frankly, always keeping the things as strings just makes things 
so much easier and the interfaces very simple. And if/when you want to 
turn the string into a boolean or a regular integer, there are two helper 
functions that do exactly that, so it's not very hard.

I considered using some "smart" parser (ie using something like 
flex/bison), but the thing is, I didn't want smart. I personally think 
it's a lot more important for the file format to be _nice_, and there I 
think it's fine if

	[diff]
		external=/usr/local/bin/gnu-diff

doesn't need quotes, even if it's obviously a string. Simplicity is a 
virtue (both in parsing and in the "language" parsed).

In fact, even the example I had - with a space and an argument - doesn't 
need quotes (since the single space will be collapsed to a single space), 
I just put that as an example.

Side note: the design is meant to allow different programs to share the 
same config file without having to know about each others config 
variables. Anything they don't recognize is just ignored. The downside is 
that if you mistype an option name, nobody will recognize it, and nobody 
will complain either. 

That was one of the reasons for the "scoping". It not only allows grouping 
of variables, but it means that the "git-diff-xyz" family of programs 
might decide that they'll report anything that starts with "diff." but 
that they don't understand as a warning (but preferably not error, since 
it might be a newer option that an older version of git just doesn't 
understand).

		Linus