git-archive and unwanted .gitattributes

git-archive and unwanted .gitattributes

[Nguyen Thai Ngoc Duy]

Hi,

Currently attr.c will read .gitattributes on disk no matter there is a
real worktree or not. This can lead to strange behavior. For example
when I do

mkdir a && cd a
git init
echo '$Format:%s$' > a
git add a && git commit -m initial
cd ..
echo 'a export-subst' > .gitattributes # let's assume this is an accident.
git --git-dir=a/.git archive --format=tar HEAD|tar xO a

I expect it to show '$Format:%s$', not "initial". git-archive should
not bother reading that .gitattributes. I thought an
is_inside_work_tree() check would be enough. Unfortunately, setting
--git-dir will automatically set worktree too. Any ideas?
-- 
Duy

Re: git-archive and unwanted .gitattributes

[Jakub Narebski]

"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:

> Currently attr.c will read .gitattributes on disk no matter there is a
> real worktree or not. 

Currently .gitattributes are read _only_ from the work tree.
There isn't even infrastructure to read .gitattributes for commit
(from a tree); git-check-attr, and I guess also internal git API,
deals only with in-tree .gitattribute file.

> This can lead to strange behavior. For example when I do
> 
> mkdir a && cd a
> git init
> echo '$Format:%s$' > a
> git add a && git commit -m initial
> cd ..
> echo 'a export-subst' > .gitattributes # let's assume this is an accident.
> git --git-dir=a/.git archive --format=tar HEAD|tar xO a
> 
> I expect it to show '$Format:%s$', not "initial". git-archive should
> not bother reading that .gitattributes. I thought an
> is_inside_work_tree() check would be enough. Unfortunately, setting
> --git-dir will automatically set worktree too. Any ideas?

And it doesn't work at all for bare repositories.

I guess that might have been caused by the fact, that .gitattributes
are similar to .gitignore file; but in this they are different.  Of
course there is chicken-and-egg problem with attributes affecting
checkout...

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: git-archive and unwanted .gitattributes

[Nguyen Thai Ngoc Duy]

On Sat, Jun 7, 2008 at 10:47 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> "Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
>
>> Currently attr.c will read .gitattributes on disk no matter there is a
>> real worktree or not.
>
> Currently .gitattributes are read _only_ from the work tree.
> There isn't even infrastructure to read .gitattributes for commit
> (from a tree); git-check-attr, and I guess also internal git API,
> deals only with in-tree .gitattribute file.

