From 1afca444953639fef53f3e5520202971505d77b9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 10 Jan 2012 13:08:21 -0500 Subject: [PATCH 1/6] attr: don't confuse prefixes with leading directories When we prepare the attribute stack for a lookup on a path, we start with the cached stack from the previous lookup (because it is common to do several lookups in the same directory hierarchy). So the first thing we must do in preparing the stack is to pop any entries that point to directories we are no longer interested in. For example, if our stack contains gitattributes for: foo/bar/baz foo/bar foo but we want to do a lookup in "foo/bar/bleep", then we want to pop the top element, but retain the others. To do this we walk down the stack from the top, popping elements that do not match our lookup directory. However, the test do this simply checked strncmp, meaning we would mistake "foo/bar/baz" as a leading directory of "foo/bar/baz_plus". We must also check that the character after our match is '/', meaning we matched the whole path component. There are two special cases to consider: 1. The top of our attr stack has the empty path. So we must not check for '/', but rather special-case the empty path, which always matches. 2. Typically when matching paths in this way, you would also need to check for a full string match (i.e., the character after is '\0'). We don't need to do so in this case, though, because our path string is actually just the directory component of the path to a file (i.e., we know that it terminates with "/", because the filename comes after that). Helped-by: Michael Haggerty Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- attr.c | 3 ++- t/t0003-attributes.sh | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/attr.c b/attr.c index f6b3f7e850..924b4408d5 100644 --- a/attr.c +++ b/attr.c @@ -573,7 +573,8 @@ static void prepare_attr_stack(const char *path, int dirlen) elem = attr_stack; if (namelen <= dirlen && - !strncmp(elem->origin, path, namelen)) + !strncmp(elem->origin, path, namelen) && + (!namelen || path[namelen] == '/')) break; debug_pop(elem); diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index ebbc7554a7..61b5a2eba6 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -60,6 +60,16 @@ test_expect_success 'attribute test' ' ' +test_expect_success 'prefixes are not confused with leading directories' ' + attr_check a_plus/g unspecified && + cat >expect <<-\EOF && + a/g: test: a/g + a_plus/g: test: unspecified + EOF + git check-attr test a/g a_plus/g >actual && + test_cmp expect actual +' + test_expect_success 'core.attributesfile' ' attr_check global unspecified && git config core.attributesfile "$HOME/global-gitattributes" && From 77f7f822889411bfede0507dd60aebf11edb2849 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 10 Jan 2012 14:32:06 -0500 Subject: [PATCH 2/6] attr: drop misguided defensive coding In prepare_attr_stack, we pop the old elements of the stack (which were left from a previous lookup and may or may not be useful to us). Our loop to do so checks that we never reach the top of the stack. However, the code immediately afterwards will segfault if we did actually reach the top of the stack. Fortunately, this is not an actual bug, since we will never pop all of the stack elements (we will always keep the root gitattributes, as well as the builtin ones). So the extra check in the loop condition simply clutters the code and makes the intent less clear. Let's get rid of it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/attr.c b/attr.c index 924b4408d5..ad7eb9cd31 100644 --- a/attr.c +++ b/attr.c @@ -568,7 +568,7 @@ static void prepare_attr_stack(const char *path, int dirlen) * Pop the ones from directories that are not the prefix of * the path we are checking. */ - while (attr_stack && attr_stack->origin) { + while (attr_stack->origin) { int namelen = strlen(attr_stack->origin); elem = attr_stack; From 909ca7b9ac11711478aaa5dd4ab17a0d1dabe075 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 10 Jan 2012 12:27:37 -0800 Subject: [PATCH 3/6] attr.c: make bootstrap_attr_stack() leave early Thas would de-dent the body of a function that has grown rather large over time, making it a bit easier to read. Signed-off-by: Junio C Hamano --- attr.c | 65 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/attr.c b/attr.c index ad7eb9cd31..a6e6523d29 100644 --- a/attr.c +++ b/attr.c @@ -488,48 +488,49 @@ static int git_attr_config(const char *var, const char *value, void *dummy) static void bootstrap_attr_stack(void) { - if (!attr_stack) { - struct attr_stack *elem; + struct attr_stack *elem; - elem = read_attr_from_array(builtin_attr); - elem->origin = NULL; - elem->prev = attr_stack; - attr_stack = elem; + if (attr_stack) + return; - if (git_attr_system()) { - elem = read_attr_from_file(git_etc_gitattributes(), 1); - if (elem) { - elem->origin = NULL; - elem->prev = attr_stack; - attr_stack = elem; - } - } + elem = read_attr_from_array(builtin_attr); + elem->origin = NULL; + elem->prev = attr_stack; + attr_stack = elem; - git_config(git_attr_config, NULL); - if (attributes_file) { - elem = read_attr_from_file(attributes_file, 1); - if (elem) { - elem->origin = NULL; - elem->prev = attr_stack; - attr_stack = elem; - } - } - - if (!is_bare_repository() || direction == GIT_ATTR_INDEX) { - elem = read_attr(GITATTRIBUTES_FILE, 1); - elem->origin = strdup(""); + if (git_attr_system()) { + elem = read_attr_from_file(git_etc_gitattributes(), 1); + if (elem) { + elem->origin = NULL; elem->prev = attr_stack; attr_stack = elem; - debug_push(elem); } + } - elem = read_attr_from_file(git_path(INFOATTRIBUTES_FILE), 1); - if (!elem) - elem = xcalloc(1, sizeof(*elem)); - elem->origin = NULL; + git_config(git_attr_config, NULL); + if (attributes_file) { + elem = read_attr_from_file(attributes_file, 1); + if (elem) { + elem->origin = NULL; + elem->prev = attr_stack; + attr_stack = elem; + } + } + + if (!is_bare_repository() || direction == GIT_ATTR_INDEX) { + elem = read_attr(GITATTRIBUTES_FILE, 1); + elem->origin = strdup(""); elem->prev = attr_stack; attr_stack = elem; + debug_push(elem); } + + elem = read_attr_from_file(git_path(INFOATTRIBUTES_FILE), 1); + if (!elem) + elem = xcalloc(1, sizeof(*elem)); + elem->origin = NULL; + elem->prev = attr_stack; + attr_stack = elem; } static void prepare_attr_stack(const char *path, int dirlen) From c432ef996e28a2f02369c505b841f86ec30c85fb Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 10 Jan 2012 12:28:38 -0800 Subject: [PATCH 4/6] attr.c: clarify the logic to pop attr_stack Signed-off-by: Junio C Hamano --- attr.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/attr.c b/attr.c index a6e6523d29..2ce7365138 100644 --- a/attr.c +++ b/attr.c @@ -567,7 +567,9 @@ static void prepare_attr_stack(const char *path, int dirlen) /* * Pop the ones from directories that are not the prefix of - * the path we are checking. + * the path we are checking. Break out of the loop when we see + * the root one (whose origin is an empty string "") or the builtin + * one (whose origin is NULL) without popping it. */ while (attr_stack->origin) { int namelen = strlen(attr_stack->origin); @@ -587,6 +589,13 @@ static void prepare_attr_stack(const char *path, int dirlen) * Read from parent directories and push them down */ if (!is_bare_repository() || direction == GIT_ATTR_INDEX) { + /* + * bootstrap_attr_stack() should have added, and the + * above loop should have stopped before popping, the + * root element whose attr_stack->origin is set to an + * empty string. + */ + assert(attr_stack->origin); while (1) { char *cp; From b6fb7fed6afca3376e4cdd70c9fe9737bcd73a6e Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 10 Jan 2012 15:57:27 +0100 Subject: [PATCH 5/6] Documentation: rerere's rr-cache auto-creation and rerere.enabled The description of rerere.enabled left the user in the dark as to who might create an rr-cache directory. Add a note that simply invoking rerere does this. Signed-off-by: Junio C Hamano --- Documentation/config.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 18ab1ae65e..27b57d226b 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1703,7 +1703,8 @@ rerere.enabled:: conflict hunks can be resolved automatically, should they be encountered again. By default, linkgit:git-rerere[1] is enabled if there is an `rr-cache` directory under the - `$GIT_DIR`. + `$GIT_DIR`, e.g. if "rerere" was previously used in the + repository. sendemail.identity:: A configuration identity. When given, causes values in the From f14f9803ef13e972371e3f4bce69bca13dd1cd2d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 10 Jan 2012 13:11:03 -0800 Subject: [PATCH 6/6] Prepare for 1.7.6.6 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/1.7.6.6.txt | 11 +++++++++++ RelNotes | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 Documentation/RelNotes/1.7.6.6.txt diff --git a/Documentation/RelNotes/1.7.6.6.txt b/Documentation/RelNotes/1.7.6.6.txt new file mode 100644 index 0000000000..13ce2dc2d7 --- /dev/null +++ b/Documentation/RelNotes/1.7.6.6.txt @@ -0,0 +1,11 @@ +Git v1.7.6.6 Release Notes +========================== + +Fixes since v1.7.6.5 +-------------------- + + * The code to look up attributes for paths reused entries from a wrong + directory when two paths in question are in adjacent directories and + the name of the one directory is a prefix of the other. + +Also contains minor fixes and documentation updates. diff --git a/RelNotes b/RelNotes index 08cae90a05..fb9320b14b 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/1.7.6.5.txt \ No newline at end of file +Documentation/RelNotes/1.7.6.6.txt \ No newline at end of file