Re: attr.c doesn't honor --work-tree option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6 February 2014 18:54, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Lasse Makholm <lasse.makholm@xxxxxxxxx> writes:
>
>> Here's a repro with -DDEBUG_ATTR=1 and a printf() in read_attr_from_file():
>>
>> $ cd /tmp/
>> $ mkdir -p attr-test/repo
>> $ cd attr-test/repo
>> $ git init
>> Initialized empty Git repository in /tmp/attr-test/repo/.git/
>> $ echo 'dir/* filter=foo' >.gitattributes
>> $
>>
>> Inside the working tree, it works:
>>
>> $ ~/src/git.git/git check-attr -a dir/file
>
> Does check-ignore misbehave the same way?

No, check-ignore works but also has NEED_WORK_TREE set. And that
actually also feels a bit wrong to me because check-attr and
check-ignore both seem like reasonable things to do in a bare repo
because .git(attributes|ignore) files are likely to be committed in
the repo.

> I suspect that is this because check-attr is not a command that
> requires a working tree.  The command was written primarily as a
> debugging aid that can be used anywhere as long as you have a
> repository to read strings from either its standard input or its
> arguments, and gives them directly to check_attr(), but it does so
> without first going to the top of the real working tree like
> check-ignore does.

Fair point. I actually stumbled across this because a git cat-file
--textconv ... was failing, so that's at least one other (and arguably
more real) use case that is broken in the same way.

> Forcing it to go to the top of the working tree (see the attached
> one-liner, but note that I didn't test it) may give you want you
> want.

For this case, it does, yes. But it also breaks check-attr in bare
repos with attributes defined in $GIT_DIR/info/attributes because it
will refuse to run without a work tree...

In any case the current state seems broken because --work-tree clearly
doesn't work for all commands...

Setting NEED_WORK_TREE for check-attr risks breaking existing scripts
but on the other hand there doesn't seem to be any good reason why
check-attr and check-ignore should differ in this regard...

It seems like the ideal solution would be an optional NEED_WORK_TREE
of some sort that would let these commands work correctly both with
--work-tree, without it and in bare repos but I get that that might
not be easy to fix...

Another approach might be to deprecate --work-tree and tell people to
use -C instead...

/L

>  git.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git.c b/git.c
> index 7cf2953..314ec9f 100644
> --- a/git.c
> +++ b/git.c
> @@ -342,7 +342,7 @@ static struct cmd_struct commands[] = {
>         { "branch", cmd_branch, RUN_SETUP },
>         { "bundle", cmd_bundle, RUN_SETUP_GENTLY },
>         { "cat-file", cmd_cat_file, RUN_SETUP },
> -       { "check-attr", cmd_check_attr, RUN_SETUP },
> +       { "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
>         { "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
>         { "check-mailmap", cmd_check_mailmap, RUN_SETUP },
>         { "check-ref-format", cmd_check_ref_format },
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]