I saw it read from index but forgot that index != tree :( So
git-archive + export-subst just does not work as expected.

-- 
Duy

Re: git-archive and unwanted .gitattributes

[René Scharfe]

Jakub Narebski schrieb:
> "Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
> 
>> Currently attr.c will read .gitattributes on disk no matter there is a
>> real worktree or not. 
> 
> Currently .gitattributes are read _only_ from the work tree.
> There isn't even infrastructure to read .gitattributes for commit
> (from a tree); git-check-attr, and I guess also internal git API,
> deals only with in-tree .gitattribute file.

Attributes are taken from three places (in order of increasing
precedence): a table of built-in defaults, .gitattributes files in the
work tree, and the file $GIT_DIR/info/attributes.

René

[PATCH] fix attribute handling in bare repositories

[René Scharfe]

Nguyen Thai Ngoc Duy schrieb:
> Hi,
> 
> Currently attr.c will read .gitattributes on disk no matter there is a
> real worktree or not. This can lead to strange behavior.

Yes, it probably shouldn't do that.  What about this patch?

-- snip! --
Attributes can be specified at three different places: the internal table
of default values, the file $GIT_DIR/info/attributes and files named
.gitattributes in the work tree.  Since bare repositories don't have a
work tree, git should ignore any .gitattributes files there.

This patch makes git do that, so the only way left for a user to specify
attributes in a bare repository is the file info/attributes (in addition
to changing the defaults and recompiling).

In addition, git-check-attr is allowed to run without a work tree.  Like
any user of the code in attr.c, it simply ignores the .gitattributes
files when run in a bare repository.  And we need the command to work
there, because it's used in the tests that this patch adds.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 attr.c                |   46 +++++++++++++++++++++++++---------------------
 git.c                 |    2 +-
 t/t0003-attributes.sh |   35 +++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 22 deletions(-)

diff --git a/attr.c b/attr.c
index 1a15fad..0fb47d3 100644
--- a/attr.c
+++ b/attr.c
@@ -438,11 +438,13 @@ static void bootstrap_attr_stack(void)
 		elem->prev = attr_stack;
 		attr_stack = elem;
 
-		elem = read_attr(GITATTRIBUTES_FILE, 1);
-		elem->origin = strdup("");
-		elem->prev = attr_stack;
-		attr_stack = elem;
-		debug_push(elem);
+		if (!is_bare_repository()) {
+			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)
@@ -501,22 +503,24 @@ static void prepare_attr_stack(const char *path, int dirlen)
 	/*
 	 * Read from parent directories and push them down
 	 */
-	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(pathbuf, 0);
-		*cp = '\0';
-		elem->origin = strdup(pathbuf);
-		elem->prev = attr_stack;
-		attr_stack = elem;
-		debug_push(elem);
+	if (!is_bare_repository()) {
+		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(pathbuf, 0);
+			*cp = '\0';
+			elem->origin = strdup(pathbuf);
+			elem->prev = attr_stack;
+			attr_stack = elem;
+			debug_push(elem);
+		}
 	}
 
 	/*
diff --git a/git.c b/git.c
index 15a0e71..59f0fcc 100644
--- a/git.c
+++ b/git.c
@@ -286,7 +286,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "checkout-index", cmd_checkout_index,
 			RUN_SETUP | NEED_WORK_TREE},
 		{ "check-ref-format", cmd_check_ref_format },
-		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
+		{ "check-attr", cmd_check_attr, RUN_SETUP },
 		{ "cherry", cmd_cherry, RUN_SETUP },
 		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
 		{ "clone", cmd_clone },
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index c56d2fb..3d8e06a 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -54,4 +54,39 @@ test_expect_success 'root subdir attribute test' '
 
 '
 
+test_expect_success 'setup bare' '
+
+	git clone --bare . bare.git &&
+	cd bare.git
+
+'
+
+test_expect_success 'bare repository: check that .gitattribute is ignored' '
+
+	(
+		echo "f	test=f"
+		echo "a/i test=a/i"
+	) >.gitattributes &&
+	attr_check f unspecified &&
+	attr_check a/f unspecified &&
+	attr_check a/c/f unspecified &&
+	attr_check a/i unspecified &&
+	attr_check subdir/a/i unspecified
+
+'
+
+test_expect_success 'bare repository: test info/attributes' '
+
+	(
+		echo "f	test=f"
+		echo "a/i test=a/i"
+	) >info/attributes &&
+	attr_check f f &&
+	attr_check a/f f &&
+	attr_check a/c/f f &&
+	attr_check a/i a/i &&
+	attr_check subdir/a/i unspecified
+
+'
+
 test_done

Re: [PATCH] fix attribute handling in bare repositories

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Nguyen Thai Ngoc Duy schrieb:
>> Hi,
>> 
>> Currently attr.c will read .gitattributes on disk no matter there is a
>> real worktree or not. This can lead to strange behavior.
>
> Yes, it probably shouldn't do that.  What about this patch?

Hmm.  I do not know if it breaks anything, but if you are indeed in a bare
repository, the files the codepaths affected try to read would not exist
anyway, so I am not sure what this would fix, other than changing the
behaviour of check-attr from noticing that it was asked for nonsense and
bail out to not noticing nor saying anything useful.

Re: [PATCH] fix attribute handling in bare repositories

[René Scharfe]

Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> Nguyen Thai Ngoc Duy schrieb:
>>> Hi,
>>>
>>> Currently attr.c will read .gitattributes on disk no matter there is a
>>> real worktree or not. This can lead to strange behavior.
>> Yes, it probably shouldn't do that.  What about this patch?
> 
> Hmm.  I do not know if it breaks anything, but if you are indeed in a bare
> repository, the files the codepaths affected try to read would not exist
> anyway, so I am not sure what this would fix, other than changing the
> behaviour of check-attr from noticing that it was asked for nonsense and
> bail out to not noticing nor saying anything useful.

The patch does two things: it fixes git-check-attr to work in a bare
repository, and it makes git ignore .gitattributes files in bare
repositories.  True, the latter is fixable by simply not creating these
files in the first place.

Duy somehow ended up with them, though, and reported it as strange that
they are not ignored.  And I agree: if we don't have a work tree then we
should not look at it. :-)

René

Re: [PATCH] fix attribute handling in bare repositories

[Nguyen Thai Ngoc Duy]

On Mon, Jun 9, 2008 at 3:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
>> Nguyen Thai Ngoc Duy schrieb:
>>> Hi,
>>>
>>> Currently attr.c will read .gitattributes on disk no matter there is a
>>> real worktree or not. This can lead to strange behavior.
>>
>> Yes, it probably shouldn't do that.  What about this patch?
>
> Hmm.  I do not know if it breaks anything, but if you are indeed in a bare
> repository, the files the codepaths affected try to read would not exist
> anyway, so I am not sure what this would fix, other than changing the
> behaviour of check-attr from noticing that it was asked for nonsense and
> bail out to not noticing nor saying anything useful.

Yes. But just in case those .gitattributes exist by accident (for
example you access a bare repository from a worktree of another
repository). I don't want to check cwd for .gitattributes everytime I
want to use git-archive :)

Anyway if I do "git-archive <commit>" it should read .gitattributes
from that commit, not from working directory.
-- 
Duy