Honoring a checked out gitattributes file

14 messages, 5 authors, 2016-06-15 · open the first message on its own page

Honoring a checked out gitattributes file

From: Kristian Amlie <hidden>
Date: 2016-06-15 22:46:03

Hi!

We currently use msysGit in our company test farm to checkout the latest
development branch and do autotests. However, we have one problem:
Certain files require UNIX line endings, even though this is a Windows
system. For this we use .gitattributes.

However, if the .gitattributes file is also checked in to the branch, it
will not always be honored. I browsed the code a bit, and it seems to
happen whenever there is an existing .gitattributes file, but the
checkout adds new files to it. These new files will not get the correct
line endings (although I'm not sure if it happens *every* time, it could
depend on the order they are checked out).

I think this should be fairly straightforward to fix, by ensuring that
if there is a file called .gitattributes in the index of the current
directory, check it out first, before all the other files that may be
affected by it. I can produce a patch and testcase for it, but I wanted
to hear the opinions of some developers in case there is an obvious flaw
in my solution.

What do you think?

--
Kristian Amlie

Re: Honoring a checked out gitattributes file

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:46:03

Kristian Amlie venit, vidit, dixit 28.01.2009 16:25:
Hi!

We currently use msysGit in our company test farm to checkout the latest
development branch and do autotests. However, we have one problem:
Certain files require UNIX line endings, even though this is a Windows
system. For this we use .gitattributes.

However, if the .gitattributes file is also checked in to the branch, it
will not always be honored. I browsed the code a bit, and it seems to
happen whenever there is an existing .gitattributes file, but the
checkout adds new files to it. These new files will not get the correct
line endings (although I'm not sure if it happens *every* time, it could
depend on the order they are checked out).

I think this should be fairly straightforward to fix, by ensuring that
if there is a file called .gitattributes in the index of the current
directory, check it out first, before all the other files that may be
affected by it. I can produce a patch and testcase for it, but I wanted
to hear the opinions of some developers in case there is an obvious flaw
in my solution.

What do you think?
I think there's a general time ordering problem. Say you do the
following commits:

A: add a.txt
B: add a .gitattributes file covering *.txt (say with crlf or any filter)
C: add c.txt

Now, with an empty dir, you do either

1) checkout A, B, C sequentially
2) checkout C

The contents of the checkout will be different in cases 1) and 2):
1) a.txt is checked out out as is, c.txt according to the attributes
2) with current git: probably like 1), with your suggestion: both a.txt
and c.txt filtered according to the attributes.

If you add a file and .gitattributes covering it in the same commit
there is an ordering ambiguity which can be solved (patched away)
easily, but I think the difference between 1) and 2) is still
problematic, and would need to be dealt with.

The main problem seems to be that changing a file like gitattributes can
potentially change (by changing filters) the contents which should be
stored for a different file even if the checkout of that file doesn't
change.

Cheers,
Michael

Re: Honoring a checked out gitattributes file

From: Kristian Amlie <hidden>
Date: 2016-06-15 22:46:03

Michael J Gruber wrote:
I think there's a general time ordering problem. Say you do the
following commits:

A: add a.txt
B: add a .gitattributes file covering *.txt (say with crlf or any filter)
C: add c.txt

Now, with an empty dir, you do either

1) checkout A, B, C sequentially
2) checkout C

The contents of the checkout will be different in cases 1) and 2):
1) a.txt is checked out out as is, c.txt according to the attributes
2) with current git: probably like 1), with your suggestion: both a.txt
and c.txt filtered according to the attributes.

If you add a file and .gitattributes covering it in the same commit
there is an ordering ambiguity which can be solved (patched away)
easily, but I think the difference between 1) and 2) is still
problematic, and would need to be dealt with.
I agree.
The main problem seems to be that changing a file like gitattributes can
potentially change (by changing filters) the contents which should be
stored for a different file even if the checkout of that file doesn't
change.
Yes, that is a problem. Ideally, the crlf attribute would be tied to the
file entry itself rather than a separate file (so changing the attribute
would mean a change to the file), but I guess we are stuck with what we
have.

