René Scharfe <rene.scharfe@xxxxxxxxxxxxxx> writes:
> Junio C Hamano schrieb:
> ...
>> 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.
> 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".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html