Re: [PATCH v2 3/3] archive: do not read .gitattributes in working directory

René Scharfe [off-list ref] writes:

René Scharfe schrieb:

quoted

quoted

I
think it makes sense to create a separate script for the new tests and
eventually move the existing archive attribute tests there.

Something like this?

I forgot to add tests against bare repositories.  Otherwise I'd noticed
earlier that read_attr() is only called for non-bare repositories
currently, i.e. your patches won't allow reading of .gitattribute files
from the tree in bare repos.

Curious.

Shouldn't the call chain look like:

    write_archive()
    ->write_archive_entries()
      ->unpack_trees() to read the tree into the in-core index
      ->git_attr_set_direction()
      ->read_tree_recursive()
        ->write_archive_entry()
          ->git_checkattr()

Ah, bootstrap_attr_stack() and prepare_attr_stack() still assume that you
won't be doing any per-level attributes in a bare repository because the
concept of attributes is inherently tied to having a work tree from their
point of view.

How about this "mostly re-indent with four line removal" patch?

 attr.c |   48 ++++++++++++++++++++++--------------------------
 1 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/attr.c b/attr.c
index 37ca288..f5917de 100644
--- a/attr.c
+++ b/attr.c

@@ -468,13 +468,11 @@ static void bootstrap_attr_stack(void)
 		elem->prev = attr_stack;
 		attr_stack = 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(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)

@@ -535,25 +533,23 @@ static void prepare_attr_stack(const char *path, int dirlen)
 	/*
 	 * Read from parent directories and push them down
 	 */
-	if (!is_bare_repository()) {
-		while (1) {
-			char *cp;
-
-			len = strlen(attr_stack->origin);
-			if (dirlen <= len)
-				break;
-			strbuf_reset(&pathbuf);
-			strbuf_add(&pathbuf, path, dirlen);
-			strbuf_addch(&pathbuf, '/');
-			cp = strchr(pathbuf.buf + len + 1, '/');
-			strcpy(cp + 1, GITATTRIBUTES_FILE);
-			elem = read_attr(pathbuf.buf, 0);
-			*cp = '\0';
-			elem->origin = strdup(pathbuf.buf);
-			elem->prev = attr_stack;
-			attr_stack = elem;
-			debug_push(elem);
-		}
+	while (1) {
+		char *cp;
+
+		len = strlen(attr_stack->origin);
+		if (dirlen <= len)
+			break;
+		strbuf_reset(&pathbuf);
+		strbuf_add(&pathbuf, path, dirlen);
+		strbuf_addch(&pathbuf, '/');
+		cp = strchr(pathbuf.buf + len + 1, '/');
+		strcpy(cp + 1, GITATTRIBUTES_FILE);
+		elem = read_attr(pathbuf.buf, 0);
+		*cp = '\0';
+		elem->origin = strdup(pathbuf.buf);
+		elem->prev = attr_stack;
+		attr_stack = elem;
+		debug_push(elem);
 	}
 
 	/*

Junio C Hamano schrieb:

René Scharfe [off-list ref] writes:

quoted

René Scharfe schrieb:

quoted

quoted

I
think it makes sense to create a separate script for the new tests and
eventually move the existing archive attribute tests there.

Something like this?

I forgot to add tests against bare repositories.  Otherwise I'd noticed
earlier that read_attr() is only called for non-bare repositories
currently, i.e. your patches won't allow reading of .gitattribute files
from the tree in bare repos.

Curious.

Shouldn't the call chain look like:

    write_archive()
    ->write_archive_entries()
      ->unpack_trees() to read the tree into the in-core index
      ->git_attr_set_direction()
      ->read_tree_recursive()
        ->write_archive_entry()
          ->git_checkattr()

Ah, bootstrap_attr_stack() and prepare_attr_stack() still assume that you
won't be doing any per-level attributes in a bare repository because the
concept of attributes is inherently tied to having a work tree from their
point of view.

How about this "mostly re-indent with four line removal" patch?

Plus the following (on top of Duy's GIT_ATTR_INDEX patch)?

diff --git a/attr.c b/attr.c
index f5917de..cce561b 100644
--- a/attr.c
+++ b/attr.c

@@ -672,6 +672,8 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)
 void git_attr_set_direction(enum git_attr_direction new, struct index_state *istate)
 {
 	enum git_attr_direction old = direction;
+	if (is_bare_repository())
+		new = GIT_ATTR_INDEX;
 	direction = new;
 	if (new != old)
 		drop_attr_stack();