I still think that case 2) is the most common, and fixing it has the
appealing property that if the repository line endings are broken for
some reason (because of case 1 or something else), then recloning or
checking out from scratch is guaranteed to bring the working tree into
the "correct" state.

Since fixing both cases is a pretty big task and fixing only case 2 is
small, I propose that we go ahead with that.

--
Kristian Amlie

Re: Honoring a checked out gitattributes file

From: Jeff King <hidden>
Date: 2016-06-15 22:46:03

On Wed, Jan 28, 2009 at 04:25:37PM +0100, Kristian Amlie wrote:
However, if the .gitattributes file is also checked in to the branch, it
will not always be honored. I browsed the code a bit, and it seems to
happen whenever there is an existing .gitattributes file, but the
checkout adds new files to it. These new files will not get the correct
line endings (although I'm not sure if it happens *every* time, it could
depend on the order they are checked out).
This is a known limitation of gitattributes. There has been some
discussion in the past on how it should work, but I don't recall the
specifics; try searching the list archive. I think it is really just
waiting for somebody to step up and write some patches.

As a workaround, you might be able to do something like:

  branch=master
  git show $branch:.gitattributes >.git/info/attributes
  git checkout $branch

which is very hacky, but might work depending on your setup. Notably it
will overwrite any actual use you were making of .git/info/attributes,
and it will not respect any .gitattributes files in subdirectories.

-Peff

[PATCH] Add a test for checking whether gitattributes is honored by checkout.

From: Kristian Amlie <hidden>
Date: 2016-06-15 22:46:04

The original bug will not honor new entries in gitattributes if they
are changed in the same checkout as the files they affect.
---
 t/t2012-checkout-crlf.sh |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)
 create mode 100755 t/t2012-checkout-crlf.sh
diff --git a/t/t2012-checkout-crlf.sh b/t/t2012-checkout-crlf.sh
new file mode 100755
index 0000000..cb997a8
--- /dev/null
+++ b/t/t2012-checkout-crlf.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+test_description='checkout should honor .gitattributes'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+
+	git config core.autocrlf true &&
+	printf "dummy dummy=true\r\n" > .gitattributes &&
+	git add .gitattributes &&
+	git commit -m initial &&
+	printf "file -crlf\n" >> .gitattributes &&
+	printf "contents\n" > file &&
+	git add .gitattributes file &&
+	git commit -m second
+
+'
+
+test_expect_success 'checkout with existing .gitattributes' '
+
+	git checkout master~1 &&
+	git checkout master &&
+	test "$(git diff-files --raw)" = ""
+
+'
+
+test_done
+
-- 
1.6.0.3

Re: Honoring a checked out gitattributes file

From: Kristian Amlie <hidden>
Date: 2016-06-15 22:46:04

I have now created a test case which demonstrates the problem.

I want to create a patch for it too, but I am unfortunately not so
familiar with the Git source code.

To put it short, I want to make sure that when checking out a tree,
.gitattributes will be checked out before all other files, so that
files that are affected by it will be guaranteed to get the correct
attributes. Maybe someone from this list could point me in the right
direction in the source code for something like this?

--
Kristian Amlie

Re: Honoring a checked out gitattributes file

From: Kristian Amlie <hidden>
Date: 2016-06-15 22:46:22

Ok, it took a bit longer than I thought to get around to this, but I
finally have a patch for the problem. I am not so familiar with the
Git source code, so I apologize if I do something incredibly stupid.

Hopefully the commit message should explain what I do, but please ask
questions if it's not clear. I'm sure there's room for improvements.

--
Kristian

[PATCH 1/2] Add a test for checking whether gitattributes is honored by checkout.

From: Kristian Amlie <hidden>
Date: 2016-06-15 22:46:22

The original bug will not honor new entries in gitattributes if they
are changed in the same checkout as the files they affect.
---
 t/t2013-checkout-crlf.sh |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)
 create mode 100755 t/t2013-checkout-crlf.sh
