From a9cc857ada7c57069ff00eed8d0addcf55849f39 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 2 Nov 2007 00:24:27 -0700 Subject: War on whitespace: first, a bit of retreat. This introduces core.whitespace configuration variable that lets you specify the definition of "whitespace error". Currently there are two kinds of whitespace errors defined: * trailing-space: trailing whitespaces at the end of the line. * space-before-tab: a SP appears immediately before HT in the indent part of the line. You can specify the desired types of errors to be detected by listing their names (unique abbreviations are accepted) separated by comma. By default, these two errors are always detected, as that is the traditional behaviour. You can disable detection of a particular type of error by prefixing a '-' in front of the name of the error, like this: [core] whitespace = -trailing-space This patch teaches the code to output colored diff with DIFF_WHITESPACE color to highlight the detected whitespace errors to honor the new configuration. Signed-off-by: Junio C Hamano --- config.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) (limited to 'config.c') diff --git a/config.c b/config.c index dc3148d456..ffb418ccff 100644 --- a/config.c +++ b/config.c @@ -246,6 +246,53 @@ static unsigned long get_unit_factor(const char *end) die("unknown unit: '%s'", end); } +static struct whitespace_rule { + const char *rule_name; + unsigned rule_bits; +} whitespace_rule_names[] = { + { "trailing-space", WS_TRAILING_SPACE }, + { "space-before-tab", WS_SPACE_BEFORE_TAB }, +}; + +static unsigned parse_whitespace_rule(const char *string) +{ + unsigned rule = WS_DEFAULT_RULE; + + while (string) { + int i; + size_t len; + const char *ep; + int negated = 0; + + string = string + strspn(string, ", \t\n\r"); + ep = strchr(string, ','); + if (!ep) + len = strlen(string); + else + len = ep - string; + + if (*string == '-') { + negated = 1; + string++; + len--; + } + if (!len) + break; + for (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++) { + if (strncmp(whitespace_rule_names[i].rule_name, + string, len)) + continue; + if (negated) + rule &= ~whitespace_rule_names[i].rule_bits; + else + rule |= whitespace_rule_names[i].rule_bits; + break; + } + string = ep; + } + return rule; +} + int git_parse_long(const char *value, long *ret) { if (value && *value) { @@ -431,6 +478,11 @@ int git_default_config(const char *var, const char *value) return 0; } + if (!strcmp(var, "core.whitespace")) { + whitespace_rule = parse_whitespace_rule(value); + return 0; + } + /* Add other config variables here and to Documentation/config.txt. */ return 0; } -- cgit v1.2.3 From 459fa6d0fe6a45b8b120463b56a68e943e3a8101 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 2 Oct 2007 18:00:27 -0700 Subject: git-diff: complain about >=8 consecutive spaces in initial indent This introduces a new whitespace error type, "indent-with-non-tab". The error is about starting a line with 8 or more SP, instead of indenting it with a HT. This is not enabled by default, as some projects employ an indenting policy to use only SPs and no HTs. The kernel folks and git contributors may want to enable this detection with: [core] whitespace = indent-with-non-tab Signed-off-by: Junio C Hamano --- config.c | 1 + 1 file changed, 1 insertion(+) (limited to 'config.c') diff --git a/config.c b/config.c index ffb418ccff..d5b976696c 100644 --- a/config.c +++ b/config.c @@ -252,6 +252,7 @@ static struct whitespace_rule { } whitespace_rule_names[] = { { "trailing-space", WS_TRAILING_SPACE }, { "space-before-tab", WS_SPACE_BEFORE_TAB }, + { "indent-with-non-tab", WS_INDENT_WITH_NON_TAB }, }; static unsigned parse_whitespace_rule(const char *string) -- cgit v1.2.3 From 039bc64e886716593d59910694a6c8ed5b72c515 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 14 Nov 2007 00:05:00 -0800 Subject: core.excludesfile clean-up There are inconsistencies in the way commands currently handle the core.excludesfile configuration variable. The problem is the variable is too new to be noticed by anything other than git-add and git-status. * git-ls-files does not notice any of the "ignore" files by default, as it predates the standardized set of ignore files. The calling scripts established the convention to use .git/info/exclude, .gitignore, and later core.excludesfile. * git-add and git-status know about it because they call add_excludes_from_file() directly with their own notion of which standard set of ignore files to use. This is just a stupid duplication of code that need to be updated every time the definition of the standard set of ignore files is changed. * git-read-tree takes --exclude-per-directory=, not because the flexibility was needed. Again, this was because the option predates the standardization of the ignore files. * git-merge-recursive uses hardcoded per-directory .gitignore and nothing else. git-clean (scripted version) does not honor core.* because its call to underlying ls-files does not know about it. git-clean in C (parked in 'pu') doesn't either. We probably could change git-ls-files to use the standard set when no excludes are specified on the command line and ignore processing was asked, or something like that, but that will be a change in semantics and might break people's scripts in a subtle way. I am somewhat reluctant to make such a change. On the other hand, I think it makes perfect sense to fix git-read-tree, git-merge-recursive and git-clean to follow the same rule as other commands. I do not think of a valid use case to give an exclude-per-directory that is nonstandard to read-tree command, outside a "negative" test in the t1004 test script. This patch is the first step to untangle this mess. The next step would be to teach read-tree, merge-recursive and clean (in C) to use setup_standard_excludes(). Signed-off-by: Junio C Hamano --- config.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'config.c') diff --git a/config.c b/config.c index dc3148d456..56e99fc0f4 100644 --- a/config.c +++ b/config.c @@ -431,6 +431,13 @@ int git_default_config(const char *var, const char *value) return 0; } + if (!strcmp(var, "core.excludesfile")) { + if (!value) + die("core.excludesfile without value"); + excludes_file = xstrdup(value); + return 0; + } + /* Add other config variables here and to Documentation/config.txt. */ return 0; } -- cgit v1.2.3 From 506b17b136ba8ee29098c2ee26b94f527cea9ebc Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Tue, 13 Nov 2007 21:05:05 +0100 Subject: Introduce git_etc_gitconfig() that encapsulates access of ETC_GITCONFIG. In a subsequent patch the path to the system-wide config file will be computed. This is a preparation for that change. It turns all accesses of ETC_GITCONFIG into function calls. There is no change in behavior. As a consequence, config.c is the only file that needs the definition of ETC_GITCONFIG. Hence, -DETC_GITCONFIG is removed from the CFLAGS and a special build rule for config.c is introduced. As a side-effect, changing the defintion of ETC_GITCONFIG (e.g. in config.mak) does not trigger a complete rebuild anymore. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- config.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 56e99fc0f4..cee2d26d0c 100644 --- a/config.c +++ b/config.c @@ -459,6 +459,11 @@ int git_config_from_file(config_fn_t fn, const char *filename) return ret; } +const char *git_etc_gitconfig(void) +{ + return ETC_GITCONFIG; +} + int git_config(config_fn_t fn) { int ret = 0; @@ -471,8 +476,8 @@ int git_config(config_fn_t fn) * config file otherwise. */ filename = getenv(CONFIG_ENVIRONMENT); if (!filename) { - if (!access(ETC_GITCONFIG, R_OK)) - ret += git_config_from_file(fn, ETC_GITCONFIG); + if (!access(git_etc_gitconfig(), R_OK)) + ret += git_config_from_file(fn, git_etc_gitconfig()); home = getenv("HOME"); filename = getenv(CONFIG_LOCAL_ENVIRONMENT); if (!filename) -- cgit v1.2.3 From 7f0e39faa2929541f61039be96d3f40f74207585 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Tue, 13 Nov 2007 21:05:06 +0100 Subject: Allow ETC_GITCONFIG to be a relative path. If ETC_GITCONFIG is not an absolute path, interpret it relative to --exec-dir. This makes the installed binaries relocatable because the prefix is not compiled-in. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- config.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'config.c') diff --git a/config.c b/config.c index cee2d26d0c..ed96213c44 100644 --- a/config.c +++ b/config.c @@ -6,6 +6,7 @@ * */ #include "cache.h" +#include "exec_cmd.h" #define MAXNAME (256) @@ -461,7 +462,17 @@ int git_config_from_file(config_fn_t fn, const char *filename) const char *git_etc_gitconfig(void) { - return ETC_GITCONFIG; + static const char *system_wide; + if (!system_wide) { + system_wide = ETC_GITCONFIG; + if (!is_absolute_path(system_wide)) { + /* interpret path relative to exec-dir */ + const char *exec_path = git_exec_path(); + system_wide = prefix_path(exec_path, strlen(exec_path), + system_wide); + } + } + return system_wide; } int git_config(config_fn_t fn) -- cgit v1.2.3 From dcf0c16ef1a8c2e468afe686a27a5549fea59798 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 16 Nov 2007 17:05:02 -0800 Subject: core.excludesfile clean-up There are inconsistencies in the way commands currently handle the core.excludesfile configuration variable. The problem is the variable is too new to be noticed by anything other than git-add and git-status. * git-ls-files does not notice any of the "ignore" files by default, as it predates the standardized set of ignore files. The calling scripts established the convention to use .git/info/exclude, .gitignore, and later core.excludesfile. * git-add and git-status know about it because they call add_excludes_from_file() directly with their own notion of which standard set of ignore files to use. This is just a stupid duplication of code that need to be updated every time the definition of the standard set of ignore files is changed. * git-read-tree takes --exclude-per-directory=, not because the flexibility was needed. Again, this was because the option predates the standardization of the ignore files. * git-merge-recursive uses hardcoded per-directory .gitignore and nothing else. git-clean (scripted version) does not honor core.* because its call to underlying ls-files does not know about it. git-clean in C (parked in 'pu') doesn't either. We probably could change git-ls-files to use the standard set when no excludes are specified on the command line and ignore processing was asked, or something like that, but that will be a change in semantics and might break people's scripts in a subtle way. I am somewhat reluctant to make such a change. On the other hand, I think it makes perfect sense to fix git-read-tree, git-merge-recursive and git-clean to follow the same rule as other commands. I do not think of a valid use case to give an exclude-per-directory that is nonstandard to read-tree command, outside a "negative" test in the t1004 test script. This patch is the first step to untangle this mess. The next step would be to teach read-tree, merge-recursive and clean (in C) to use setup_standard_excludes(). Signed-off-by: Junio C Hamano --- config.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'config.c') diff --git a/config.c b/config.c index dc3148d456..56e99fc0f4 100644 --- a/config.c +++ b/config.c @@ -431,6 +431,13 @@ int git_default_config(const char *var, const char *value) return 0; } + if (!strcmp(var, "core.excludesfile")) { + if (!value) + die("core.excludesfile without value"); + excludes_file = xstrdup(value); + return 0; + } + /* Add other config variables here and to Documentation/config.txt. */ return 0; } -- cgit v1.2.3 From cf1b7869f0c571bbd4f72a4355d9aca558baa0da Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 6 Dec 2007 00:14:14 -0800 Subject: Use gitattributes to define per-path whitespace rule The `core.whitespace` configuration variable allows you to define what `diff` and `apply` should consider whitespace errors for all paths in the project (See gitlink:git-config[1]). This attribute gives you finer control per path. For example, if you have these in the .gitattributes: frotz whitespace nitfol -whitespace xyzzy whitespace=-trailing all types of whitespace problems known to git are noticed in path 'frotz' (i.e. diff shows them in diff.whitespace color, and apply warns about them), no whitespace problem is noticed in path 'nitfol', and the default types of whitespace problems except "trailing whitespace" are noticed for path 'xyzzy'. A project with mixed Python and C might want to have: *.c whitespace *.py whitespace=-indent-with-non-tab in its toplevel .gitattributes file. Signed-off-by: Junio C Hamano --- config.c | 50 +------------------------------------------------- 1 file changed, 1 insertion(+), 49 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index d5b976696c..2500e0d59a 100644 --- a/config.c +++ b/config.c @@ -246,54 +246,6 @@ static unsigned long get_unit_factor(const char *end) die("unknown unit: '%s'", end); } -static struct whitespace_rule { - const char *rule_name; - unsigned rule_bits; -} whitespace_rule_names[] = { - { "trailing-space", WS_TRAILING_SPACE }, - { "space-before-tab", WS_SPACE_BEFORE_TAB }, - { "indent-with-non-tab", WS_INDENT_WITH_NON_TAB }, -}; - -static unsigned parse_whitespace_rule(const char *string) -{ - unsigned rule = WS_DEFAULT_RULE; - - while (string) { - int i; - size_t len; - const char *ep; - int negated = 0; - - string = string + strspn(string, ", \t\n\r"); - ep = strchr(string, ','); - if (!ep) - len = strlen(string); - else - len = ep - string; - - if (*string == '-') { - negated = 1; - string++; - len--; - } - if (!len) - break; - for (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++) { - if (strncmp(whitespace_rule_names[i].rule_name, - string, len)) - continue; - if (negated) - rule &= ~whitespace_rule_names[i].rule_bits; - else - rule |= whitespace_rule_names[i].rule_bits; - break; - } - string = ep; - } - return rule; -} - int git_parse_long(const char *value, long *ret) { if (value && *value) { @@ -480,7 +432,7 @@ int git_default_config(const char *var, const char *value) } if (!strcmp(var, "core.whitespace")) { - whitespace_rule = parse_whitespace_rule(value); + whitespace_rule_cfg = parse_whitespace_rule(value); return 0; } -- cgit v1.2.3 From 6281f394674bf2db861967da6c2215cfc3fc78af Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sat, 8 Dec 2007 16:48:05 +0100 Subject: config.c:store_write_pair(): don't read the byte before a malloc'd buffer. Signed-off-by: Jim Meyering Signed-off-by: Junio C Hamano --- config.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 56e99fc0f4..914cfce855 100644 --- a/config.c +++ b/config.c @@ -630,13 +630,19 @@ static int store_write_pair(int fd, const char* key, const char* value) int length = strlen(key+store.baselen+1); int quote = 0; - /* Check to see if the value needs to be quoted. */ + /* + * Check to see if the value needs to be surrounded with a dq pair. + * Note that problematic characters are always backslash-quoted; this + * check is about not losing leading or trailing SP and strings that + * follow beginning-of-comment characters (i.e. ';' and '#') by the + * configuration parser. + */ if (value[0] == ' ') quote = 1; for (i = 0; value[i]; i++) if (value[i] == ';' || value[i] == '#') quote = 1; - if (value[i-1] == ' ') + if (i && value[i-1] == ' ') quote = 1; if (write_in_full(fd, "\t", 1) != 1 || -- cgit v1.2.3 From cb891a5989c41ed479db9eb4b0a69cef8bb98ea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristian=20H=C3=B8gsberg?= Date: Fri, 14 Dec 2007 15:59:58 -0500 Subject: Use a strbuf for building up section header and key/value pair strings. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoids horrible 1-byte write(2) calls and cleans up the logic a bit. Signed-off-by: Kristian Høgsberg Signed-off-by: Junio C Hamano --- config.c | 91 +++++++++++++++++++++++++++------------------------------------- 1 file changed, 38 insertions(+), 53 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 49d2b427e5..9a5c5470cc 100644 --- a/config.c +++ b/config.c @@ -610,46 +610,36 @@ static int write_error(void) static int store_write_section(int fd, const char* key) { - const char *dot = strchr(key, '.'); - int len1 = store.baselen, len2 = -1; + const char *dot; + int i, success; + struct strbuf sb; - dot = strchr(key, '.'); + strbuf_init(&sb, 0); + dot = memchr(key, '.', store.baselen); if (dot) { - int dotlen = dot - key; - if (dotlen < len1) { - len2 = len1 - dotlen - 1; - len1 = dotlen; + strbuf_addf(&sb, "[%.*s \"", (int)(dot - key), key); + for (i = dot - key + 1; i < store.baselen; i++) { + if (key[i] == '"') + strbuf_addch(&sb, '\\'); + strbuf_addch(&sb, key[i]); } + strbuf_addstr(&sb, "\"]\n"); + } else { + strbuf_addf(&sb, "[%.*s]\n", store.baselen, key); } - if (write_in_full(fd, "[", 1) != 1 || - write_in_full(fd, key, len1) != len1) - return 0; - if (len2 >= 0) { - if (write_in_full(fd, " \"", 2) != 2) - return 0; - while (--len2 >= 0) { - unsigned char c = *++dot; - if (c == '"') - if (write_in_full(fd, "\\", 1) != 1) - return 0; - if (write_in_full(fd, &c, 1) != 1) - return 0; - } - if (write_in_full(fd, "\"", 1) != 1) - return 0; - } - if (write_in_full(fd, "]\n", 2) != 2) - return 0; + success = write_in_full(fd, sb.buf, sb.len) == sb.len; + strbuf_release(&sb); - return 1; + return success; } static int store_write_pair(int fd, const char* key, const char* value) { - int i; - int length = strlen(key+store.baselen+1); - int quote = 0; + int i, success; + int length = strlen(key + store.baselen + 1); + const char *quote = ""; + struct strbuf sb; /* * Check to see if the value needs to be surrounded with a dq pair. @@ -659,43 +649,38 @@ static int store_write_pair(int fd, const char* key, const char* value) * configuration parser. */ if (value[0] == ' ') - quote = 1; + quote = "\""; for (i = 0; value[i]; i++) if (value[i] == ';' || value[i] == '#') - quote = 1; - if (i && value[i-1] == ' ') - quote = 1; + quote = "\""; + if (i && value[i - 1] == ' ') + quote = "\""; + + strbuf_init(&sb, 0); + strbuf_addf(&sb, "\t%.*s = %s", + length, key + store.baselen + 1, quote); - if (write_in_full(fd, "\t", 1) != 1 || - write_in_full(fd, key+store.baselen+1, length) != length || - write_in_full(fd, " = ", 3) != 3) - return 0; - if (quote && write_in_full(fd, "\"", 1) != 1) - return 0; for (i = 0; value[i]; i++) switch (value[i]) { case '\n': - if (write_in_full(fd, "\\n", 2) != 2) - return 0; + strbuf_addstr(&sb, "\\n"); break; case '\t': - if (write_in_full(fd, "\\t", 2) != 2) - return 0; + strbuf_addstr(&sb, "\\t"); break; case '"': case '\\': - if (write_in_full(fd, "\\", 1) != 1) - return 0; + strbuf_addch(&sb, '\\'); default: - if (write_in_full(fd, value+i, 1) != 1) - return 0; + strbuf_addch(&sb, value[i]); break; } - if (quote && write_in_full(fd, "\"", 1) != 1) - return 0; - if (write_in_full(fd, "\n", 1) != 1) - return 0; - return 1; + strbuf_addf(&sb, "%s\n", quote); + + success = write_in_full(fd, sb.buf, sb.len) == sb.len; + strbuf_release(&sb); + + return success; } static ssize_t find_beginning_of_line(const char* contents, size_t size, -- cgit v1.2.3 From c8deb5a14686c9c2d0ca1e8f42aec8ed44d16954 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Tue, 25 Dec 2007 02:18:05 -0500 Subject: Improve error messages when int/long cannot be parsed from config If a config file has become mildly corrupted due to a missing LF we may discover some other option joined up against the end of a numeric value. For example: [section] number = 1auto where the "auto" flag was meant to occur on the next line, below "number", but the missing LF has caused it to no longer be its own option. Instead the word "auto" is parsed as a 'unit factor' for the value of "number". Before this change we got the confusing error message: fatal: unknown unit: 'auto' which told us nothing about where the problem appeared. Now we get: fatal: bad config value for 'aninvalid.unit' which at least points the user in the right direction of where to search for the incorrectly formatted configuration file. Noticed by erikh on #git, which received the original error from a simple `git checkout -b` due to a midly corrupted config. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano --- config.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 9a5c5470cc..80db92990a 100644 --- a/config.c +++ b/config.c @@ -234,17 +234,23 @@ static int git_parse_file(config_fn_t fn) die("bad config file line %d in %s", config_linenr, config_file_name); } -static unsigned long get_unit_factor(const char *end) +static int parse_unit_factor(const char *end, unsigned long *val) { if (!*end) return 1; - else if (!strcasecmp(end, "k")) - return 1024; - else if (!strcasecmp(end, "m")) - return 1024 * 1024; - else if (!strcasecmp(end, "g")) - return 1024 * 1024 * 1024; - die("unknown unit: '%s'", end); + else if (!strcasecmp(end, "k")) { + *val *= 1024; + return 1; + } + else if (!strcasecmp(end, "m")) { + *val *= 1024 * 1024; + return 1; + } + else if (!strcasecmp(end, "g")) { + *val *= 1024 * 1024 * 1024; + return 1; + } + return 0; } int git_parse_long(const char *value, long *ret) @@ -252,7 +258,10 @@ int git_parse_long(const char *value, long *ret) if (value && *value) { char *end; long val = strtol(value, &end, 0); - *ret = val * get_unit_factor(end); + unsigned long factor = 1; + if (!parse_unit_factor(end, &factor)) + return 0; + *ret = val * factor; return 1; } return 0; @@ -263,7 +272,9 @@ int git_parse_ulong(const char *value, unsigned long *ret) if (value && *value) { char *end; unsigned long val = strtoul(value, &end, 0); - *ret = val * get_unit_factor(end); + if (!parse_unit_factor(end, &val)) + return 0; + *ret = val; return 1; } return 0; -- cgit v1.2.3 From 02e5ba4ae63729c28704280f1b8cfcb205c06960 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 1 Jan 2008 01:17:34 -0500 Subject: config: handle lack of newline at end of file better The config parsing routines use the static global 'config_file' to store the FILE* pointing to the current config file being parsed. The function get_next_char() automatically converts an EOF on this file to a newline for the convenience of its callers, and it sets config_file to NULL to indicate that EOF was reached. This throws away useful information, though, since some routines want to call ftell on 'config_file' to find out exactly _where_ the routine ended. In the case of a key ending at EOF boundary, we ended up segfaulting in some cases (changing that key or adding another key in its section), or failing to provide the necessary newline (adding a new section). This patch adds a new flag to indicate EOF and uses that instead of setting config_file to NULL. It also makes sure to add newlines where necessary for truncated input. All three included tests fail without the patch. Signed-off-by: Junio C Hamano --- config.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 80db92990a..857deb6c85 100644 --- a/config.c +++ b/config.c @@ -13,6 +13,7 @@ static FILE *config_file; static const char *config_file_name; static int config_linenr; +static int config_file_eof; static int zlib_compression_seen; static int get_next_char(void) @@ -34,7 +35,7 @@ static int get_next_char(void) if (c == '\n') config_linenr++; if (c == EOF) { - config_file = NULL; + config_file_eof = 1; c = '\n'; } } @@ -118,7 +119,7 @@ static int get_value(config_fn_t fn, char *name, unsigned int len) /* Get the full name */ for (;;) { c = get_next_char(); - if (c == EOF) + if (config_file_eof) break; if (!iskeychar(c)) break; @@ -182,7 +183,7 @@ static int get_base_var(char *name) for (;;) { int c = get_next_char(); - if (c == EOF) + if (config_file_eof) return -1; if (c == ']') return baselen; @@ -205,8 +206,7 @@ static int git_parse_file(config_fn_t fn) for (;;) { int c = get_next_char(); if (c == '\n') { - /* EOF? */ - if (!config_file) + if (config_file_eof) return 0; comment = 0; continue; @@ -469,6 +469,7 @@ int git_config_from_file(config_fn_t fn, const char *filename) config_file = f; config_file_name = filename; config_linenr = 1; + config_file_eof = 0; ret = git_parse_file(fn); fclose(f); config_file_name = NULL; @@ -918,6 +919,9 @@ int git_config_set_multivar(const char* key, const char* value, contents, contents_sz, store.offset[i]-2, &new_line); + if (copy_end > 0 && contents[copy_end-1] != '\n') + new_line = 1; + /* write the first part of the config */ if (copy_end > copy_begin) { if (write_in_full(fd, contents + copy_begin, -- cgit v1.2.3 From 4ed7cd3ab07f7c721daf4241fe1dac306fefd1fb Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 16 Jan 2008 13:12:46 -0600 Subject: Improve use of lockfile API Remove remaining double close(2)'s. i.e. close() before commit_locked_index() or commit_lock_file(). Signed-off-by: Junio C Hamano --- config.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 857deb6c85..526a3f4294 100644 --- a/config.c +++ b/config.c @@ -955,14 +955,12 @@ int git_config_set_multivar(const char* key, const char* value, munmap(contents, contents_sz); } - if (close(fd) || commit_lock_file(lock) < 0) { + if (commit_lock_file(lock) < 0) { fprintf(stderr, "Cannot commit config file!\n"); ret = 4; goto out_free; } - /* fd is closed, so don't try to close it below. */ - fd = -1; /* * lock is committed, so don't try to roll it back below. * NOTE: Since lockfile.c keeps a linked list of all created @@ -973,8 +971,6 @@ int git_config_set_multivar(const char* key, const char* value, ret = 0; out_free: - if (0 <= fd) - close(fd); if (lock) rollback_lock_file(lock); free(config_filename); @@ -1072,7 +1068,7 @@ int git_config_rename_section(const char *old_name, const char *new_name) } fclose(config_file); unlock_and_out: - if (close(out_fd) || commit_lock_file(lock) < 0) + if (commit_lock_file(lock) < 0) ret = error("Cannot commit config file!"); out: free(config_filename); -- cgit v1.2.3 From 7a5375395f8104a8679f9482e0c5faf60e7e9e54 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 3 Feb 2008 22:37:58 -0800 Subject: fix misuse of prefix_path() When DEFAULT_GIT_TEMPLATE_DIR is specified as a relative path, init-db made it relative to exec_path using prefix_path(), which is wrong. prefix_path() is about a file inside the work tree. There was a similar misuse in config.c that takes relative ETC_GITCONFIG path. A convenience function prefix_filename() can concatenate two paths to form a path that points at somewhere outside the work tree. Use it in these codepaths instead. Signed-off-by: Junio C Hamano --- config.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 526a3f4294..0b0c9bd050 100644 --- a/config.c +++ b/config.c @@ -485,8 +485,9 @@ const char *git_etc_gitconfig(void) if (!is_absolute_path(system_wide)) { /* interpret path relative to exec-dir */ const char *exec_path = git_exec_path(); - system_wide = prefix_path(exec_path, strlen(exec_path), - system_wide); + system_wide = strdup(prefix_filename(exec_path, + strlen(exec_path), + system_wide)); } } return system_wide; -- cgit v1.2.3 From ef5b9d6e2286630bf8afb5bdf1c6e3356f3d50c7 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Tue, 5 Feb 2008 09:17:33 +0100 Subject: Fix misuse of prefix_path() When DEFAULT_GIT_TEMPLATE_DIR is specified as a relative path, init-db made it relative to exec_path using prefix_path(), which is wrong. prefix_path() is about a file inside the work tree. There was a similar misuse in config.c that takes relative ETC_GITCONFIG path. Noticed by Junio C Hamano. We concatenate the paths manually. (prefix_filename() won't do because it expects a prefix with a trailing '/'.) Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- config.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 0b0c9bd050..498259ebef 100644 --- a/config.c +++ b/config.c @@ -484,10 +484,9 @@ const char *git_etc_gitconfig(void) system_wide = ETC_GITCONFIG; if (!is_absolute_path(system_wide)) { /* interpret path relative to exec-dir */ - const char *exec_path = git_exec_path(); - system_wide = strdup(prefix_filename(exec_path, - strlen(exec_path), - system_wide)); + struct strbuf d = STRBUF_INIT; + strbuf_addf(&d, "%s/%s", git_exec_path(), system_wide); + system_wide = strbuf_detach(&d, NULL); } } return system_wide; -- cgit v1.2.3 From 21e5ad50fc5e7277c74cfbb3cf6502468e840f86 Mon Sep 17 00:00:00 2001 From: Steffen Prohaska Date: Wed, 6 Feb 2008 12:25:58 +0100 Subject: safecrlf: Add mechanism to warn about irreversible crlf conversions CRLF conversion bears a slight chance of corrupting data. autocrlf=true will convert CRLF to LF during commit and LF to CRLF during checkout. A file that contains a mixture of LF and CRLF before the commit cannot be recreated by git. For text files this is the right thing to do: it corrects line endings such that we have only LF line endings in the repository. But for binary files that are accidentally classified as text the conversion can corrupt data. If you recognize such corruption early you can easily fix it by setting the conversion type explicitly in .gitattributes. Right after committing you still have the original file in your work tree and this file is not yet corrupted. You can explicitly tell git that this file is binary and git will handle the file appropriately. Unfortunately, the desired effect of cleaning up text files with mixed line endings and the undesired effect of corrupting binary files cannot be distinguished. In both cases CRLFs are removed in an irreversible way. For text files this is the right thing to do because CRLFs are line endings, while for binary files converting CRLFs corrupts data. This patch adds a mechanism that can either warn the user about an irreversible conversion or can even refuse to convert. The mechanism is controlled by the variable core.safecrlf, with the following values: - false: disable safecrlf mechanism - warn: warn about irreversible conversions - true: refuse irreversible conversions The default is to warn. Users are only affected by this default if core.autocrlf is set. But the current default of git is to leave core.autocrlf unset, so users will not see warnings unless they deliberately chose to activate the autocrlf mechanism. The safecrlf mechanism's details depend on the git command. The general principles when safecrlf is active (not false) are: - we warn/error out if files in the work tree can modified in an irreversible way without giving the user a chance to backup the original file. - for read-only operations that do not modify files in the work tree we do not not print annoying warnings. There are exceptions. Even though... - "git add" itself does not touch the files in the work tree, the next checkout would, so the safety triggers; - "git apply" to update a text file with a patch does touch the files in the work tree, but the operation is about text files and CRLF conversion is about fixing the line ending inconsistencies, so the safety does not trigger; - "git diff" itself does not touch the files in the work tree, it is often run to inspect the changes you intend to next "git add". To catch potential problems early, safety triggers. The concept of a safety check was originally proposed in a similar way by Linus Torvalds. Thanks to Dimitry Potapov for insisting on getting the naked LF/autocrlf=true case right. Signed-off-by: Steffen Prohaska --- config.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'config.c') diff --git a/config.c b/config.c index 498259ebef..3256c99553 100644 --- a/config.c +++ b/config.c @@ -407,6 +407,15 @@ int git_default_config(const char *var, const char *value) return 0; } + if (!strcmp(var, "core.safecrlf")) { + if (value && !strcasecmp(value, "warn")) { + safe_crlf = SAFE_CRLF_WARN; + return 0; + } + safe_crlf = git_config_bool(var, value); + return 0; + } + if (!strcmp(var, "user.name")) { strlcpy(git_default_name, value, sizeof(git_default_name)); return 0; -- cgit v1.2.3 From ab88c36321df647e17d477f19591cf6ca95de7f0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 6 Feb 2008 05:11:18 -0500 Subject: allow suppressing of global and system config The GIT_CONFIG_NOGLOBAL and GIT_CONFIG_NOSYSTEM environment variables are magic undocumented switches that can be used to ensure a totally clean environment. This is necessary for running reliable tests, since those config files may contain settings that change the outcome of tests. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 498259ebef..2f85e9d3e3 100644 --- a/config.c +++ b/config.c @@ -492,6 +492,22 @@ const char *git_etc_gitconfig(void) return system_wide; } +int git_env_bool(const char *k, int def) +{ + const char *v = getenv(k); + return v ? git_config_bool(k, v) : def; +} + +int git_config_system(void) +{ + return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0); +} + +int git_config_global(void) +{ + return !git_env_bool("GIT_CONFIG_NOGLOBAL", 0); +} + int git_config(config_fn_t fn) { int ret = 0; @@ -504,7 +520,7 @@ int git_config(config_fn_t fn) * config file otherwise. */ filename = getenv(CONFIG_ENVIRONMENT); if (!filename) { - if (!access(git_etc_gitconfig(), R_OK)) + if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) ret += git_config_from_file(fn, git_etc_gitconfig()); home = getenv("HOME"); filename = getenv(CONFIG_LOCAL_ENVIRONMENT); @@ -512,7 +528,7 @@ int git_config(config_fn_t fn) filename = repo_config = xstrdup(git_path("config")); } - if (home) { + if (git_config_global() && home) { char *user_config = xstrdup(mkpath("%s/.gitconfig", home)); if (!access(user_config, R_OK)) ret = git_config_from_file(fn, user_config); -- cgit v1.2.3 From 7a31cc0f96681d6cba54b38bb87daa2440bef448 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Mon, 11 Feb 2008 01:23:03 +0100 Subject: config: Fix --unset for continuation lines find_beginning_of_line didn't take into account that the previous line might have ended with \ in which case it shouldn't stop but continue its search. Signed-off-by: Frank Lichtenheld Acked-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- config.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'config.c') diff --git a/config.c b/config.c index 526a3f4294..e799f40db7 100644 --- a/config.c +++ b/config.c @@ -701,12 +701,17 @@ static ssize_t find_beginning_of_line(const char* contents, size_t size, size_t equal_offset = size, bracket_offset = size; ssize_t offset; +contline: for (offset = offset_-2; offset > 0 && contents[offset] != '\n'; offset--) switch (contents[offset]) { case '=': equal_offset = offset; break; case ']': bracket_offset = offset; break; } + if (offset > 0 && contents[offset-1] == '\\') { + offset_ = offset; + goto contline; + } if (bracket_offset < equal_offset) { *found_bracket = 1; offset = bracket_offset+1; -- cgit v1.2.3 From 40ea4ed9032a80c9dba706d6030bd11b08c35f4d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 11 Feb 2008 10:41:18 -0800 Subject: Add config_error_nonbool() helper function This is used to report misconfigured configuration file that does not give any value to a non-boolean variable, e.g. [section] var It is perfectly fine to say it if the section.var is a boolean (it means true), but if a variable expects a string value it should be flagged as a configuration error. Signed-off-by: Junio C Hamano --- config.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'config.c') diff --git a/config.c b/config.c index e799f40db7..03c94a2704 100644 --- a/config.c +++ b/config.c @@ -1079,3 +1079,12 @@ int git_config_rename_section(const char *old_name, const char *new_name) free(config_filename); return ret; } + +/* + * Call this to report error for your variable that should not + * get a boolean value (i.e. "[my] var" means "true"). + */ +int config_error_nonbool(const char *var) +{ + return error("Missing value for '%s'", var); +} -- cgit v1.2.3 From 6c47d0e8f3983cff5bf633cb8e6f7ecfecf48db7 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 11 Feb 2008 13:10:27 -0800 Subject: config.c: guard config parser from value=NULL user.{name,email}, core.{pager,editor,excludesfile,whitespace} and i18n.{commit,logoutput}encoding all expect string values. Signed-off-by: Junio C Hamano --- config.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'config.c') diff --git a/config.c b/config.c index 03c94a2704..3f4d3b1602 100644 --- a/config.c +++ b/config.c @@ -408,21 +408,29 @@ int git_default_config(const char *var, const char *value) } if (!strcmp(var, "user.name")) { + if (!value) + return config_error_nonbool(var); strlcpy(git_default_name, value, sizeof(git_default_name)); return 0; } if (!strcmp(var, "user.email")) { + if (!value) + return config_error_nonbool(var); strlcpy(git_default_email, value, sizeof(git_default_email)); return 0; } if (!strcmp(var, "i18n.commitencoding")) { + if (!value) + return config_error_nonbool(var); git_commit_encoding = xstrdup(value); return 0; } if (!strcmp(var, "i18n.logoutputencoding")) { + if (!value) + return config_error_nonbool(var); git_log_output_encoding = xstrdup(value); return 0; } @@ -434,23 +442,29 @@ int git_default_config(const char *var, const char *value) } if (!strcmp(var, "core.pager")) { + if (!value) + return config_error_nonbool(var); pager_program = xstrdup(value); return 0; } if (!strcmp(var, "core.editor")) { + if (!value) + return config_error_nonbool(var); editor_program = xstrdup(value); return 0; } if (!strcmp(var, "core.excludesfile")) { if (!value) - die("core.excludesfile without value"); + return config_error_nonbool(var); excludes_file = xstrdup(value); return 0; } if (!strcmp(var, "core.whitespace")) { + if (!value) + return config_error_nonbool(var); whitespace_rule_cfg = parse_whitespace_rule(value); return 0; } -- cgit v1.2.3 From ea5105a5e3c6629ee64b499ea918c2b80882fc22 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sat, 16 Feb 2008 06:00:24 +0100 Subject: config: add 'git_config_string' to refactor string config variables. In many places we just check if a value from the config file is not NULL, then we duplicate it and return 0. This patch introduces the new 'git_config_string' function to do that. This function is also used to refactor some code in 'config.c'. Refactoring other files is left for other patches. Also not all the code in "config.c" is refactored, because the function takes a "const char **" as its first parameter, but in many places a "char *" is used instead of a "const char *". (And C does not allow using a "char **" instead of a "const char **" without a warning.) Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- config.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 3f4d3b1602..7b8615d87d 100644 --- a/config.c +++ b/config.c @@ -309,6 +309,14 @@ int git_config_bool(const char *name, const char *value) return git_config_int(name, value) != 0; } +int git_config_string(const char **dest, const char *var, const char *value) +{ + if (!value) + return config_error_nonbool(var); + *dest = xstrdup(value); + return 0; +} + int git_default_config(const char *var, const char *value) { /* This needs a better name */ @@ -421,20 +429,11 @@ int git_default_config(const char *var, const char *value) return 0; } - if (!strcmp(var, "i18n.commitencoding")) { - if (!value) - return config_error_nonbool(var); - git_commit_encoding = xstrdup(value); - return 0; - } - - if (!strcmp(var, "i18n.logoutputencoding")) { - if (!value) - return config_error_nonbool(var); - git_log_output_encoding = xstrdup(value); - return 0; - } + if (!strcmp(var, "i18n.commitencoding")) + return git_config_string(&git_commit_encoding, var, value); + if (!strcmp(var, "i18n.logoutputencoding")) + return git_config_string(&git_log_output_encoding, var, value); if (!strcmp(var, "pager.color") || !strcmp(var, "color.pager")) { pager_use_color = git_config_bool(var,value); -- cgit v1.2.3 From 872da32d80c004c26a19a5d6cb31eb3e7f034094 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sat, 16 Feb 2008 06:01:11 +0100 Subject: Add "const" qualifier to "char *pager_program". Also use "git_config_string" to simplify "config.c" code where "pager_program" is set. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- config.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 7b8615d87d..eb6d2ad78b 100644 --- a/config.c +++ b/config.c @@ -440,12 +440,8 @@ int git_default_config(const char *var, const char *value) return 0; } - if (!strcmp(var, "core.pager")) { - if (!value) - return config_error_nonbool(var); - pager_program = xstrdup(value); - return 0; - } + if (!strcmp(var, "core.pager")) + return git_config_string(&pager_program, var, value); if (!strcmp(var, "core.editor")) { if (!value) -- cgit v1.2.3 From ee9601e6bef2281e3183e127e1e4e36ed257af7a Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sat, 16 Feb 2008 06:01:41 +0100 Subject: Add "const" qualifier to "char *editor_program". Also use "git_config_string" to simplify "config.c" code where "editor_program" is set. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- config.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index eb6d2ad78b..703c2085a5 100644 --- a/config.c +++ b/config.c @@ -443,12 +443,8 @@ int git_default_config(const char *var, const char *value) if (!strcmp(var, "core.pager")) return git_config_string(&pager_program, var, value); - if (!strcmp(var, "core.editor")) { - if (!value) - return config_error_nonbool(var); - editor_program = xstrdup(value); - return 0; - } + if (!strcmp(var, "core.editor")) + return git_config_string(&editor_program, var, value); if (!strcmp(var, "core.excludesfile")) { if (!value) -- cgit v1.2.3 From dfb068be8deef2065970b2a7889acc51abf4dd78 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sat, 16 Feb 2008 06:01:59 +0100 Subject: Add "const" qualifier to "char *excludes_file". Also use "git_config_string" to simplify "config.c" code where "excludes_file" is set. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- config.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 703c2085a5..b82907cb85 100644 --- a/config.c +++ b/config.c @@ -446,12 +446,8 @@ int git_default_config(const char *var, const char *value) if (!strcmp(var, "core.editor")) return git_config_string(&editor_program, var, value); - if (!strcmp(var, "core.excludesfile")) { - if (!value) - return config_error_nonbool(var); - excludes_file = xstrdup(value); - return 0; - } + if (!strcmp(var, "core.excludesfile")) + return git_config_string(&excludes_file, var, value); if (!strcmp(var, "core.whitespace")) { if (!value) -- cgit v1.2.3 From c1867cea90f8e7ee8e1be3fc07a402dde1b2666d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Feb 2008 19:00:32 -0500 Subject: git_config_*: don't assume we are parsing a config file These functions get called by other code, including parsing config options from the command line. In that case, config_file_name is NULL, leading to an ugly message or even a segfault on some implementations of printf. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'config.c') diff --git a/config.c b/config.c index 8064cae182..cba2bcfb67 100644 --- a/config.c +++ b/config.c @@ -280,11 +280,18 @@ int git_parse_ulong(const char *value, unsigned long *ret) return 0; } +static void die_bad_config(const char *name) +{ + if (config_file_name) + die("bad config value for '%s' in %s", name, config_file_name); + die("bad config value for '%s'", name); +} + int git_config_int(const char *name, const char *value) { long ret; if (!git_parse_long(value, &ret)) - die("bad config value for '%s' in %s", name, config_file_name); + die_bad_config(name); return ret; } @@ -292,7 +299,7 @@ unsigned long git_config_ulong(const char *name, const char *value) { unsigned long ret; if (!git_parse_ulong(value, &ret)) - die("bad config value for '%s' in %s", name, config_file_name); + die_bad_config(name); return ret; } -- cgit v1.2.3