Re: [PATCH v2 3/3] archive: do not read .gitattributes in working directory
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:36
René Scharfe [off-list ref] writes:
Junio C Hamano schrieb: ...quoted
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)?
Actually, the "mostly re-indent" patch breaks things for normal users that expect the attributes never kicks in when you are doing things in a bare repository as-is.
quoted hunk
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();
This looks _conceptually_ wrong. I think your patch came from the fact that check_updates() unconditionally calls git_attr_set_direction() without checking o->update, and I think it is a bug in check_updates(). A bare repository is by definition without a work tree, and we shouldn't be reading the index in general. I wouldn't go as far to say that a bare repository should not have the index file, because people often clone forgetting the --bare option and manually convert that to a bare repository, and they forget to remove the index that is never used. ... thinks a bit more ... I think codepaths that make calls to git_attr_set_direction() inside is_bare_repository() are special already. If we were to teach check-attr how to check attributes for paths _inside a given tree-ish_, most likely the implementation would be similar to what Duy did for archive; read the tree into in-core index, set direction to the INDEX, and start using the attribute mechanism. So I think we'd rather not have the patch to force GIT_ATTR_INDEX in set_direction(); if anything, the patch should say something like "if we are in a bare repository and the new direction is not INDEX, it is a programming error". Instead, such specialized codepaths should call set_direction() itself, perhaps after reading the tree-ish into the in-core index. And we should fix the "mostly re-indent" patch not to remove the conditional, but make the conditional to check "If in a bare repository and the direction is not explicitly set to INDEX, do not use the attributes".