From b8a9733377a648483958bf5472158f7c01641420 Mon Sep 17 00:00:00 2001 From: Ramsay Jones Date: Thu, 16 Jun 2011 21:22:16 +0100 Subject: [PATCH 1/3] help.c: Fix detection of custom merge strategy on cygwin Test t7606-merge-custom.sh fails on cygwin when git-merge fails with an "Could not find merge strategy 'theirs'" error, despite the test correctly preparing an (executable) git-merge-theirs script. The cause of the failure is the mis-detection of the executable status of the script, by the is_executable() function, while the load_command_list() function is searching the path for additional merge strategy programs. Note that the l/stat() "functions" on cygwin are somewhat schizophrenic (see commits adbc0b6, 7faee6b and 7974843), and their behaviour depends on the timing of various git setup and config function calls. In particular, until the "git_dir" has been set (have_git_dir() returns true), the real cygwin (POSIX emulating) l/stat() functions are called. Once "git_dir" has been set, the "native Win32 API" implementations of l/stat() may, or may not, be called depending on the setting of the core.filemode and core.ignorecygwinfstricks config variables. We also note that, since commit c869753, core.filemode is forced to false, even on NTFS, by git-init and git-clone. A user (or a test) can, of course, reset core.filemode to true explicitly if the filesystem supports it (and he doesn't use any problematic windows software). The test-suite currently runs all tests on cygwin with core.filemode set to false. Given the above, we see that the built-in merge strategies are correctly detected as executable, since they are checked for before "git_dir" is set, whereas all custom merge strategies are not, since they are checked for after "git_dir" is set. In order to fix the mis-detection problem, we change the code in is_executable() to re-use the conditional WIN32 code section, which actually looks at the content of the file to determine if the file is executable. On cygwin we also make the additional code conditional on the executable bit of the file mode returned by the initial stat() call. (only the real cygwin function would set the executable bit in the file mode.) Signed-off-by: Ramsay Jones Signed-off-by: Junio C Hamano --- help.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/help.c b/help.c index 7654f1bfd6..e925ca1f89 100644 --- a/help.c +++ b/help.c @@ -127,7 +127,10 @@ static int is_executable(const char *name) !S_ISREG(st.st_mode)) return 0; -#ifdef WIN32 +#if defined(WIN32) || defined(__CYGWIN__) +#if defined(__CYGWIN__) +if ((st.st_mode & S_IXUSR) == 0) +#endif { /* cannot trust the executable bit, peek into the file instead */ char buf[3] = { 0 }; int n; From 452993c297530498780d4b0c7b2b267258cb625f Mon Sep 17 00:00:00 2001 From: Ramsay Jones Date: Thu, 16 Jun 2011 21:23:14 +0100 Subject: [PATCH 2/3] t1301-*.sh: Fix the 'forced modes' test on cygwin The 'forced modes' test fails on cygwin because the post-update hook loses it's executable bit when copied from the templates directory by git-init. The template loses it's executable bit because the lstat() function resolves to the "native Win32 API" implementation. This call to lstat() happens after git-init has set the "git_dir" (so has_git_dir() returns true), but before the configuration has been fully initialised. At this point git_config() does not find any config files to parse and returns 0. Unfortunately, the code used to determine the cygwin l/stat() function bindings did not check the return from git_config() and assumed that the config was complete and accessible once "git_dir" was set. In order to fix the test, we simply change the binding code to test the return value from git_config(), to ensure that it actually had config values to read, before determining the requested binding. Signed-off-by: Ramsay Jones Signed-off-by: Junio C Hamano --- compat/cygwin.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compat/cygwin.c b/compat/cygwin.c index b4a51b958c..b38dbd7f8f 100644 --- a/compat/cygwin.c +++ b/compat/cygwin.c @@ -114,8 +114,7 @@ static int git_cygwin_config(const char *var, const char *value, void *cb) static int init_stat(void) { - if (have_git_dir()) { - git_config(git_cygwin_config, NULL); + if (have_git_dir() && git_config(git_cygwin_config,NULL)) { if (!core_filemode && native_stat) { cygwin_stat_fn = cygwin_stat; cygwin_lstat_fn = cygwin_lstat; From 924aaf3ef764a5e8e976f68e024ecacf54ff6306 Mon Sep 17 00:00:00 2001 From: Ramsay Jones Date: Thu, 16 Jun 2011 21:24:51 +0100 Subject: [PATCH 3/3] config.c: Make git_config() work correctly when called recursively On Cygwin, this fixes a test failure in t3301-notes.sh (test 98, "git notes copy --for-rewrite (disabled)"). The test failure is caused by a recursive call to git_config() which has the effect of skipping to the end-of-file while processing the "notes.rewriteref" config variable. Thus, any config variables that appear after "notes.rewriteref" are simply ignored by git_config(). Also, we note that the original FILE handle is leaked as a result of the recursive call. The recursive call to git_config() is due to the "schizophrenic stat" functions on cygwin, where one of two different implementations of the l/stat functions is selected lazily, depending on some config variables. In this case, the init_copy_notes_for_rewrite() function calls git_config() with the notes_rewrite_config() callback function. This callback, while processing the "notes.rewriteref" variable, in turn calls string_list_add_refs_by_glob() to process the associated ref value. This eventually leads to a call to the get_ref_dir() function, which in turn calls stat(). On cygwin, the stat() macro leads to an indirect call to cygwin_stat_stub() which, via init_stat(), then calls git_config() in order to determine which l/stat implementation to bind to. In order to solve this problem, we modify git_config() so that the global state variables used by the config reading code is packaged up and managed on a local state stack. Signed-off-by: Ramsay Jones Signed-off-by: Junio C Hamano --- config.c | 80 +++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 30 deletions(-) diff --git a/config.c b/config.c index e0b3b80d92..1fc063b256 100644 --- a/config.c +++ b/config.c @@ -12,10 +12,18 @@ #define MAXNAME (256) -static FILE *config_file; -static const char *config_file_name; -static int config_linenr; -static int config_file_eof; +typedef struct config_file { + struct config_file *prev; + FILE *f; + const char *name; + int linenr; + int eof; + struct strbuf value; + char var[MAXNAME]; +} config_file; + +static config_file *cf; + static int zlib_compression_seen; const char *config_exclusive_filename = NULL; @@ -99,7 +107,7 @@ static int get_next_char(void) FILE *f; c = '\n'; - if ((f = config_file) != NULL) { + if (cf && ((f = cf->f) != NULL)) { c = fgetc(f); if (c == '\r') { /* DOS like systems */ @@ -110,9 +118,9 @@ static int get_next_char(void) } } if (c == '\n') - config_linenr++; + cf->linenr++; if (c == EOF) { - config_file_eof = 1; + cf->eof = 1; c = '\n'; } } @@ -121,21 +129,20 @@ static int get_next_char(void) static char *parse_value(void) { - static struct strbuf value = STRBUF_INIT; int quote = 0, comment = 0, space = 0; - strbuf_reset(&value); + strbuf_reset(&cf->value); for (;;) { int c = get_next_char(); if (c == '\n') { if (quote) return NULL; - return value.buf; + return cf->value.buf; } if (comment) continue; if (isspace(c) && !quote) { - if (value.len) + if (cf->value.len) space++; continue; } @@ -146,7 +153,7 @@ static char *parse_value(void) } } for (; space; space--) - strbuf_addch(&value, ' '); + strbuf_addch(&cf->value, ' '); if (c == '\\') { c = get_next_char(); switch (c) { @@ -168,14 +175,14 @@ static char *parse_value(void) default: return NULL; } - strbuf_addch(&value, c); + strbuf_addch(&cf->value, c); continue; } if (c == '"') { quote = 1-quote; continue; } - strbuf_addch(&value, c); + strbuf_addch(&cf->value, c); } } @@ -192,7 +199,7 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len) /* Get the full name */ for (;;) { c = get_next_char(); - if (config_file_eof) + if (cf->eof) break; if (!iskeychar(c)) break; @@ -256,7 +263,7 @@ static int get_base_var(char *name) for (;;) { int c = get_next_char(); - if (config_file_eof) + if (cf->eof) return -1; if (c == ']') return baselen; @@ -274,7 +281,7 @@ static int git_parse_file(config_fn_t fn, void *data) { int comment = 0; int baselen = 0; - static char var[MAXNAME]; + char *var = cf->var; /* U+FEFF Byte Order Mark in UTF8 */ static const unsigned char *utf8_bom = (unsigned char *) "\xef\xbb\xbf"; @@ -298,7 +305,7 @@ static int git_parse_file(config_fn_t fn, void *data) } } if (c == '\n') { - if (config_file_eof) + if (cf->eof) return 0; comment = 0; continue; @@ -323,7 +330,7 @@ static int git_parse_file(config_fn_t fn, void *data) if (get_value(fn, data, var, baselen+1) < 0) break; } - die("bad config file line %d in %s", config_linenr, config_file_name); + die("bad config file line %d in %s", cf->linenr, cf->name); } static int parse_unit_factor(const char *end, unsigned long *val) @@ -374,8 +381,8 @@ int git_parse_ulong(const char *value, unsigned long *ret) static void die_bad_config(const char *name) { - if (config_file_name) - die("bad config value for '%s' in %s", name, config_file_name); + if (cf && cf->name) + die("bad config value for '%s' in %s", name, cf->name); die("bad config value for '%s'", name); } @@ -795,13 +802,24 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data) ret = -1; if (f) { - config_file = f; - config_file_name = filename; - config_linenr = 1; - config_file_eof = 0; + config_file top; + + /* push config-file parsing state stack */ + top.prev = cf; + top.f = f; + top.name = filename; + top.linenr = 1; + top.eof = 0; + strbuf_init(&top.value, 1024); + cf = ⊤ + ret = git_parse_file(fn, data); + + /* pop config-file parsing state stack */ + strbuf_release(&top.value); + cf = top.prev; + fclose(f); - config_file_name = NULL; } return ret; } @@ -909,6 +927,7 @@ static int store_aux(const char *key, const char *value, void *cb) { const char *ep; size_t section_len; + FILE *f = cf->f; switch (store.state) { case KEY_SEEN: @@ -920,7 +939,7 @@ static int store_aux(const char *key, const char *value, void *cb) return 1; } - store.offset[store.seen] = ftell(config_file); + store.offset[store.seen] = ftell(f); store.seen++; } break; @@ -947,19 +966,19 @@ static int store_aux(const char *key, const char *value, void *cb) * Do not increment matches: this is no match, but we * just made sure we are in the desired section. */ - store.offset[store.seen] = ftell(config_file); + store.offset[store.seen] = ftell(f); /* fallthru */ case SECTION_END_SEEN: case START: if (matches(key, value)) { - store.offset[store.seen] = ftell(config_file); + store.offset[store.seen] = ftell(f); store.state = KEY_SEEN; store.seen++; } else { if (strrchr(key, '.') - key == store.baselen && !strncmp(key, store.key, store.baselen)) { store.state = SECTION_SEEN; - store.offset[store.seen] = ftell(config_file); + store.offset[store.seen] = ftell(f); } } } @@ -1415,6 +1434,7 @@ int git_config_rename_section(const char *old_name, const char *new_name) struct lock_file *lock = xcalloc(sizeof(struct lock_file), 1); int out_fd; char buf[1024]; + FILE *config_file; if (config_exclusive_filename) config_filename = xstrdup(config_exclusive_filename);