{"thread":{"id":"35813","subject":"attr.c doesn't honor --work-tree option","startedAt":"2014-02-06T12:48:43Z","lastAt":"2014-02-16T11:15:22Z","messageCount":13,"participants":["Lasse Makholm","Junio C Hamano","Jonathan Nieder","Michael Haggerty"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"234363","messageId":"CAB7pA0_2kLJmRZb8Z_9+xOcrbwMZLgnZwC5f7etbE4OuJ1wgmA@mail.gmail.com","threadId":"35813","inReplyTo":null,"subject":"attr.c doesn't honor --work-tree option","fromName":"Lasse Makholm","fromEmail":"lasse.makholm@gmail.com","sentAt":"2014-02-06T12:48:43Z","receivedAt":"2014-02-06T12:48:43Z","isPatch":false,"sender":{"key":"lasse.makholm@gmail.com","avatar":"https://gravatar.com/avatar/5bc3a34e4fa8ba26b38c333f370b15c034351e18e09b056a2608f3904ae1a4c1?d=mp&s=160"},"body":"Hi,\n\nIt seems that code in attr.c does not honor the current work tree path\n(set by e.g. --work-tree ...) and simply always assumes CWD. When the\ncurrent dir is not in the work tree, git will attempt to find\n.gitattributes under ./ instead of under the correct work tree.\n\nHere's a repro with -DDEBUG_ATTR=1 and a printf() in read_attr_from_file():\n\n$ cd /tmp/\n$ mkdir -p attr-test/repo\n$ cd attr-test/repo\n$ git init\nInitialized empty Git repository in /tmp/attr-test/repo/.git/\n$ echo 'dir/* filter=foo' >.gitattributes\n$\n\nInside the working tree, it works:\n\n$ ~/src/git.git/git check-attr -a dir/file\nread_attr_from_file: /home/lasse/etc/gitattributes\nread_attr_from_file: /home/lasse/.config/git/attributes\nread_attr_from_file: .gitattributes\npush:\nread_attr_from_file: .git/info/attributes\nread_attr_from_file: dir/.gitattributes\npush: dir\nfill: filter => foo (dir/*)\ndir/file: filter: foo\n$\n\nOutside, it fails to find the .gitattributes file:\n\n$ cd ..\n$ ~/src/git.git/git --work-tree /tmp/attr-test/repo --git-dir\n/tmp/attr-test/repo/.git check-attr -a dir/file\nread_attr_from_file: /home/lasse/etc/gitattributes\nread_attr_from_file: /home/lasse/.config/git/attributes\nread_attr_from_file: .gitattributes\npush:\nread_attr_from_file: /tmp/attr-test/repo/.git/info/attributes\nread_attr_from_file: dir/.gitattributes\npush: dir\n$\n\nThis is with the latest rev on master:\n\n$ ~/src/git.git/git --version\ngit version 1.8.5.2.192.g7794a68.dirty\n$\n\nIt (sort of) works with a committed .gitattributes file because git\nwill find it in the index, but that will still yield incorrect results\nif the .gitattributes file happens to be dirty.\n\nLooking at the code, I'm not really sure if this can be fixed in\nread_attr_from_file() by resolving relative paths against\nget_git_work_tree(). I doubt it's that simple though...\n\nThoughts?\n\n/Lasse\n"},{"id":"234378","messageId":"xmqqvbwsyvn8.fsf@gitster.dls.corp.google.com","threadId":"35813","inReplyTo":"CAB7pA0_2kLJmRZb8Z_9+xOcrbwMZLgnZwC5f7etbE4OuJ1wgmA@mail.gmail.com","subject":"Re: attr.c doesn't honor --work-tree option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-06T17:54:19Z","receivedAt":"2014-02-06T17:54:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lasse Makholm <lasse.makholm@gmail.com> writes:\n\n> Here's a repro with -DDEBUG_ATTR=1 and a printf() in read_attr_from_file():\n>\n> $ cd /tmp/\n> $ mkdir -p attr-test/repo\n> $ cd attr-test/repo\n> $ git init\n> Initialized empty Git repository in /tmp/attr-test/repo/.git/\n> $ echo 'dir/* filter=foo' >.gitattributes\n> $\n>\n> Inside the working tree, it works:\n>\n> $ ~/src/git.git/git check-attr -a dir/file\n\nDoes check-ignore misbehave the same way?\n\nI suspect that is this because check-attr is not a command that\nrequires a working tree.  The command was written primarily as a\ndebugging aid that can be used anywhere as long as you have a\nrepository to read strings from either its standard input or its\narguments, and gives them directly to check_attr(), but it does so\nwithout first going to the top of the real working tree like\ncheck-ignore does.\n\nForcing it to go to the top of the working tree (see the attached\none-liner, but note that I didn't test it) may give you want you\nwant.\n\n git.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git.c b/git.c\nindex 7cf2953..314ec9f 100644\n--- a/git.c\n+++ b/git.c\n@@ -342,7 +342,7 @@ static struct cmd_struct commands[] = {\n \t{ \"branch\", cmd_branch, RUN_SETUP },\n \t{ \"bundle\", cmd_bundle, RUN_SETUP_GENTLY },\n \t{ \"cat-file\", cmd_cat_file, RUN_SETUP },\n-\t{ \"check-attr\", cmd_check_attr, RUN_SETUP },\n+\t{ \"check-attr\", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"check-ignore\", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"check-mailmap\", cmd_check_mailmap, RUN_SETUP },\n \t{ \"check-ref-format\", cmd_check_ref_format },\n"},{"id":"234380","messageId":"1391712033-9443-1-git-send-email-gitster@pobox.com","threadId":"35813","inReplyTo":"xmqqvbwsyvn8.fsf@gitster.dls.corp.google.com","subject":"[PATCH 1/2] t0003: do not chdir the whole test process","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-06T18:40:32Z","receivedAt":"2014-02-06T18:40:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Moving to some other directory and letting the remainder of the test\npieces to expect that they start there is a bad practice.  The test\nthat contains chdir itself may fail (or by mistake skipped via the\nGIT_SKIP_TESTS mechanism) in which case the remainder may operate on\nfiles in unexpected places.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This is purely a preparatory clean-up in the test script I'll be\n   adding a new test to in the next patch.\n\n t/t0003-attributes.sh | 52 +++++++++++++++++++++++++++++----------------------\n 1 file changed, 30 insertions(+), 22 deletions(-)\n\ndiff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh\nindex febc45c..0554b13 100755\n--- a/t/t0003-attributes.sh\n+++ b/t/t0003-attributes.sh\n@@ -197,39 +197,47 @@ test_expect_success 'root subdir attribute test' '\n '\n \n test_expect_success 'setup bare' '\n-\tgit clone --bare . bare.git &&\n-\tcd bare.git\n+\tgit clone --bare . bare.git\n '\n \n test_expect_success 'bare repository: check that .gitattribute is ignored' '\n \t(\n-\t\techo \"f\ttest=f\"\n-\t\techo \"a/i test=a/i\"\n-\t) >.gitattributes &&\n-\tattr_check f unspecified &&\n-\tattr_check a/f unspecified &&\n-\tattr_check a/c/f unspecified &&\n-\tattr_check a/i unspecified &&\n-\tattr_check subdir/a/i unspecified\n+\t\tcd bare.git &&\n+\t\t(\n+\t\t\techo \"f\ttest=f\"\n+\t\t\techo \"a/i test=a/i\"\n+\t\t) >.gitattributes &&\n+\t\tattr_check f unspecified &&\n+\t\tattr_check a/f unspecified &&\n+\t\tattr_check a/c/f unspecified &&\n+\t\tattr_check a/i unspecified &&\n+\t\tattr_check subdir/a/i unspecified\n+\t)\n '\n \n test_expect_success 'bare repository: check that --cached honors index' '\n-\tGIT_INDEX_FILE=../.git/index \\\n-\tgit check-attr --cached --stdin --all <../stdin-all |\n-\tsort >actual &&\n-\ttest_cmp ../specified-all actual\n+\t(\n+\t\tcd bare.git &&\n+\t\tGIT_INDEX_FILE=../.git/index \\\n+\t\tgit check-attr --cached --stdin --all <../stdin-all |\n+\t\tsort >actual &&\n+\t\ttest_cmp ../specified-all actual\n+\t)\n '\n \n test_expect_success 'bare repository: test info/attributes' '\n \t(\n-\t\techo \"f\ttest=f\"\n-\t\techo \"a/i test=a/i\"\n-\t) >info/attributes &&\n-\tattr_check f f &&\n-\tattr_check a/f f &&\n-\tattr_check a/c/f f &&\n-\tattr_check a/i a/i &&\n-\tattr_check subdir/a/i unspecified\n+\t\tcd bare.git &&\n+\t\t(\n+\t\t\techo \"f\ttest=f\"\n+\t\t\techo \"a/i test=a/i\"\n+\t\t) >info/attributes &&\n+\t\tattr_check f f &&\n+\t\tattr_check a/f f &&\n+\t\tattr_check a/c/f f &&\n+\t\tattr_check a/i a/i &&\n+\t\tattr_check subdir/a/i unspecified\n+\t)\n '\n \n test_done\n-- \n1.9-rc2-233-ged4ee9f\n"},{"id":"234381","messageId":"1391712033-9443-2-git-send-email-gitster@pobox.com","threadId":"35813","inReplyTo":"1391712033-9443-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/2] check-attr: move to the top of working tree when in non-bare repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-06T18:40:33Z","receivedAt":"2014-02-06T18:40:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lasse Makholm noticed that running \"git check-attr\" from a place\ntotally unrelated to $GIT_DIR and $GIT_WORK_TREE does not give\nexpected results.  I think it is because the command does not say it\nwants to call setup_work_tree().\n\nWe still need to support use cases where only a bare repository is\ninvolved, so unconditionally requiring a working tree would not work\nwell.  Instead, make a call only in a non-bare repository.\n\nWe may want to see if we want to do a similar fix in the opposite\ndirection to check-ignore.  The command unconditionally requires a\nworking tree, but it should be usable in a bare repository just like\ncheck-attr attempts to be.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/check-attr.c  |  3 +++\n t/t0003-attributes.sh | 10 ++++++++++\n 2 files changed, 13 insertions(+)\n\ndiff --git a/builtin/check-attr.c b/builtin/check-attr.c\nindex 075d01d..f29d6c3 100644\n--- a/builtin/check-attr.c\n+++ b/builtin/check-attr.c\n@@ -94,6 +94,9 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)\n \tstruct git_attr_check *check;\n \tint cnt, i, doubledash, filei;\n \n+\tif (!is_bare_repository())\n+\t\tsetup_work_tree();\n+\n \tgit_config(git_default_config, NULL);\n \n \targc = parse_options(argc, argv, prefix, check_attr_options,\ndiff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh\nindex 0554b13..6e6aef5 100755\n--- a/t/t0003-attributes.sh\n+++ b/t/t0003-attributes.sh\n@@ -196,6 +196,16 @@ test_expect_success 'root subdir attribute test' '\n \tattr_check subdir/a/i unspecified\n '\n \n+test_expect_success 'using --git-dir and --work-tree' '\n+\tmkdir unreal real &&\n+\tgit init real &&\n+\techo \"file test=in-real\" >real/.gitattributes &&\n+\t(\n+\t\tcd unreal &&\n+\t\tattr_check file in-real \"--git-dir ../real/.git --work-tree ../real\"\n+\t)\n+'\n+\n test_expect_success 'setup bare' '\n \tgit clone --bare . bare.git\n '\n-- \n1.9-rc2-233-ged4ee9f\n"},{"id":"234392","messageId":"20140206194521.GU30398@google.com","threadId":"35813","inReplyTo":"1391712033-9443-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 1/2] t0003: do not chdir the whole test process","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-02-06T19:45:21Z","receivedAt":"2014-02-06T19:45:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> Moving to some other directory and letting the remainder of the test\n> pieces to expect that they start there is a bad practice.\n\nI agree with the above, and I like the patch...\n\n>                                                            The test\n> that contains chdir itself may fail (or by mistake skipped via the\n> GIT_SKIP_TESTS mechanism) in which case the remainder may operate on\n> files in unexpected places.\n\n... but this logic seems wrong.  I don't think we've ever supported\nsetup tests failing or being skipped in the past.\n\nThanks,\nJonathan\n"},{"id":"234393","messageId":"20140206195310.GV30398@google.com","threadId":"35813","inReplyTo":"1391712033-9443-2-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 2/2] check-attr: move to the top of working tree when in non-bare repository","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-02-06T19:53:10Z","receivedAt":"2014-02-06T19:53:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n\n> --- a/builtin/check-attr.c\n> +++ b/builtin/check-attr.c\n> @@ -94,6 +94,9 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)\n>  \tstruct git_attr_check *check;\n>  \tint cnt, i, doubledash, filei;\n>  \n> +\tif (!is_bare_repository())\n> +\t\tsetup_work_tree();\n\nHm.  Shouldn't check-attr error out when run without a worktree and\nwithout --cached?\n\nThat would mean something like\n\ndiff --git i/builtin/check-attr.c w/builtin/check-attr.c\nindex e9af7b2..c34b6ee 100644\n--- i/builtin/check-attr.c\n+++ w/builtin/check-attr.c\n@@ -107,6 +107,9 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, check_attr_options,\n \t\t\t     check_attr_usage, PARSE_OPT_KEEP_DASHDASH);\n \n+\tif (!cached_attrs)\n+\t\tsetup_work_tree();\n+\n \tif (read_cache() < 0) {\n \t\tdie(\"invalid cache\");\n \t}\n"},{"id":"234397","messageId":"20140206201743.GW30398@google.com","threadId":"35813","inReplyTo":"20140206195310.GV30398@google.com","subject":"Re: [PATCH 2/2] check-attr: move to the top of working tree when in non-bare repository","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-02-06T20:17:43Z","receivedAt":"2014-02-06T20:17:43Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi again,\n\nJonathan Nieder wrote:\n> Junio C Hamano wrote:\n\n>> +\tif (!is_bare_repository())\n>> +\t\tsetup_work_tree();\n>\n> Hm.  Shouldn't check-attr error out when run without a worktree and\n> without --cached?\n> \n> That would mean something like\n> \n> diff --git i/builtin/check-attr.c w/builtin/check-attr.c\n> index e9af7b2..c34b6ee 100644\n> --- i/builtin/check-attr.c\n> +++ w/builtin/check-attr.c\n> @@ -107,6 +107,9 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)\n>  \targc = parse_options(argc, argv, prefix, check_attr_options,\n>  \t\t\t     check_attr_usage, PARSE_OPT_KEEP_DASHDASH);\n>  \n> +\tif (!cached_attrs)\n> +\t\tsetup_work_tree();\n\nSomeone asked in a private reply how this interacts with t0003.\n\nt0003 tries check-attr in a bare repository.  The question is, is that\na desirable feature, and are people relying on it?  If people are\nrelying on it, perhaps the intuitive behavior would be to make\ncheck-attr use an only-look-at-HEAD mode by default when running in a\nbare repo.\n\nHow do I use the only-look-at-HEAD mode from a non-bare repo?  If I\nwant attributes with respect to some other commit instead of HEAD, is\nthere a syntax for that?  The command doesn't seem to have been well\nthought out.\n\nHope that helps,\nJonathan\n"},{"id":"234413","messageId":"xmqqvbwsxa2b.fsf@gitster.dls.corp.google.com","threadId":"35813","inReplyTo":"20140206194521.GU30398@google.com","subject":"Re: [PATCH 1/2] t0003: do not chdir the whole test process","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-06T20:25:48Z","receivedAt":"2014-02-06T20:25:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>\n>> Moving to some other directory and letting the remainder of the test\n>> pieces to expect that they start there is a bad practice.\n>\n> I agree with the above, and I like the patch...\n>\n>>                                                            The test\n>> that contains chdir itself may fail (or by mistake skipped via the\n>> GIT_SKIP_TESTS mechanism) in which case the remainder may operate on\n>> files in unexpected places.\n>\n> ... but this logic seems wrong.  I don't think we've ever supported\n> setup tests failing or being skipped in the past.\n\nThe first set-up test, yes, but something in the middle added as an\nafterthought?\n"},{"id":"234414","messageId":"20140206203101.GX30398@google.com","threadId":"35813","inReplyTo":"xmqqvbwsxa2b.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/2] t0003: do not chdir the whole test process","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-02-06T20:31:01Z","receivedAt":"2014-02-06T20:31:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>>>                                                            The test\n>>> that contains chdir itself may fail (or by mistake skipped via the\n>>> GIT_SKIP_TESTS mechanism) in which case the remainder may operate on\n>>> files in unexpected places.\n>>\n>> ... but this logic seems wrong.  I don't think we've ever supported\n>> setup tests failing or being skipped in the past.\n>\n> The first set-up test, yes, but something in the middle added as an\n> afterthought?\n\nEven set-up in the middle added as an afterthought, yes.\n\nFor a while I've been wanting to teach GIT_SKIP_TESTS not to skip\ntests with 'setup' or 'set up' in their name, but I never got around\nto it.  If I try to skip the setup test this patch touches, then there\nis no bare.git and lots of later tests fail.  Perhaps it would be\nbetter for each test to do\n\n\trm -fr bare.git &&\n\tgit clone --bare . bare.git &&\n\t(\n\t\tcd bare.git &&\n\t\t...\n\t)\n\nfor itself to make the state easier to think about.\n\nOn the other hand I agree that the 'cd' here is a bad practice.  I\njust don't think it's about skipping setup --- instead, it's about it\nbeing hard to remember the cwd in general.\n\nThanks,\nJonathan\n"},{"id":"234415","messageId":"xmqqr47gx9r4.fsf@gitster.dls.corp.google.com","threadId":"35813","inReplyTo":"20140206201743.GW30398@google.com","subject":"Re: [PATCH 2/2] check-attr: move to the top of working tree when in non-bare repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-06T20:32:31Z","receivedAt":"2014-02-06T20:32:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Someone asked in a private reply how this interacts with t0003.\n\nIt was me mistakenly using \"reply\" not \"reply all\".\n\n> t0003 tries check-attr in a bare repository.  The question is, is that\n> a desirable feature, and are people relying on it?\n\nRunning tar-tree from a public distribution point comes to mind.\nbootstrap-attr-stack seems to have reference to is-bare-repository\nto validate the attribute direction to read from the index, but I\ntend to think what it really wants is to read from HEAD not the\nindex.\n\n> How do I use the only-look-at-HEAD mode from a non-bare repo?\n\nIs \"You don't\" a good answer?\n\nUse of --cached when your index matches HEAD is the equivalent, and\nif the index differs from HEAD, you must have had a reason to add\nthat change to .gitattributes to the index, so I think it is\nreasonable to honour that change.\n"},{"id":"234417","messageId":"xmqqd2j0x78w.fsf@gitster.dls.corp.google.com","threadId":"35813","inReplyTo":"20140206203101.GX30398@google.com","subject":"Re: [PATCH 1/2] t0003: do not chdir the whole test process","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-06T21:26:39Z","receivedAt":"2014-02-06T21:26:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> For a while I've been wanting to teach GIT_SKIP_TESTS not to skip\n> tests with 'setup' or 'set up' in their name, but I never got around\n> to it.\n\nYeah, that would be a good thing.  As part of doing so, we might\nwant to come up with a way to test the tests, randomly skipping\npieces that are not \"setup\" and find ones that break the later tests\nwhen skipped, and mark test scripts that fail such a test for fixing.\n\n> If I try to skip the setup test this patch touches, then there\n> is no bare.git and lots of later tests fail.  Perhaps it would be\n> better for each test to do\n>\n> \trm -fr bare.git &&\n> \tgit clone --bare . bare.git &&\n> \t(\n> \t\tcd bare.git &&\n> \t\t...\n> \t)\n>\n> for itself to make the state easier to think about.\n\nThat is a better and worse way to do it at the same time ;-)  It\ndefinitely is better from maintainability POV to keep each test as\nindependent as possible.  It however also is worse if it forces us\nto be repetitive X-<.\n\n> On the other hand I agree that the 'cd' here is a bad practice.  I\n> just don't think it's about skipping setup --- instead, it's about it\n> being hard to remember the cwd in general.\n\nExactly.\n"},{"id":"234579","messageId":"CAB7pA0-dhFS_QOqxTdLmHAnsOUXSrmGi+dRR=+fMyY4M9m-XLA@mail.gmail.com","threadId":"35813","inReplyTo":"xmqqvbwsyvn8.fsf@gitster.dls.corp.google.com","subject":"Re: attr.c doesn't honor --work-tree option","fromName":"Lasse Makholm","fromEmail":"lasse.makholm@gmail.com","sentAt":"2014-02-10T12:57:13Z","receivedAt":"2014-02-10T12:57:13Z","isPatch":false,"sender":{"key":"lasse.makholm@gmail.com","avatar":"https://gravatar.com/avatar/5bc3a34e4fa8ba26b38c333f370b15c034351e18e09b056a2608f3904ae1a4c1?d=mp&s=160"},"body":"On 6 February 2014 18:54, Junio C Hamano <gitster@pobox.com> wrote:\n> Lasse Makholm <lasse.makholm@gmail.com> writes:\n>\n>> Here's a repro with -DDEBUG_ATTR=1 and a printf() in read_attr_from_file():\n>>\n>> $ cd /tmp/\n>> $ mkdir -p attr-test/repo\n>> $ cd attr-test/repo\n>> $ git init\n>> Initialized empty Git repository in /tmp/attr-test/repo/.git/\n>> $ echo 'dir/* filter=foo' >.gitattributes\n>> $\n>>\n>> Inside the working tree, it works:\n>>\n>> $ ~/src/git.git/git check-attr -a dir/file\n>\n> Does check-ignore misbehave the same way?\n\nNo, check-ignore works but also has NEED_WORK_TREE set. And that\nactually also feels a bit wrong to me because check-attr and\ncheck-ignore both seem like reasonable things to do in a bare repo\nbecause .git(attributes|ignore) files are likely to be committed in\nthe repo.\n\n> I suspect that is this because check-attr is not a command that\n> requires a working tree.  The command was written primarily as a\n> debugging aid that can be used anywhere as long as you have a\n> repository to read strings from either its standard input or its\n> arguments, and gives them directly to check_attr(), but it does so\n> without first going to the top of the real working tree like\n> check-ignore does.\n\nFair point. I actually stumbled across this because a git cat-file\n--textconv ... was failing, so that's at least one other (and arguably\nmore real) use case that is broken in the same way.\n\n> Forcing it to go to the top of the working tree (see the attached\n> one-liner, but note that I didn't test it) may give you want you\n> want.\n\nFor this case, it does, yes. But it also breaks check-attr in bare\nrepos with attributes defined in $GIT_DIR/info/attributes because it\nwill refuse to run without a work tree...\n\nIn any case the current state seems broken because --work-tree clearly\ndoesn't work for all commands...\n\nSetting NEED_WORK_TREE for check-attr risks breaking existing scripts\nbut on the other hand there doesn't seem to be any good reason why\ncheck-attr and check-ignore should differ in this regard...\n\nIt seems like the ideal solution would be an optional NEED_WORK_TREE\nof some sort that would let these commands work correctly both with\n--work-tree, without it and in bare repos but I get that that might\nnot be easy to fix...\n\nAnother approach might be to deprecate --work-tree and tell people to\nuse -C instead...\n\n/L\n\n>  git.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/git.c b/git.c\n> index 7cf2953..314ec9f 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -342,7 +342,7 @@ static struct cmd_struct commands[] = {\n>         { \"branch\", cmd_branch, RUN_SETUP },\n>         { \"bundle\", cmd_bundle, RUN_SETUP_GENTLY },\n>         { \"cat-file\", cmd_cat_file, RUN_SETUP },\n> -       { \"check-attr\", cmd_check_attr, RUN_SETUP },\n> +       { \"check-attr\", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },\n>         { \"check-ignore\", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },\n>         { \"check-mailmap\", cmd_check_mailmap, RUN_SETUP },\n>         { \"check-ref-format\", cmd_check_ref_format },\n"},{"id":"234884","messageId":"53009DCA.2060001@alum.mit.edu","threadId":"35813","inReplyTo":"20140206201743.GW30398@google.com","subject":"Re: [PATCH 2/2] check-attr: move to the top of working tree when in non-bare repository","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-02-16T11:15:22Z","receivedAt":"2014-02-16T11:15:22Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/06/2014 09:17 PM, Jonathan Nieder wrote:\n> How do I use the only-look-at-HEAD mode from a non-bare repo?  If I\n> want attributes with respect to some other commit instead of HEAD, is\n> there a syntax for that?  The command doesn't seem to have been well\n> thought out.\n\nI agree that it would be nice for \"git check-attr\" to handle this case.\n Currently, I believe that one has to resort to a temporary index file\nvia something like\n\n    (\n        export GIT_INDEX_FILE=\"$(mktemp)\"\n        git read-tree HEAD\n        git check-attr --cached ...\n        rm \"$GIT_INDEX_FILE\"\n    )\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"}]}