diff --git a/t/t2013-checkout-crlf.sh b/t/t2013-checkout-crlf.sh
new file mode 100755
index 0000000..cb997a8
--- /dev/null
+++ b/t/t2013-checkout-crlf.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+test_description='checkout should honor .gitattributes'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+
+	git config core.autocrlf true &&
+	printf "dummy dummy=true\r\n" > .gitattributes &&
+	git add .gitattributes &&
+	git commit -m initial &&
+	printf "file -crlf\n" >> .gitattributes &&
+	printf "contents\n" > file &&
+	git add .gitattributes file &&
+	git commit -m second
+
+'
+
+test_expect_success 'checkout with existing .gitattributes' '
+
+	git checkout master~1 &&
+	git checkout master &&
+	test "$(git diff-files --raw)" = ""
+
+'
+
+test_done
+
-- 
1.6.2.105.g16bc7.dirty

[PATCH 2/2] Make Git respect changes to .gitattributes during checkout.

From: Kristian Amlie <hidden>
Date: 2016-06-15 22:46:22

We do this by popping off elements on the attribute stack, until we
reach the level where a new .gitattributes was checked out. The next
time someone calls git_checkattr(), it will reconstruct the
attributes from that point.
---
 attr.c  |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 attr.h  |    1 +
 entry.c |   23 ++++++++++++++++++++
 3 files changed, 84 insertions(+), 12 deletions(-)
diff --git a/attr.c b/attr.c
index 17f6a4d..7deb51a 100644
--- a/attr.c
+++ b/attr.c
@@ -455,6 +455,23 @@ static void bootstrap_attr_stack(void)
 	}
 }
 
+static void pop_attr_stack(const char *path, int dirlen)
+{
+	struct attr_stack *elem;
+	while (attr_stack && attr_stack->origin) {
+		int namelen = strlen(attr_stack->origin);
+
+		elem = attr_stack;
+		if (namelen <= dirlen &&
+			!strncmp(elem->origin, path, namelen))
+			break;
+
+		debug_pop(elem);
+		attr_stack = elem->prev;
+		free_attr_elem(elem);
+	}
+}
+
 static void prepare_attr_stack(const char *path, int dirlen)
 {
 	struct attr_stack *elem, *info;
@@ -489,18 +506,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) {
-		int namelen = strlen(attr_stack->origin);
-
-		elem = attr_stack;
-		if (namelen <= dirlen &&
-		    !strncmp(elem->origin, path, namelen))
-			break;
-
-		debug_pop(elem);
-		attr_stack = elem->prev;
-		free_attr_elem(elem);
-	}
+	pop_attr_stack(path, dirlen);
 
 	/*
 	 * Read from parent directories and push them down
@@ -642,3 +648,45 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)
 
 	return 0;
 }
+
+void git_attr_invalidate_path(const char *path)
+{
+	int dirlen;
+	const char *cp;
+	struct attr_stack *info, *elem;
+
+	bootstrap_attr_stack();
+
+	/*
+	 * Pop the "info" one that is always at the top of the stack.
+	 */
+	info = attr_stack;
+	attr_stack = info->prev;
+
+	cp = strrchr(path, '/');
+	dirlen = cp ? cp - path : 0;
+	/* Pop everything up to, and including, path. */
+	pop_attr_stack(path, dirlen);
+
+	if (!strcmp(path, "") && attr_stack->origin && !strcmp(attr_stack->origin, "")) {
+		/* Special handling when the root attributes must be invalidated. */
+		elem = attr_stack;
+		debug_pop(elem);
+		attr_stack = elem->prev;
+		free_attr_elem(elem);
+
+		if (!is_bare_repository()) {
+			elem = read_attr(GITATTRIBUTES_FILE, 1);
+			elem->origin = strdup("");
+			elem->prev = attr_stack;
+			attr_stack = elem;
+			debug_push(elem);
+		}
+	}
+
+	/*
+	 * Finally push the "info" one at the top of the stack.
+	 */
+	info->prev = attr_stack;
+	attr_stack = info;
+}
diff --git a/attr.h b/attr.h
index f1c2038..8f4135b 100644
--- a/attr.h
+++ b/attr.h
@@ -30,5 +30,6 @@ struct git_attr_check {
 };
 
 int git_checkattr(const char *path, int, struct git_attr_check *);
+void git_attr_invalidate_path(const char *path);
 
 #endif /* ATTR_H */
