[PATCH 1/2] attr: fix attribute handling if .gitattributes is involved

From: Steffen Prohaska <prohaska@zib.de>
Date: Aug 12, 2007, 20:34 UTC
Message-ID: <11869508753328-git-send-email-prohaska@zib.de>

If we checkout .gitattributes, we must not try to parse .gitattributes. If we tried to read it, it may not be present because checkout_entry unlinks files before checkout. But we would record that no attributes are present for this directory, which is wrong. And worse, we would never try again.

This fix skips read_attr_from_file if the path triggering the read ends with .gitattributes. This is a bit more than we need, but it helps to checkout all .gitattributes before any other file, without starting the attr machinery.

This solves part of the problem of correct attributes during checkout. For example

   rm .gitattributes other
   git-checkout-index -f .gitattributes other

will work as expected, while

   rm .gitattributes other
   git-checkout-index -f other .gitattributes

will not. The problem in the second case is that .gitattributes is not yet present when it is needed.

The second case could be fixed by ordering files such that all .gitattributes are checked out before any other file. But this is perhaps too expensive and not really needed. If the user deliberately chooses to checkout .gitattributes after another file, he will not benefit from the attr machinery.

However, for checkout-index --all we should fix the problem, which is done by the next patch, building on top of this one.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 attr.c |   58 ++++++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 42 insertions(+), 16 deletions(-)

This together with the patch that follows fixes a problem, which is most likely to bite Windows users. I recognized it by setting autocrlf globally to true and doing a fresh checkout of msysgit. The checkout contained etc/termcap converted to CRLF, although is was marked as '-crlf' in etc/.gitattributes.

If we believe autocrlf is a reasonable default for Windows users, we really should use it ourselves, to find such problems.

The fixed problem is not really critical but may be quite annoying, and complex to understand.

    Steffen

diff --git a/attr.c b/attr.c
index a071254..e942f6c 100644
--- a/attr.c
+++ b/attr.c
@@ -383,6 +383,18 @@ static void bootstrap_attr_stack(void)
 	}
 }
 
+static int ends_with_gitattributes (const char* path)
+{
+	int attributes_len = strlen (GITATTRIBUTES_FILE);
+	int path_len = strlen (path);
+	if (path_len >= attributes_len
+	     && strcmp (path + path_len - attributes_len, GITATTRIBUTES_FILE) == 0)
+	{
+		return 1;
+	}
+	return 0;
+}
+
 static void prepare_attr_stack(const char *path, int dirlen)
 {
 	struct attr_stack *elem, *info;
@@ -430,23 +442,37 @@ static void prepare_attr_stack(const char *path, int dirlen)
 
 	/*
 	 * Read from parent directories and push them down
+	 *
+	 * But don't try to read if path ends with .gitattributes.
+	 * In this case we could fail and record no attributes for
+	 * a directory. We better wait and see if we need the
+	 * attributes later.
+	 *
+	 * We skip all .gitattributes, even in higher directories.
+	 * Thus, we can checkout all .gitattributes in any order
+	 * before the attr machinery starts to work. .gitattributes
+	 * should not be controlled by .gitattributes in the working
+	 * tree anyway.
+	 *
 	 */
-	while (1) {
-		char *cp;
-
-		len = strlen(attr_stack->origin);
-		if (dirlen <= len)
-			break;
-		memcpy(pathbuf, path, dirlen);
-		memcpy(pathbuf + dirlen, "/", 2);
-		cp = strchr(pathbuf + len + 1, '/');
-		strcpy(cp + 1, GITATTRIBUTES_FILE);
-		elem = read_attr_from_file(pathbuf, 0);
-		*cp = '\0';
-		elem->origin = strdup(pathbuf);
-		elem->prev = attr_stack;
-		attr_stack = elem;
-		debug_push(elem);
+	if (!ends_with_gitattributes (path)) {
+		while (1) {
+			char *cp;
+
+			len = strlen(attr_stack->origin);
+			if (dirlen <= len)
+				break;
+			memcpy(pathbuf, path, dirlen);
+			memcpy(pathbuf + dirlen, "/", 2);
+			cp = strchr(pathbuf + len + 1, '/');
+			strcpy(cp + 1, GITATTRIBUTES_FILE);
+			elem = read_attr_from_file(pathbuf, 0);
+			*cp = '\0';
+			elem->origin = strdup(pathbuf);
+			elem->prev = attr_stack;
+			attr_stack = elem;
+			debug_push(elem);
+		}
 	}
 
 	/*

-- 
1.5.3.rc4.96.g6ceb