diff --git a/entry.c b/entry.c
index 05aa58d..121c979 100644
--- a/entry.c
+++ b/entry.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "blob.h"
 #include "dir.h"
+#include "attr.h"
 
 static void create_directories(const char *path, const struct checkout *state)
 {
@@ -91,6 +92,9 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout
 {
 	int fd;
 	long wrote;
+	int gitattrlen;
+	int pathlen;
+	char *inv_path;
 
 	switch (ce->ce_mode & S_IFMT) {
 		char *new;
@@ -171,6 +175,25 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout
 		return error("git checkout-index: unknown file mode for %s", path);
 	}
 
+	gitattrlen = strlen(GITATTRIBUTES_FILE);
+	pathlen = strlen(path);
+	if (!strncmp(path + pathlen - gitattrlen, GITATTRIBUTES_FILE, gitattrlen)) {
+		/* Invalidate attributes if a new .gitattributes file was checked out. */
+		inv_path = strrchr(path, '/');
+		if (!inv_path) {
+			pathlen = 0;
+			inv_path = xmalloc(1);
+			*inv_path = '\0';
+		} else {
+			pathlen = inv_path - path;
+			inv_path = xmalloc(pathlen + 1);
+			strncpy(inv_path, path, pathlen);
+			inv_path[pathlen] = '\0';
+		}
+		git_attr_invalidate_path(inv_path);
+		free(inv_path);
+	}
+
 	if (state->refresh_cache) {
 		struct stat st;
 		lstat(ce->name, &st);
-- 
1.6.2.105.g16bc7.dirty

Re: [PATCH 2/2] Make Git respect changes to .gitattributes during checkout.

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:46:22

Kristian Amlie schrieb:
We do this by popping off elements on the attribute stack, until we
reach the level where a new .gitattributes was checked out. The next
time someone calls git_checkattr(), it will reconstruct the
attributes from that point.
...
+	gitattrlen = strlen(GITATTRIBUTES_FILE);
+	pathlen = strlen(path);
+	if (!strncmp(path + pathlen - gitattrlen, GITATTRIBUTES_FILE, gitattrlen)) {
+		/* Invalidate attributes if a new .gitattributes file was checked out. */
But if there was file .abc.txt that was checked out before .gitattributes
was checked out, then the new .gitattributes won't have been used for
.abc.txt, yet, right?

(Disclaimer: I didn't read the code, just your description and this comment.)

-- Hannes

Re: [PATCH 2/2] Make Git respect changes to .gitattributes during checkout.

From: Kristian Amlie <hidden>
Date: 2016-06-15 22:46:22

ext Johannes Sixt wrote:
Kristian Amlie schrieb:
quoted
We do this by popping off elements on the attribute stack, until we
reach the level where a new .gitattributes was checked out. The next
time someone calls git_checkattr(), it will reconstruct the
attributes from that point.
...
quoted
+	gitattrlen = strlen(GITATTRIBUTES_FILE);
+	pathlen = strlen(path);
+	if (!strncmp(path + pathlen - gitattrlen, GITATTRIBUTES_FILE, gitattrlen)) {
+		/* Invalidate attributes if a new .gitattributes file was checked out. */
But if there was file .abc.txt that was checked out before .gitattributes
was checked out, then the new .gitattributes won't have been used for
.abc.txt, yet, right?
You're right, that fails.

I'm guessing my initial patch (and testcase) works because the sorting
is alphabetical and .gitattributes happens to be checked out before
files with letters. I would need to make sure that .gitattributes gets
checked out first.

--
Kristian

Re: Honoring a checked out gitattributes file

From: Kristian Amlie <hidden>
Date: 2016-06-15 22:46:23

Ok, here's another round. I fixed the test case to expect failure, as
pointed out by Matthieu, and I also added code to make sure that
.gitattributes gets checked out first.

I also added a test case and some code to support the case where
.gitattributes is removed in a commit, but this doesn't work properly
yet (see commit message). I'm not sure how to solve this without
resolving to ugly hacks like passing the new index_state* all the way
down to where git_checkattr gets called. If anybody has any
suggestions, do share.

The main usecase where .gitattributes is modified, is anyway covered
by this patch.

--
Kristian

[PATCH 2/2] Make Git respect changes to .gitattributes during checkout.

From: Kristian Amlie <hidden>
Date: 2016-06-15 22:46:23

The fix is twofold:

First, we force .gitattributes files to always be the first ones
checked out. This is the part in check_updates().

Second, we make sure that the checked out attributes get honored by
popping off elements on the attribute stack, until we reach the level
where a new .gitattributes was checked out. The next time someone
calls git_checkattr(), it will reconstruct the attributes from that
point.

Note that in unlink_entry() there is code to support the case where
.gitattributes is removed (test case #3 in t2013-checkout-crlf.sh),
but this doesn't work properly because Git tries to read new
attributes from the index, but the old index (where .gitattributes
still exists) is still active.
---
 attr.c                   |   86 +++++++++++++++++++++++++++++++++++++++------
 attr.h                   |    1 +
 entry.c                  |   19 ++++++++++
 t/t2013-checkout-crlf.sh |    2 +-
 unpack-trees.c           |   77 +++++++++++++++++++++++++++++++----------
 5 files changed, 154 insertions(+), 31 deletions(-)
diff --git a/attr.c b/attr.c
index 17f6a4d..2bc0847 100644
--- a/attr.c
+++ b/attr.c
@@ -455,6 +455,23 @@ static void bootstrap_attr_stack(void)
 	}
 }
 
+static void pop_attr_stack(const char *path, int dirlen)
+{
+	struct attr_stack *elem;
+	while (attr_stack && attr_stack->origin) {
+		int namelen = strlen(attr_stack->origin);
+
+		elem = attr_stack;
+		if (namelen <= dirlen &&
+			!strncmp(elem->origin, path, namelen))
+			break;
+
+		debug_pop(elem);
+		attr_stack = elem->prev;
+		free_attr_elem(elem);
+	}
+}
+
 static void prepare_attr_stack(const char *path, int dirlen)
 {
 	struct attr_stack *elem, *info;
@@ -489,18 +506,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) {
-		int namelen = strlen(attr_stack->origin);
-
-		elem = attr_stack;
-		if (namelen <= dirlen &&
-		    !strncmp(elem->origin, path, namelen))
-			break;
-
-		debug_pop(elem);
-		attr_stack = elem->prev;
-		free_attr_elem(elem);
-	}
+	pop_attr_stack(path, dirlen);
 
 	/*
 	 * Read from parent directories and push them down
@@ -642,3 +648,59 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)
 
 	return 0;
 }
+
+void git_attr_invalidate_path(const char *path)
+{
+	int dirlen;
+	const char *cp;
+	struct attr_stack *info, *elem;
+
+	bootstrap_attr_stack();
+
+	/*
+	 * Pop the "info" one that is always at the top of the stack.
+	 */
+	info = attr_stack;
+	attr_stack = info->prev;
+
+	cp = strrchr(path, '/');
+	dirlen = cp ? cp - path : 0;
+	/* Pop everything up to, and including, path. */
+	pop_attr_stack(path, dirlen);
+
+	if (!strcmp(path, "") && attr_stack->origin && !strcmp(attr_stack->origin, "")) {
+		/*
+		 * Special handling when the root attributes must be invalidated.
+		 * Note that we cannot reread the attributes here. The reason is that
+		 * when unlinking the root .gitattributes file, the index entry is
+		 * removed after unlinking. If we reread the attributes here, we
+		 * would get the same attributes as before because .gitattributes
+		 * is still in the index. Thus we must wait until the next call to
+		 * read_attr, when the index has been updated.
+		 */
+		/* $GIT_DIR/info/attributes */
+		elem = info;
+		debug_pop(elem);
+		free_attr_elem(elem);
+
+		/* $GIT_WORK_TREE/.gitattributes */
+		elem = attr_stack;
+		debug_pop(elem);
+		attr_stack = elem->prev;
+		free_attr_elem(elem);
+
+		/* builtins */
+		elem = attr_stack;
+		debug_pop(elem);
+		attr_stack = elem->prev;
+		free_attr_elem(elem);
+
+		assert(!attr_stack);
+	} else {
+		/*
+		 * Finally push the "info" one at the top of the stack.
+		 */
+		info->prev = attr_stack;
+		attr_stack = info;
+	}
+}
diff --git a/attr.h b/attr.h
index f1c2038..8f4135b 100644
--- a/attr.h
+++ b/attr.h
@@ -30,5 +30,6 @@ struct git_attr_check {
 };
 
 int git_checkattr(const char *path, int, struct git_attr_check *);
+void git_attr_invalidate_path(const char *path);
 
 #endif /* ATTR_H */
diff --git a/entry.c b/entry.c
index 05aa58d..7fb27fd 100644
--- a/entry.c
+++ b/entry.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "blob.h"
 #include "dir.h"
+#include "attr.h"
 
 static void create_directories(const char *path, const struct checkout *state)
 {
@@ -91,6 +92,9 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout
 {
 	int fd;
 	long wrote;
+	int gitattrlen;
+	int pathlen;
+	char *cp;
 
 	switch (ce->ce_mode & S_IFMT) {
 		char *new;
@@ -171,6 +175,21 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout
 		return error("git checkout-index: unknown file mode for %s", path);
 	}
 
+	gitattrlen = strlen(GITATTRIBUTES_FILE);
+	pathlen = strlen(path);
+	if (pathlen >= gitattrlen && !strncmp(path + pathlen - gitattrlen,
+				GITATTRIBUTES_FILE, gitattrlen)) {
+		/* Invalidate attributes if a new .gitattributes file was checked out. */
+		cp = strrchr(path, '/');
+		if (!cp) {
+			git_attr_invalidate_path("");
+		} else {
+			*cp = '\0';
+			git_attr_invalidate_path(path);
+			*cp = '/';
+		}
+	}
+
 	if (state->refresh_cache) {
 		struct stat st;
 		lstat(ce->name, &st);
diff --git a/t/t2013-checkout-crlf.sh b/t/t2013-checkout-crlf.sh
index d9d6465..e704c93 100755
--- a/t/t2013-checkout-crlf.sh
+++ b/t/t2013-checkout-crlf.sh
@@ -21,7 +21,7 @@ test_expect_success 'setup' '
 
 '
 
-test_expect_failure 'checkout with existing .gitattributes' '
+test_expect_success 'checkout with existing .gitattributes' '
 
         git checkout master~2 &&
         git checkout master~1 &&
diff --git a/unpack-trees.c b/unpack-trees.c
index e547282..63a41f8 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -7,6 +7,7 @@
 #include "unpack-trees.h"
 #include "progress.h"
 #include "refs.h"
+#include "attr.h"
 
 /*
  * Error messages expected by scripts out of plumbing commands such as
@@ -60,6 +61,7 @@ static void unlink_entry(struct cache_entry *ce)
 {
 	char *cp, *prev;
 	char *name = ce->name;
+	int namelen, gitattrlen;
 
 	if (has_symlink_or_noent_leading_path(ce_namelen(ce), ce->name))
 		return;
@@ -82,6 +84,21 @@ static void unlink_entry(struct cache_entry *ce)
 		}
 		prev = cp;
 	}
+
+	if (!prev) {
+		gitattrlen = strlen(GITATTRIBUTES_FILE);
+		namelen = strlen(name);
+		if (namelen >= gitattrlen && !strncmp(name + namelen - gitattrlen,
+				GITATTRIBUTES_FILE, gitattrlen)) {
+			if (cp) {
+				*cp = '\0';
+				git_attr_invalidate_path(name);
+				*cp = '/';
+			} else {
+				git_attr_invalidate_path("");
+			}
+		}
+	}
 }
 
 static struct checkout state;
@@ -92,6 +109,9 @@ static int check_updates(struct unpack_trees_options *o)
 	struct index_state *index = &o->result;
 	int i;
 	int errs = 0;
+	int attr;
+	int gitattrlen;
+	int namelen;
 
 	if (o->update && o->verbose_update) {
 		for (total = cnt = 0; cnt < index->cache_nr; cnt++) {
@@ -105,27 +125,48 @@ static int check_updates(struct unpack_trees_options *o)
 		cnt = 0;
 	}
 
-	for (i = 0; i < index->cache_nr; i++) {
-		struct cache_entry *ce = index->cache[i];
+	gitattrlen = strlen(GITATTRIBUTES_FILE);
 
-		if (ce->ce_flags & CE_REMOVE) {
-			display_progress(progress, ++cnt);
-			if (o->update)
-				unlink_entry(ce);
-			remove_index_entry_at(&o->result, i);
-			i--;
-			continue;
+	/*
+	 * We want to checkout .gitattributes before everything else. So:
+	 * attr == 0 -> .gitattributes
+	 * attr == 1 -> everything else
+	 */
+	for (attr = 0; attr < 2; attr++) {
+		for (i = 0; i < index->cache_nr; i++) {
+			struct cache_entry *ce = index->cache[i];
+
+			namelen = strlen(ce->name);
+			if ((attr == 0) != (namelen >= gitattrlen && strncmp(
+					ce->name + namelen - gitattrlen,
+					GITATTRIBUTES_FILE, gitattrlen) == 0))
+				continue;
+			if (ce->ce_flags & CE_REMOVE) {
+				display_progress(progress, ++cnt);
+				if (o->update)
+					unlink_entry(ce);
+				remove_index_entry_at(&o->result, i);
+				i--;
+				continue;
+			}
 		}
 	}
 
-	for (i = 0; i < index->cache_nr; i++) {
-		struct cache_entry *ce = index->cache[i];
+	for (attr = 0; attr < 2; attr++) {
+		for (i = 0; i < index->cache_nr; i++) {
+			struct cache_entry *ce = index->cache[i];
 
-		if (ce->ce_flags & CE_UPDATE) {
-			display_progress(progress, ++cnt);
-			ce->ce_flags &= ~CE_UPDATE;
-			if (o->update) {
-				errs |= checkout_entry(ce, &state, NULL);
+			namelen = strlen(ce->name);
+			if ((attr == 0) != (namelen >= gitattrlen && strncmp(
+					ce->name + namelen - gitattrlen,
+					GITATTRIBUTES_FILE, gitattrlen) == 0))
+				continue;
+			if (ce->ce_flags & CE_UPDATE) {
+				display_progress(progress, ++cnt);
+				ce->ce_flags &= ~CE_UPDATE;
+				if (o->update) {
+					errs |= checkout_entry(ce, &state, NULL);
+				}
 			}
 		}
 	}
-- 
1.6.1.1

[PATCH 1/2] Add a test for checking whether gitattributes is honored by checkout.

From: Kristian Amlie <hidden>
Date: 2016-06-15 22:46:23

The original bug will not honor new entries in gitattributes if they
are changed in the same checkout as the files they affect.

It will also keep using .gitattributes, even if it is deleted in the
same commit as the files it affects.
---
 t/t2013-checkout-crlf.sh |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)
 create mode 100755 t/t2013-checkout-crlf.sh
diff --git a/t/t2013-checkout-crlf.sh b/t/t2013-checkout-crlf.sh
new file mode 100755
index 0000000..d9d6465
--- /dev/null
+++ b/t/t2013-checkout-crlf.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='checkout should honor .gitattributes'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+
+        git config core.autocrlf true &&
+        printf ".file2 -crlf\r\n" > .gitattributes &&
+        git add .gitattributes &&
+        git commit -m initial &&
+        printf ".file -crlf\n" >> .gitattributes &&
+        printf "contents\n" > .file &&
+        git add .gitattributes .file &&
+        git commit -m second &&
+        git rm .gitattributes &&
+        printf "contents\r\n" > .file2 &&
+        git add .file2 &&
+        git commit -m third
+
+'
+
+test_expect_failure 'checkout with existing .gitattributes' '
+
+        git checkout master~2 &&
+        git checkout master~1 &&
+        test "$(git diff-files --raw)" = ""
+
+'
+
+test_expect_failure 'checkout when deleting .gitattributes' '
+
+        git checkout master~1 &&
+        git checkout master &&
+        test "$(stat -c %s .file2)" = "10"
+
+'
+
+test_done
+
-- 
1.6.1.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help