{"thread":{"id":"9506","subject":"[PATCH 2/2] checkout: fix attribute handling in checkout all","startedAt":"2007-08-12T20:34:34Z","lastAt":"2007-08-14T08:41:02Z","messageCount":20,"participants":["Steffen Prohaska","Junio C Hamano","Brian Downing","Marius Storm-Olsen","David Kastrup","Dmitry Kakurin","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"50594","messageId":"11869508753328-git-send-email-prohaska@zib.de","threadId":"9506","inReplyTo":null,"subject":"[PATCH 1/2] attr: fix attribute handling if .gitattributes is involved","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-08-12T20:34:34Z","receivedAt":"2007-08-12T20:34:34Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"If we checkout .gitattributes, we must not try to parse\n.gitattributes. If we tried to read it, it may not be\npresent because checkout_entry unlinks files before checkout.\nBut we would record that no attributes are present for this\ndirectory, which is wrong. And worse, we would never try again.\n\nThis fix skips read_attr_from_file if the path triggering\nthe read ends with .gitattributes. This is a bit more than we\nneed, but it helps to checkout all .gitattributes before any\nother file, without starting the attr machinery.\n\nThis solves part of the problem of correct attributes during\ncheckout. For example\n\n   rm .gitattributes other\n   git-checkout-index -f .gitattributes other\n\nwill work as expected, while\n\n   rm .gitattributes other\n   git-checkout-index -f other .gitattributes\n\nwill not. The problem in the second case is that .gitattributes\nis not yet present when it is needed.\n\nThe second case could be fixed by ordering files such that\nall .gitattributes are checked out before any other file. But\nthis is perhaps too expensive and not really needed. If the user\ndeliberately chooses to checkout .gitattributes after another\nfile, he will not benefit from the attr machinery.\n\nHowever, for checkout-index --all we should fix the problem,\nwhich is done by the next patch, building on top of this one.\n\nSigned-off-by: Steffen Prohaska <prohaska@zib.de>\n---\n attr.c |   58 ++++++++++++++++++++++++++++++++++++++++++----------------\n 1 files changed, 42 insertions(+), 16 deletions(-)\n\nThis together with the patch that follows fixes a problem, which\nis most likely to bite Windows users. I recognized it by setting\nautocrlf globally to true and doing a fresh checkout of msysgit.\nThe checkout contained etc/termcap converted to CRLF, although\nis was marked as '-crlf' in etc/.gitattributes.\n\nIf we believe autocrlf is a reasonable default for Windows users,\nwe really should use it ourselves, to find such problems. \n\nThe fixed problem is not really critical but may be quite annoying,\nand complex to understand.\n\n    Steffen\n\ndiff --git a/attr.c b/attr.c\nindex a071254..e942f6c 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -383,6 +383,18 @@ static void bootstrap_attr_stack(void)\n \t}\n }\n \n+static int ends_with_gitattributes (const char* path)\n+{\n+\tint attributes_len = strlen (GITATTRIBUTES_FILE);\n+\tint path_len = strlen (path);\n+\tif (path_len >= attributes_len\n+\t     && strcmp (path + path_len - attributes_len, GITATTRIBUTES_FILE) == 0)\n+\t{\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n static void prepare_attr_stack(const char *path, int dirlen)\n {\n \tstruct attr_stack *elem, *info;\n@@ -430,23 +442,37 @@ static void prepare_attr_stack(const char *path, int dirlen)\n \n \t/*\n \t * Read from parent directories and push them down\n+\t *\n+\t * But don't try to read if path ends with .gitattributes.\n+\t * In this case we could fail and record no attributes for\n+\t * a directory. We better wait and see if we need the\n+\t * attributes later.\n+\t *\n+\t * We skip all .gitattributes, even in higher directories.\n+\t * Thus, we can checkout all .gitattributes in any order\n+\t * before the attr machinery starts to work. .gitattributes\n+\t * should not be controlled by .gitattributes in the working\n+\t * tree anyway.\n+\t *\n \t */\n-\twhile (1) {\n-\t\tchar *cp;\n-\n-\t\tlen = strlen(attr_stack->origin);\n-\t\tif (dirlen <= len)\n-\t\t\tbreak;\n-\t\tmemcpy(pathbuf, path, dirlen);\n-\t\tmemcpy(pathbuf + dirlen, \"/\", 2);\n-\t\tcp = strchr(pathbuf + len + 1, '/');\n-\t\tstrcpy(cp + 1, GITATTRIBUTES_FILE);\n-\t\telem = read_attr_from_file(pathbuf, 0);\n-\t\t*cp = '\\0';\n-\t\telem->origin = strdup(pathbuf);\n-\t\telem->prev = attr_stack;\n-\t\tattr_stack = elem;\n-\t\tdebug_push(elem);\n+\tif (!ends_with_gitattributes (path)) {\n+\t\twhile (1) {\n+\t\t\tchar *cp;\n+\n+\t\t\tlen = strlen(attr_stack->origin);\n+\t\t\tif (dirlen <= len)\n+\t\t\t\tbreak;\n+\t\t\tmemcpy(pathbuf, path, dirlen);\n+\t\t\tmemcpy(pathbuf + dirlen, \"/\", 2);\n+\t\t\tcp = strchr(pathbuf + len + 1, '/');\n+\t\t\tstrcpy(cp + 1, GITATTRIBUTES_FILE);\n+\t\t\telem = read_attr_from_file(pathbuf, 0);\n+\t\t\t*cp = '\\0';\n+\t\t\telem->origin = strdup(pathbuf);\n+\t\t\telem->prev = attr_stack;\n+\t\t\tattr_stack = elem;\n+\t\t\tdebug_push(elem);\n+\t\t}\n \t}\n \n \t/*\n-- \n1.5.3.rc4.96.g6ceb\n"},{"id":"50593","messageId":"118695087531-git-send-email-prohaska@zib.de","threadId":"9506","inReplyTo":"11869508753328-git-send-email-prohaska@zib.de","subject":"[PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-08-12T20:34:35Z","receivedAt":"2007-08-12T20:34:35Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"We need to check out .gitattributes files first to have\nthem in place when we check out the remaining files. This\nis needed to get the right attributes during checkout,\nfor example having the right crlf conversion on the first\ncheckout if crlf is controlled by a .gitattribute file.\n\nThis works only together with the commit\n\n'attr: fix attribute handling if .gitattributes is involved'\n\nwhich ensures that .gitattributes files do not trigger the\nattribute machinery too early.\n\nSigned-off-by: Steffen Prohaska <prohaska@zib.de>\n---\n builtin-checkout-index.c |   47 ++++++++++++++++++++++++++++-----------------\n 1 files changed, 29 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin-checkout-index.c b/builtin-checkout-index.c\nindex 75377b9..5e87a39 100644\n--- a/builtin-checkout-index.c\n+++ b/builtin-checkout-index.c\n@@ -125,27 +125,38 @@ static int checkout_file(const char *name, int prefix_length)\n \n static void checkout_all(const char *prefix, int prefix_length)\n {\n-\tint i, errs = 0;\n+\tint i, pass, errs = 0;\n \tstruct cache_entry* last_ce = NULL;\n \n-\tfor (i = 0; i < active_nr ; i++) {\n-\t\tstruct cache_entry *ce = active_cache[i];\n-\t\tif (ce_stage(ce) != checkout_stage\n-\t\t    && (CHECKOUT_ALL != checkout_stage || !ce_stage(ce)))\n-\t\t\tcontinue;\n-\t\tif (prefix && *prefix &&\n-\t\t    (ce_namelen(ce) <= prefix_length ||\n-\t\t     memcmp(prefix, ce->name, prefix_length)))\n-\t\t\tcontinue;\n-\t\tif (last_ce && to_tempfile) {\n-\t\t\tif (ce_namelen(last_ce) != ce_namelen(ce)\n-\t\t\t    || memcmp(last_ce->name, ce->name, ce_namelen(ce)))\n-\t\t\t\twrite_tempfile_record(last_ce->name, prefix_length);\n+\t/* pass 0: check out only .gitattribute files\n+\t   pass 1: check out every file\n+\n+\t   This is needed to have all .gitattributes in place before\n+\t   checking out files, and thus do the right conversion.\n+\t */\n+\tfor (pass = 0; pass < 2; pass++) {\n+\t\tfor (i = 0; i < active_nr ; i++) {\n+\t\t\tstruct cache_entry *ce = active_cache[i];\n+\t\t\tif (pass == 0 && strstr (ce->name, GITATTRIBUTES_FILE) == 0) {\n+\t\t\t    continue;\n+\t\t\t}\n+\t\t\tif (ce_stage(ce) != checkout_stage\n+\t\t\t    && (CHECKOUT_ALL != checkout_stage || !ce_stage(ce)))\n+\t\t\t\tcontinue;\n+\t\t\tif (prefix && *prefix &&\n+\t\t\t    (ce_namelen(ce) <= prefix_length ||\n+\t\t\t     memcmp(prefix, ce->name, prefix_length)))\n+\t\t\t\tcontinue;\n+\t\t\tif (last_ce && to_tempfile) {\n+\t\t\t\tif (ce_namelen(last_ce) != ce_namelen(ce)\n+\t\t\t\t    || memcmp(last_ce->name, ce->name, ce_namelen(ce)))\n+\t\t\t\t\twrite_tempfile_record(last_ce->name, prefix_length);\n+\t\t\t}\n+\t\t\tif (checkout_entry(ce, &state,\n+\t\t\t    to_tempfile ? topath[ce_stage(ce)] : NULL) < 0)\n+\t\t\t\terrs++;\n+\t\t\tlast_ce = ce;\n \t\t}\n-\t\tif (checkout_entry(ce, &state,\n-\t\t    to_tempfile ? topath[ce_stage(ce)] : NULL) < 0)\n-\t\t\terrs++;\n-\t\tlast_ce = ce;\n \t}\n \tif (last_ce && to_tempfile)\n \t\twrite_tempfile_record(last_ce->name, prefix_length);\n-- \n1.5.3.rc4.96.g6ceb\n"},{"id":"50599","messageId":"7veji8ifs2.fsf@assigned-by-dhcp.cox.net","threadId":"9506","inReplyTo":"118695087531-git-send-email-prohaska@zib.de","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-12T21:50:21Z","receivedAt":"2007-08-12T21:50:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steffen Prohaska <prohaska@zib.de> writes:\n\n> We need to check out .gitattributes files first to have\n> them in place when we check out the remaining files. This\n> is needed to get the right attributes during checkout,\n> for example having the right crlf conversion on the first\n> checkout if crlf is controlled by a .gitattribute file.\n>\n> This works only together with the commit\n>\n> 'attr: fix attribute handling if .gitattributes is involved'\n\nWhile I think it is _one_ good approach to make things two-pass,\nI do not know if this is enough.  A logic similar to this should\nbe made available to the codepath that switches branches,\nshouldn't it?\n\nIt feels somewhat bogus to treat only the files that contain\n\".gitattributes\" as substring.  Don't you want to at least say\n\"is .gitattributes or ends with /.gitattributes\"?\n\nI am not 100% convinced that it is \"unexpected\" that\nthese two sequences give different results.\n\n (1) rm -f .gitattributes other\n     git-checkout-index -f .gitattributes\n     git-checkout-index -f other\n\n (2) rm -f .gitattributes other\n     git-checkout-index -f other\n     git-checkout-index -f .gitattributes\n\nAnd if this is mostly to work around the chicken-and-egg problem\nof the initial checkout, I do not know if we would want to\ncomplicate checkout_all() nor prepare_attr_stack().  Perhaps the\n_initial_ checkout can do something like:\n\n * look at index, checkout .gitattributes and */.gitattributes;\n * checkout -f -a\n\n_at the Porcelain level_, without complicating the plumbing?\n\nBoth patches are seriously out of existing coding style, by the\nway.  Extra spaces after called function names everywhere, etc.\n"},{"id":"50602","messageId":"BC69C8EA-7486-4542-B866-EC52A5BF4B1F@zib.de","threadId":"9506","inReplyTo":"7veji8ifs2.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-08-12T22:26:15Z","receivedAt":"2007-08-12T22:26:15Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Aug 12, 2007, at 11:50 PM, Junio C Hamano wrote:\n\n> Steffen Prohaska <prohaska@zib.de> writes:\n>\n>> We need to check out .gitattributes files first to have\n>> them in place when we check out the remaining files. This\n>> is needed to get the right attributes during checkout,\n>> for example having the right crlf conversion on the first\n>> checkout if crlf is controlled by a .gitattribute file.\n>>\n>> This works only together with the commit\n>>\n>> 'attr: fix attribute handling if .gitattributes is involved'\n>\n> While I think it is _one_ good approach to make things two-pass,\n> I do not know if this is enough.  A logic similar to this should\n> be made available to the codepath that switches branches,\n> shouldn't it?\n\nI think so. I played a bit more and I am pretty sure that\nswitching branches suffers from the same problem. After I\nunderstood the problem I now remember that I wondered why\nI needed '-f' now and then to convince git to do things\nthat normally just work.\n\n\n> It feels somewhat bogus to treat only the files that contain\n> \".gitattributes\" as substring.  Don't you want to at least say\n> \"is .gitattributes or ends with /.gitattributes\"?\n\nYes, if someone really want to construct a case, he can break\nmy code. Where should I place a helper function, such as\nends_with_gitattributes()?\n\n\n> I am not 100% convinced that it is \"unexpected\" that\n> these two sequences give different results.\n>\n>  (1) rm -f .gitattributes other\n>      git-checkout-index -f .gitattributes\n>      git-checkout-index -f other\n>\n>  (2) rm -f .gitattributes other\n>      git-checkout-index -f other\n>      git-checkout-index -f .gitattributes\n\nYeah, it's not obvious to me either.\n\nMy feeling it that the working tree should match the current\ncontent of .gitattributes. That is after you modified\n.gittattributes by whatever means (checkout, editor, ...),\nor you modified the global default for autocrlf, you should\nhave a way to update your working tree. One way would be to\nforce a fresh checkout of all files in the working tree.\nHow can I do that?\n\nI'm pretty certain that all files in the working tree resulting\nfrom a single command should match the .gitattributes that\nwere modified by the same command. This is true for initial\ncheckout, but also for branch switching.\n\n\n> And if this is mostly to work around the chicken-and-egg problem\n> of the initial checkout, I do not know if we would want to\n> complicate checkout_all() nor prepare_attr_stack().  Perhaps the\n> _initial_ checkout can do something like:\n>\n>  * look at index, checkout .gitattributes and */.gitattributes;\n>  * checkout -f -a\n>\n> _at the Porcelain level_, without complicating the plumbing?\n\nI'm not convinced that it's only the initial checkout. As you\nalready mentioned above, branch switching suffers from the\nsame problem. But it could perhaps be handled on a porcelain\nlevel.\n\nMaybe we should start with some test cases first?\n\n\n> Both patches are seriously out of existing coding style, by the\n> way.  Extra spaces after called function names everywhere, etc.\n\nHmm, I see, ... my daytime coding style got burnt into my brain.\nI'd rework if needed. But let's first find out what's needed.\n\n\tSteffen\n"},{"id":"50611","messageId":"20070813015115.GI21692@lavos.net","threadId":"9506","inReplyTo":"11869508753328-git-send-email-prohaska@zib.de","subject":"Re: [PATCH 1/2] attr: fix attribute handling if .gitattributes is involved","fromName":"Brian Downing","fromEmail":"bdowning@lavos.net","sentAt":"2007-08-13T01:51:15Z","receivedAt":"2007-08-13T01:51:15Z","isPatch":true,"sender":{"key":"bdowning@lavos.net","avatar":"https://avatars.githubusercontent.com/u/366426?v=4"},"body":"On Sun, Aug 12, 2007 at 10:34:34PM +0200, Steffen Prohaska wrote:\n> This together with the patch that follows fixes a problem, which\n> is most likely to bite Windows users. I recognized it by setting\n> autocrlf globally to true and doing a fresh checkout of msysgit.\n> The checkout contained etc/termcap converted to CRLF, although\n> is was marked as '-crlf' in etc/.gitattributes.\n> \n> If we believe autocrlf is a reasonable default for Windows users,\n> we really should use it ourselves, to find such problems. \n> \n> The fixed problem is not really critical but may be quite annoying,\n> and complex to understand.\n\nI have a case in a live repository where this is not merely annoying.\n\nI have a test data in one of my repositories that must never be\nconverted.  It has a mix of Unix and Windows line-endings in it.  I\nmarked the appropriate files with the \"-crlf\" attribute.\n\nWith the current git behavior, cloning this repository with \"autocrlf\"\nglobally set irreversably corrupts the files (converting the LF\nline-enders to CRLF) on checkout.  If the user is careless upon commit,\nthese corrupted files will then be committed back (probably with all\nCRLFs, since at that point the .gitattributes is present and the -crlf\nattribute will be honored.)\n\nThere is kind of an ugly chicken-and-egg problem here, but I think it\nwould be good to figure it out to avoid this kind of broken behavior.\n\nI would also vote for this handling to be in the plumbing, since the\nautocrlf processing is in the plumbing as well.\n\nAnother thing to consider with respect to attribute access -- It would\nbe nice for git-cvsserver to be able to send the correct -k option to\nthe remote side for line endings.  Doing this correctly involves\naccessing attributes, but git-cvsserver never has a full working\ndirectory.  Having the attribute machinery work without a working\ndirectory (either directly from trees or from an index) would be a great\nbenefit here.\n\n-bcd\n"},{"id":"50616","messageId":"7vfy2ogdvl.fsf@assigned-by-dhcp.cox.net","threadId":"9506","inReplyTo":"7veji8ifs2.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-13T06:14:22Z","receivedAt":"2007-08-13T06:14:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Steffen Prohaska <prohaska@zib.de> writes:\n> ...\n>> This works only together with the commit\n>>\n>> 'attr: fix attribute handling if .gitattributes is involved'\n>\n> While I think it is _one_ good approach to make things two-pass,\n> I do not know if this is enough.  A logic similar to this should\n> be made available to the codepath that switches branches,\n> shouldn't it?\n\nOk, let's step back a bit and I'll suggest an alternative\napproach to your 1/2.  This would hopefully solve 2/2 without\nany code change your patch 2/2 has.\n\nI think this approach is very much in line with how the git\nplumbing works, but you would need to know how the world is\ndesigned to work in order to appreciate it fully.  Let's have a\nfew paragraphs to give the readers some background.\n\nThe work tree side of git is primarily about the index, and what\nis on the work tree is more or less secondary.  At the lower\nlevel, often we deliberately treat not having a working tree\nfile as equivalent to having an unmodified work tree file.  We\ncan apply the same principle to this \"missing .gitattributes\nfile\" case.\n\nPeople who only know modern git may not be aware of this, but\nyou can apply patches and perform a merge in a work tree that\ndoes not have any file checked out, as long as your index is\nfully populated.  For example, you can do something like this:\n\n    $ git clone -n git://.../git.git v.git\n    $ cd v.git\n    $ git update-ref --no-deref HEAD $(git rev-parse v1.5.3-rc4^0)\n    $ git read-tree HEAD\n    $ git apply --index patch.txt\n\nYou will have the files that are patched in the resulting work\ntree, so that you can inspect the result.  If you like the\nresult, you can even make a commit in such a sparsely populated\ntree:\n\n    $ git commit\n\nOf course, \"git commit -a\" and \"git add -u\" Porcelain options\nare more recent inventions, and they would not work with such a\nsparsely populated work tree.  But the above demonstration shows\nthat at the plumbing level the index is the king and the work\ntree is secondary, and this is very much as designed.  The merge\noperation has similar characteristics:\n\n    $ git merge master\n\n... will check out the paths that need file-level 3-way merge,\nso that you can inspect the result, but what you will have is a\nsparsely populated work tree, and this is as designed.\n\nCurrently, the attr_stack code reads only from the work tree\nand work tree alone.  We could change it to:\n\n - If the directory on the work tree has .gitattributes, use it\n   (this is what the current code does);\n\n - Otherwise if the index has .gitattributes at the\n   corresponding path, use that instead.\n\nThis essentially treats not having .gitattributes files checked\nout as equivalent to having these files checked out unmodified,\nwhich is very much in line with how the world is designed to\nwork.\n"},{"id":"50617","messageId":"46BFFB1A.4070704@trolltech.com","threadId":"9506","inReplyTo":"7vfy2ogdvl.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2007-08-13T06:32:58Z","receivedAt":"2007-08-13T06:32:58Z","isPatch":true,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"Junio C Hamano said the following on 13.08.2007 08:14:\n> Ok, let's step back a bit and I'll suggest an alternative\n> approach to your 1/2.  This would hopefully solve 2/2 without\n> any code change your patch 2/2 has.\n(..snip..)\n> I think this approach is very much in line with how the git\n> plumbing works, but you would need to know how the world is\n> designed to work in order to appreciate it fully.  Let's have a\n> few paragraphs to give the readers some background.\n(..snip..)\n> Currently, the attr_stack code reads only from the work tree\n> and work tree alone.  We could change it to:\n> \n>  - If the directory on the work tree has .gitattributes, use it\n>    (this is what the current code does);\n> \n>  - Otherwise if the index has .gitattributes at the\n>    corresponding path, use that instead.\n> \n> This essentially treats not having .gitattributes files checked\n> out as equivalent to having these files checked out unmodified,\n> which is very much in line with how the world is designed to\n> work.\n\nACK! We really need this! :-)\n\nIn msysgit.git/etc/.gitattributes we have 'termcap -crlf', to avoid \nthe termcaps being checked out with Windows EOL, if the user happens \nto have 'autocrlf = true'. However, when you checkout the working dir \nthe first time it still has Windows EOL due to exactly this problem.\n\nThe above algorithm would alleviate this issue.\n\n-- \n.marius\n\n"},{"id":"50618","messageId":"8D126F5A-5998-4CB2-89BE-1CAEF5AE621F@zib.de","threadId":"9506","inReplyTo":"7vfy2ogdvl.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-08-13T06:46:09Z","receivedAt":"2007-08-13T06:46:09Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Aug 13, 2007, at 8:14 AM, Junio C Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Steffen Prohaska <prohaska@zib.de> writes:\n>> ...\n>>> This works only together with the commit\n>>>\n>>> 'attr: fix attribute handling if .gitattributes is involved'\n>>\n>> While I think it is _one_ good approach to make things two-pass,\n>> I do not know if this is enough.  A logic similar to this should\n>> be made available to the codepath that switches branches,\n>> shouldn't it?\n>\n> Ok, let's step back a bit and I'll suggest an alternative\n> approach to your 1/2.  This would hopefully solve 2/2 without\n> any code change your patch 2/2 has.\n\nThat would be great.\n\n\n> I think this approach is very much in line with how the git\n> plumbing works, but you would need to know how the world is\n> designed to work in order to appreciate it fully.  Let's have a\n> few paragraphs to give the readers some background.\n>\n> The work tree side of git is primarily about the index, and what\n> is on the work tree is more or less secondary.  At the lower\n> level, often we deliberately treat not having a working tree\n> file as equivalent to having an unmodified work tree file.  We\n> can apply the same principle to this \"missing .gitattributes\n> file\" case.\n>\n> People who only know modern git may not be aware of this, but\n> you can apply patches and perform a merge in a work tree that\n> does not have any file checked out, as long as your index is\n> fully populated.  For example, you can do something like this:\n>\n>     $ git clone -n git://.../git.git v.git\n>     $ cd v.git\n>     $ git update-ref --no-deref HEAD $(git rev-parse v1.5.3-rc4^0)\n>     $ git read-tree HEAD\n>     $ git apply --index patch.txt\n>\n> You will have the files that are patched in the resulting work\n> tree, so that you can inspect the result.  If you like the\n> result, you can even make a commit in such a sparsely populated\n> tree:\n>\n>     $ git commit\n>\n> Of course, \"git commit -a\" and \"git add -u\" Porcelain options\n> are more recent inventions, and they would not work with such a\n> sparsely populated work tree.  But the above demonstration shows\n> that at the plumbing level the index is the king and the work\n> tree is secondary, and this is very much as designed.  The merge\n> operation has similar characteristics:\n>\n>     $ git merge master\n>\n> ... will check out the paths that need file-level 3-way merge,\n> so that you can inspect the result, but what you will have is a\n> sparsely populated work tree, and this is as designed.\n\nAh, merge ...\n\n\n> Currently, the attr_stack code reads only from the work tree\n> and work tree alone.  We could change it to:\n>\n>  - If the directory on the work tree has .gitattributes, use it\n>    (this is what the current code does);\n>\n>  - Otherwise if the index has .gitattributes at the\n>    corresponding path, use that instead.\n>\n> This essentially treats not having .gitattributes files checked\n> out as equivalent to having these files checked out unmodified,\n> which is very much in line with how the world is designed to\n> work.\n>\n\nWe may have conflicts in the .gitattributes file during a merge.\n.gitattributes may be present in different stages, and with\nconflict markers in the work tree.\n\nCould we drop reading the file in the work tree completely?\n.gitattributes would be a property of the index alone. To control\nattributes you first need to add them to the index, before adding\nthe file that has attributes set in .gitattributes.\n\nIf we have .gitattributes in different stages, the right one\nshould be chosen to checkout corresponding files in the same stage.\n\n\tSteffen\n"},{"id":"50619","messageId":"5BA0164D-8327-4B01-84CD-595D8573BEEF@zib.de","threadId":"9506","inReplyTo":"46BFFB1A.4070704@trolltech.com","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-08-13T06:50:36Z","receivedAt":"2007-08-13T06:50:36Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Aug 13, 2007, at 8:32 AM, Marius Storm-Olsen wrote:\n\n> Junio C Hamano said the following on 13.08.2007 08:14:\n>> Ok, let's step back a bit and I'll suggest an alternative\n>> approach to your 1/2.  This would hopefully solve 2/2 without\n>> any code change your patch 2/2 has.\n> (..snip..)\n>> I think this approach is very much in line with how the git\n>> plumbing works, but you would need to know how the world is\n>> designed to work in order to appreciate it fully.  Let's have a\n>> few paragraphs to give the readers some background.\n> (..snip..)\n>> Currently, the attr_stack code reads only from the work tree\n>> and work tree alone.  We could change it to:\n>>  - If the directory on the work tree has .gitattributes, use it\n>>    (this is what the current code does);\n>>  - Otherwise if the index has .gitattributes at the\n>>    corresponding path, use that instead.\n>> This essentially treats not having .gitattributes files checked\n>> out as equivalent to having these files checked out unmodified,\n>> which is very much in line with how the world is designed to\n>> work.\n>\n> ACK! We really need this! :-)\n>\n> In msysgit.git/etc/.gitattributes we have 'termcap -crlf', to avoid  \n> the termcaps being checked out with Windows EOL, if the user  \n> happens to have 'autocrlf = true'. However, when you checkout the  \n> working dir the first time it still has Windows EOL due to exactly  \n> this problem.\n\nAnd exactly this is where I recognized the issue.\n\nmsysgit devs,\nWe should really make autocrlf = true the default for us and fix all\nproblems that we'll encounter. There may be more tricky stuff ahead,\nlike merges, cherry-picks, ...\n\n\tSteffen\n"},{"id":"50620","messageId":"46C00515.5050308@trolltech.com","threadId":"9506","inReplyTo":"5BA0164D-8327-4B01-84CD-595D8573BEEF@zib.de","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2007-08-13T07:15:33Z","receivedAt":"2007-08-13T07:15:33Z","isPatch":true,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"Steffen Prohaska said the following on 13.08.2007 08:50:\n> On Aug 13, 2007, at 8:32 AM, Marius Storm-Olsen wrote:\n>> In msysgit.git/etc/.gitattributes we have 'termcap -crlf', to avoid  \n>> the termcaps being checked out with Windows EOL, if the user  \n>> happens to have 'autocrlf = true'. However, when you checkout the  \n>> working dir the first time it still has Windows EOL due to exactly  \n>> this problem.\n> \n> And exactly this is where I recognized the issue.\n> \n> msysgit devs,\n> We should really make autocrlf = true the default for us and fix\n> all problems that we'll encounter. There may be more tricky stuff\n> ahead, like merges, cherry-picks, ...\n\nI'm more leaning towards having the installer give you the option to \nchoose what kind of line-endings you want Git to work with; just like \nthe Cygwin installer.\n\n-- \n.marius\n\n"},{"id":"50621","messageId":"86eji7lww5.fsf@lola.quinscape.zz","threadId":"9506","inReplyTo":"7vfy2ogdvl.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-13T07:24:42Z","receivedAt":"2007-08-13T07:24:42Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>     $ git update-ref --no-deref HEAD $(git rev-parse v1.5.3-rc4^0)\n\nIs there a fundamental difference to using\n\ngit-symbolic-ref HEAD $(git rev-parse v1.5.3-rc4^0)\n\nhere?\n\n-- \nDavid Kastrup\n"},{"id":"50622","messageId":"521D9D91-2422-4378-BD68-37550731E06A@zib.de","threadId":"9506","inReplyTo":"46C00515.5050308@trolltech.com","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-08-13T07:32:28Z","receivedAt":"2007-08-13T07:32:28Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Aug 13, 2007, at 9:15 AM, Marius Storm-Olsen wrote:\n\n> Steffen Prohaska said the following on 13.08.2007 08:50:\n>> On Aug 13, 2007, at 8:32 AM, Marius Storm-Olsen wrote:\n>>> In msysgit.git/etc/.gitattributes we have 'termcap -crlf', to  \n>>> avoid  the termcaps being checked out with Windows EOL, if the  \n>>> user  happens to have 'autocrlf = true'. However, when you  \n>>> checkout the  working dir the first time it still has Windows EOL  \n>>> due to exactly  this problem.\n>> And exactly this is where I recognized the issue.\n>> msysgit devs,\n>> We should really make autocrlf = true the default for us and fix\n>> all problems that we'll encounter. There may be more tricky stuff\n>> ahead, like merges, cherry-picks, ...\n>\n> I'm more leaning towards having the installer give you the option  \n> to choose what kind of line-endings you want Git to work with; just  \n> like the Cygwin installer.\n\nWhich is the root of much trouble with Cygwin. People now say,\ngit works perfectly in Cygwin but forget to mention that they\nmean Cygwin A (in binmode) but not Cygwin B (in textmode).\n\nBetter choose the right default and work hard to make the\ndefault choice work perfectly. I am strongly against an option\nin the installer. An option _will_ cause confusion. Better give\npeople a hint how they can override the default for a single\nuser, or for a single repo. Then they recognize that they move\nto a non-default configuration and hopefully think twice. And we\nnever need to talk about msysgit A vs. msysgit B, but only about\nmsysgit with repo specific or user specific options.\n\nFor me, the question comes down to the following: What would the\naverage Windows user (real Windows user, not Linux user who was\nforced to work in Cygwin!) expect git to do with line endings?\nThe answer to this question should be the default.\n\n\tSteffen\n"},{"id":"50623","messageId":"46C018DA.1020309@trolltech.com","threadId":"9506","inReplyTo":"521D9D91-2422-4378-BD68-37550731E06A@zib.de","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2007-08-13T08:39:54Z","receivedAt":"2007-08-13T08:39:54Z","isPatch":true,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"Steffen Prohaska said the following on 13.08.2007 09:32:\n> On Aug 13, 2007, at 9:15 AM, Marius Storm-Olsen wrote:\n>> Steffen Prohaska said the following on 13.08.2007 08:50:\n>>> We should really make autocrlf = true the default for us and\n>>> fix all problems that we'll encounter. There may be more tricky\n>>> stuff ahead, like merges, cherry-picks, ...\n>> I'm more leaning towards having the installer give you the option\n>>  to choose what kind of line-endings you want Git to work with;\n>> just like the Cygwin installer.\n> \n> Which is the root of much trouble with Cygwin. People now say, git\n> works perfectly in Cygwin but forget to mention that they mean\n> Cygwin A (in binmode) but not Cygwin B (in textmode).\n> \n> Better choose the right default and work hard to make the default\n> choice work perfectly. I am strongly against an option in the\n> installer. An option _will_ cause confusion. Better give people a\n> hint how they can override the default for a single user, or for a\n> single repo. Then they recognize that they move to a non-default\n> configuration and hopefully think twice. And we never need to talk\n> about msysgit A vs. msysgit B, but only about msysgit with repo\n> specific or user specific options.\n> \n> For me, the question comes down to the following: What would the \n> average Windows user (real Windows user, not Linux user who was \n> forced to work in Cygwin!) expect git to do with line endings? The\n> answer to this question should be the default.\n\nIf we were talking about a huge amount (real) Windows users I would \nagree with you. However, currently most of the users using Git on \nWindows are Unix users which for some reason have to work on Windows \nevery now and then. And changing the default option to autocrlf=true \nwould be stepping on their toes, which we probably don't want to do :-)\n\nI'm a Windows developer myself, so I naturally have autocrlf=true in \nmy global settings. I don't think having the option in the installer \n(together with other things, like setting the global username, and \nemail for example) would be such a bad thing. The problem with the way \nthe Cygwin installer presents it is that it doesn't explain the pros \nand cons of the two options; it just recommends Linux EOL, which leads \nto confusion with some Windows developers. If we properly explain the \nissue in the installer, and say we recommend Windows EOL for Windows \ndevelopers, I think it's OK. It would in any case be better than the \ncurrent state where you have no option, or stepping on all the current \nmsysgit/mingw-git maintainers toes.\n\n-- \n.marius\n\n"},{"id":"50624","messageId":"DD0F51D8-390E-41DB-BD80-0BB440418D01@zib.de","threadId":"9506","inReplyTo":"46C018DA.1020309@trolltech.com","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-08-13T08:51:48Z","receivedAt":"2007-08-13T08:51:48Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Aug 13, 2007, at 10:39 AM, Marius Storm-Olsen wrote:\n\n> Steffen Prohaska said the following on 13.08.2007 09:32:\n>> On Aug 13, 2007, at 9:15 AM, Marius Storm-Olsen wrote:\n>>> Steffen Prohaska said the following on 13.08.2007 08:50:\n>>>> We should really make autocrlf = true the default for us and\n>>>> fix all problems that we'll encounter. There may be more tricky\n>>>> stuff ahead, like merges, cherry-picks, ...\n>>> I'm more leaning towards having the installer give you the option\n>>>  to choose what kind of line-endings you want Git to work with;\n>>> just like the Cygwin installer.\n>> Which is the root of much trouble with Cygwin. People now say, git\n>> works perfectly in Cygwin but forget to mention that they mean\n>> Cygwin A (in binmode) but not Cygwin B (in textmode).\n>> Better choose the right default and work hard to make the default\n>> choice work perfectly. I am strongly against an option in the\n>> installer. An option _will_ cause confusion. Better give people a\n>> hint how they can override the default for a single user, or for a\n>> single repo. Then they recognize that they move to a non-default\n>> configuration and hopefully think twice. And we never need to talk\n>> about msysgit A vs. msysgit B, but only about msysgit with repo\n>> specific or user specific options.\n>> For me, the question comes down to the following: What would the  \n>> average Windows user (real Windows user, not Linux user who was  \n>> forced to work in Cygwin!) expect git to do with line endings? The\n>> answer to this question should be the default.\n>\n> If we were talking about a huge amount (real) Windows users I would  \n> agree with you. However, currently most of the users using Git on  \n> Windows are Unix users which for some reason have to work on  \n> Windows every now and then. And changing the default option to  \n> autocrlf=true would be stepping on their toes, which we probably  \n> don't want to do :-)\n\nMy target audience of Git on Windows are Windows users and, frankly,\nthat is the only reasonable way to think about Windows. Why else should\nI boot Windows, if I don't have real Windows users in mind? I mean,\nWindows is not the superior platform to build Unix on top. The reason\nto boot Windows is Windows itself, including its real users.\n\n\n> I'm a Windows developer myself, so I naturally have autocrlf=true  \n> in my global settings. I don't think having the option in the  \n> installer (together with other things, like setting the global  \n> username, and email for example) would be such a bad thing. The  \n> problem with the way the Cygwin installer presents it is that it  \n> doesn't explain the pros and cons of the two options; it just  \n> recommends Linux EOL, which leads to confusion with some Windows  \n> developers.\n\nThe problem is that Cygwin doesn't really support textmode. It\noffers a choice, where there is no choice. After selecting textmode,\nI still can install git. But git doesn't work.\n\n\n> If we properly explain the issue in the installer, and say we  \n> recommend Windows EOL for Windows developers, I think it's OK. It  \n> would in any case be better than the current state where you have  \n> no option, or stepping on all the current msysgit/mingw-git  \n> maintainers toes.\n\nMaybe I don't fully understand what msysgit is about. I thought it would\nbe about real Windows support, which I think requires to accept what\nWindows users expect to be the right thing: Windows EOL.\n\nIn the long run it will also be easier for us, because other Windows\ntools expect Windows EOL. I'm pretty sure that git plays better on\nWindows if it has Window EOL on by default.\n\n\tSteffen\n"},{"id":"50635","messageId":"a1bbc6950708130735g73508d1ev2114cea3612d77c9@mail.gmail.com","threadId":"9506","inReplyTo":"DD0F51D8-390E-41DB-BD80-0BB440418D01@zib.de","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Dmitry Kakurin","fromEmail":"dmitry.kakurin@gmail.com","sentAt":"2007-08-13T14:35:40Z","receivedAt":"2007-08-13T14:35:40Z","isPatch":true,"sender":{"key":"dmitry.kakurin@gmail.com","avatar":null},"body":"On 8/13/07, Steffen Prohaska <prohaska@zib.de> wrote:\n> My target audience of Git on Windows are Windows users and, frankly,\n> that is the only reasonable way to think about Windows. Why else should\n> I boot Windows, if I don't have real Windows users in mind? I mean,\n> Windows is not the superior platform to build Unix on top. The reason\n> to boot Windows is Windows itself, including its real users.\n\nI agree with this approach.\n\n> Maybe I don't fully understand what msysgit is about. I thought it would\n> be about real Windows support, which I think requires to accept what\n> Windows users expect to be the right thing: Windows EOL.\n\nmsysgit is Build Environment for Git on Windows. It's purpose is to\nfacilitate Git development. This is not what end-user wants.\nAnother installer (WinGit) is targeting end users. It is just so\nhappens that msysgit is in better shape right now and more useful. But\nlong term it will not be the case.\n\nHere is another consideration: let's say I've started a new git repo\nunder Windows with no autocrlf set. Then my repo will contain crlf\nline endings.\nNow let's say that someone else checks out this repo with\nautocrlf=true. What would happen then? Will they get cr cr lf?\n-- \n- Dmitry\n"},{"id":"50636","messageId":"86fy2niiwh.fsf_-_@lola.quinscape.zz","threadId":"9506","inReplyTo":"86eji7lww5.fsf@lola.quinscape.zz","subject":"git-update-ref bug? (was: [PATCH 2/2] checkout: fix attribute handling in checkout all)","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-13T14:55:10Z","receivedAt":"2007-08-13T14:55:10Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>     $ git update-ref --no-deref HEAD $(git rev-parse v1.5.3-rc4^0)\n>\n> Is there a fundamental difference to using\n>\n> git-symbolic-ref HEAD $(git rev-parse v1.5.3-rc4^0)\n>\n> here?\n\nApart from the fact that the latter works, and the former doesn't\nbecause \"--no-deref\" is actually ignored?\n\n-- \nDavid Kastrup\n"},{"id":"50640","messageId":"Pine.LNX.4.64.0708131712030.25989@racer.site","threadId":"9506","inReplyTo":"8D126F5A-5998-4CB2-89BE-1CAEF5AE621F@zib.de","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-08-13T16:14:32Z","receivedAt":"2007-08-13T16:14:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 13 Aug 2007, Steffen Prohaska wrote:\n\n> Could we drop reading the file [.gitattributes] in the work tree \n> completely?\n\nNACK.\n\nIt is not good to hide things away from the working tree.  It is much \neasier to just edit a file than to edit it and put it into the index.\n\nCiao,\nDscho\n"},{"id":"50651","messageId":"7vtzr3fb26.fsf@assigned-by-dhcp.cox.net","threadId":"9506","inReplyTo":"86eji7lww5.fsf@lola.quinscape.zz","subject":"Re: [PATCH 2/2] checkout: fix attribute handling in checkout all","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-13T20:12:49Z","receivedAt":"2007-08-13T20:12:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>     $ git update-ref --no-deref HEAD $(git rev-parse v1.5.3-rc4^0)\n>\n> Is there a fundamental difference to using\n>\n> git-symbolic-ref HEAD $(git rev-parse v1.5.3-rc4^0)\n>\n> here?\n\nThe symbolic-ref command is about setting the HEAD to \"point at\na(nother) ref\".  There is no point talking about \"fundamental\ndifference\" here --- the latter is plain wrong, feeding\nrev-parse output (which is an object name) as its second\nparameter.\n\nDid you mean to ask about the difference between \"git-update-ref\nHEAD $param\" with or without --no-deref?\n\nWith --no-deref, it makes the HEAD detached even when HEAD is a\nsymref that points at a ref, e.g. \"refs/heads/master\".  Without\nthat option, it updates the ref that is pointed at by the HEAD\nsymref.\n"},{"id":"50687","messageId":"7v1we6ecfm.fsf_-_@assigned-by-dhcp.cox.net","threadId":"9506","inReplyTo":"46BFFB1A.4070704@trolltech.com","subject":"[PATCH 1/2] attr.c: refactoring","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-14T08:40:45Z","receivedAt":"2007-08-14T08:40:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This splits out a common routine that parses a single line of\nattribute file and adds it to the attr_stack.  It should not\nchange any behaviour, other than attrs array in the attr_stack\nstructure is now grown with alloc_nr() macro, instead of one by\none, which relied on xrealloc() to give enough slack to be\nefficient enough.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n Marius Storm-Olsen <marius@trolltech.com> writes:\n\n >> This essentially treats not having .gitattributes files checked\n >> out as equivalent to having these files checked out unmodified,\n >> which is very much in line with how the world is designed to\n >> work.\n >\n > ACK! We really need this! :-)\n\n So this is the first part, which does not do much.  The second\n one is interesting.\n\n attr.c |   67 +++++++++++++++++++++++++++++++++++++++------------------------\n 1 files changed, 41 insertions(+), 26 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex a071254..8d778f1 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -257,6 +257,7 @@ static struct attr_stack {\n \tstruct attr_stack *prev;\n \tchar *origin;\n \tunsigned num_matches;\n+\tunsigned alloc;\n \tstruct match_attr **attrs;\n } *attr_stack;\n \n@@ -287,6 +288,26 @@ static const char *builtin_attr[] = {\n \tNULL,\n };\n \n+static void handle_attr_line(struct attr_stack *res,\n+\t\t\t     const char *line,\n+\t\t\t     const char *src,\n+\t\t\t     int lineno,\n+\t\t\t     int macro_ok)\n+{\n+\tstruct match_attr *a;\n+\n+\ta = parse_attr_line(line, src, lineno, macro_ok);\n+\tif (!a)\n+\t\treturn;\n+\tif (res->alloc <= res->num_matches) {\n+\t\tres->alloc = alloc_nr(res->num_matches);\n+\t\tres->attrs = xrealloc(res->attrs,\n+\t\t\t\t      sizeof(struct match_attr *) *\n+\t\t\t\t      res->alloc);\n+\t}\n+\tres->attrs[res->num_matches++] = a;\n+}\n+\n static struct attr_stack *read_attr_from_array(const char **list)\n {\n \tstruct attr_stack *res;\n@@ -294,42 +315,34 @@ static struct attr_stack *read_attr_from_array(const char **list)\n \tint lineno = 0;\n \n \tres = xcalloc(1, sizeof(*res));\n-\twhile ((line = *(list++)) != NULL) {\n-\t\tstruct match_attr *a;\n-\n-\t\ta = parse_attr_line(line, \"[builtin]\", ++lineno, 1);\n-\t\tif (!a)\n-\t\t\tcontinue;\n-\t\tres->attrs = xrealloc(res->attrs,\n-\t\t\tsizeof(struct match_attr *) * (res->num_matches + 1));\n-\t\tres->attrs[res->num_matches++] = a;\n-\t}\n+\twhile ((line = *(list++)) != NULL)\n+\t\thandle_attr_line(res, line, \"[builtin]\", ++lineno, 1);\n \treturn res;\n }\n \n static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)\n {\n-\tFILE *fp;\n+\tFILE *fp = fopen(path, \"r\");\n \tstruct attr_stack *res;\n \tchar buf[2048];\n \tint lineno = 0;\n \n-\tres = xcalloc(1, sizeof(*res));\n-\tfp = fopen(path, \"r\");\n \tif (!fp)\n-\t\treturn res;\n+\t\treturn NULL;\n+\tres = xcalloc(1, sizeof(*res));\n+\twhile (fgets(buf, sizeof(buf), fp))\n+\t\thandle_attr_line(res, buf, path, ++lineno, macro_ok);\n+\tfclose(fp);\n+\treturn res;\n+}\n \n-\twhile (fgets(buf, sizeof(buf), fp)) {\n-\t\tstruct match_attr *a;\n+static struct attr_stack *read_attr(const char *path, int macro_ok)\n+{\n+\tstruct attr_stack *res;\n \n-\t\ta = parse_attr_line(buf, path, ++lineno, macro_ok);\n-\t\tif (!a)\n-\t\t\tcontinue;\n-\t\tres->attrs = xrealloc(res->attrs,\n-\t\t\tsizeof(struct match_attr *) * (res->num_matches + 1));\n-\t\tres->attrs[res->num_matches++] = a;\n-\t}\n-\tfclose(fp);\n+\tres = read_attr_from_file(path, macro_ok);\n+\tif (!res)\n+\t\tres = xcalloc(1, sizeof(*res));\n \treturn res;\n }\n \n@@ -370,13 +383,15 @@ static void bootstrap_attr_stack(void)\n \t\telem->prev = attr_stack;\n \t\tattr_stack = elem;\n \n-\t\telem = read_attr_from_file(GITATTRIBUTES_FILE, 1);\n+\t\telem = read_attr(GITATTRIBUTES_FILE, 1);\n \t\telem->origin = strdup(\"\");\n \t\telem->prev = attr_stack;\n \t\tattr_stack = elem;\n \t\tdebug_push(elem);\n \n \t\telem = read_attr_from_file(git_path(INFOATTRIBUTES_FILE), 1);\n+\t\tif (!elem)\n+\t\t\telem = xcalloc(1, sizeof(*elem));\n \t\telem->origin = NULL;\n \t\telem->prev = attr_stack;\n \t\tattr_stack = elem;\n@@ -441,7 +456,7 @@ static void prepare_attr_stack(const char *path, int dirlen)\n \t\tmemcpy(pathbuf + dirlen, \"/\", 2);\n \t\tcp = strchr(pathbuf + len + 1, '/');\n \t\tstrcpy(cp + 1, GITATTRIBUTES_FILE);\n-\t\telem = read_attr_from_file(pathbuf, 0);\n+\t\telem = read_attr(pathbuf, 0);\n \t\t*cp = '\\0';\n \t\telem->origin = strdup(pathbuf);\n \t\telem->prev = attr_stack;\n-- \n1.5.3.rc4.89.g18078\n"},{"id":"50688","messageId":"7vwsvycxup.fsf_-_@assigned-by-dhcp.cox.net","threadId":"9506","inReplyTo":"46BFFB1A.4070704@trolltech.com","subject":"[PATCH 2/2] attr.c: read .gitattributes from index as well.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-14T08:41:02Z","receivedAt":"2007-08-14T08:41:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This makes .gitattributes files to be read from the index when\nthey are not checked out to the work tree.  This is in line with\nthe way we always allowed low-level tools to operate in sparsely\nchecked out work tree in a reasonable way.\n\nIt swaps the order of new file creation and converting the blob\nto work tree representation; otherwise when we are in the middle\nof checking out .gitattributes we would notice an empty but\nunwritten .gitattributes file in the work tree and will ignore\nthe copy in the index.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n attr.c          |   61 ++++++++++++++++++++++++++++++++++++++++-\n entry.c         |   19 +++++++------\n t/t0020-crlf.sh |   81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 150 insertions(+), 11 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 8d778f1..1293993 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -336,13 +336,70 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)\n \treturn res;\n }\n \n+static void *read_index_data(const char *path)\n+{\n+\tint pos, len;\n+\tunsigned long sz;\n+\tenum object_type type;\n+\tvoid *data;\n+\n+\tlen = strlen(path);\n+\tpos = cache_name_pos(path, len);\n+\tif (pos < 0) {\n+\t\t/*\n+\t\t * We might be in the middle of a merge, in which\n+\t\t * case we would read stage #2 (ours).\n+\t\t */\n+\t\tint i;\n+\t\tfor (i = -pos - 1;\n+\t\t     (pos < 0 && i < active_nr &&\n+\t\t      !strcmp(active_cache[i]->name, path));\n+\t\t     i++)\n+\t\t\tif (ce_stage(active_cache[i]) == 2)\n+\t\t\t\tpos = i;\n+\t}\n+\tif (pos < 0)\n+\t\treturn NULL;\n+\tdata = read_sha1_file(active_cache[pos]->sha1, &type, &sz);\n+\tif (!data || type != OBJ_BLOB) {\n+\t\tfree(data);\n+\t\treturn NULL;\n+\t}\n+\treturn data;\n+}\n+\n static struct attr_stack *read_attr(const char *path, int macro_ok)\n {\n \tstruct attr_stack *res;\n+\tchar *buf, *sp;\n+\tint lineno = 0;\n \n \tres = read_attr_from_file(path, macro_ok);\n-\tif (!res)\n-\t\tres = xcalloc(1, sizeof(*res));\n+\tif (res)\n+\t\treturn res;\n+\n+\tres = xcalloc(1, sizeof(*res));\n+\n+\t/*\n+\t * There is no checked out .gitattributes file there, but\n+\t * we might have it in the index.  We allow operation in a\n+\t * sparsely checked out work tree, so read from it.\n+\t */\n+\tbuf = read_index_data(path);\n+\tif (!buf)\n+\t\treturn res;\n+\n+\tfor (sp = buf; *sp; ) {\n+\t\tchar *ep;\n+\t\tint more;\n+\t\tfor (ep = sp; *ep && *ep != '\\n'; ep++)\n+\t\t\t;\n+\t\tmore = (*ep == '\\n');\n+\t\t*ep = '\\0';\n+\t\thandle_attr_line(res, sp, path, ++lineno, macro_ok);\n+\t\tsp = ep + more;\n+\t}\n+\tfree(buf);\n \treturn res;\n }\n \ndiff --git a/entry.c b/entry.c\nindex 0625112..fc3a506 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -112,6 +112,16 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout\n \t\tif (!new)\n \t\t\treturn error(\"git-checkout-index: unable to read sha1 file of %s (%s)\",\n \t\t\t\tpath, sha1_to_hex(ce->sha1));\n+\n+\t\t/*\n+\t\t * Convert from git internal format to working tree format\n+\t\t */\n+\t\tbuf = convert_to_working_tree(ce->name, new, &size);\n+\t\tif (buf) {\n+\t\t\tfree(new);\n+\t\t\tnew = buf;\n+\t\t}\n+\n \t\tif (to_tempfile) {\n \t\t\tstrcpy(path, \".merge_file_XXXXXX\");\n \t\t\tfd = mkstemp(path);\n@@ -123,15 +133,6 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout\n \t\t\t\tpath, strerror(errno));\n \t\t}\n \n-\t\t/*\n-\t\t * Convert from git internal format to working tree format\n-\t\t */\n-\t\tbuf = convert_to_working_tree(ce->name, new, &size);\n-\t\tif (buf) {\n-\t\t\tfree(new);\n-\t\t\tnew = buf;\n-\t\t}\n-\n \t\twrote = write_in_full(fd, new, size);\n \t\tclose(fd);\n \t\tfree(new);\ndiff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\nindex fe1dfd0..0807d9f 100755\n--- a/t/t0020-crlf.sh\n+++ b/t/t0020-crlf.sh\n@@ -290,4 +290,85 @@ test_expect_success '.gitattributes says two and three are text' '\n \tfi\n '\n \n+test_expect_success 'in-tree .gitattributes (1)' '\n+\n+\techo \"one -crlf\" >>.gitattributes &&\n+\tgit add .gitattributes &&\n+\tgit commit -m \"Add .gitattributes\" &&\n+\n+\trm -rf tmp one dir .gitattributes patch.file three &&\n+\tgit read-tree --reset -u HEAD &&\n+\n+\tif remove_cr one >/dev/null\n+\tthen\n+\t\techo \"Eh? one should not have CRLF\"\n+\t\tfalse\n+\telse\n+\t\t: happy\n+\tfi &&\n+\tremove_cr three >/dev/null || {\n+\t\techo \"Eh? three should still have CRLF\"\n+\t\tfalse\n+\t}\n+'\n+\n+test_expect_success 'in-tree .gitattributes (2)' '\n+\n+\trm -rf tmp one dir .gitattributes patch.file three &&\n+\tgit read-tree --reset HEAD &&\n+\tgit checkout-index -f -q -u -a &&\n+\n+\tif remove_cr one >/dev/null\n+\tthen\n+\t\techo \"Eh? one should not have CRLF\"\n+\t\tfalse\n+\telse\n+\t\t: happy\n+\tfi &&\n+\tremove_cr three >/dev/null || {\n+\t\techo \"Eh? three should still have CRLF\"\n+\t\tfalse\n+\t}\n+'\n+\n+test_expect_success 'in-tree .gitattributes (3)' '\n+\n+\trm -rf tmp one dir .gitattributes patch.file three &&\n+\tgit read-tree --reset HEAD &&\n+\tgit checkout-index -u .gitattributes &&\n+\tgit checkout-index -u one dir/two three &&\n+\n+\tif remove_cr one >/dev/null\n+\tthen\n+\t\techo \"Eh? one should not have CRLF\"\n+\t\tfalse\n+\telse\n+\t\t: happy\n+\tfi &&\n+\tremove_cr three >/dev/null || {\n+\t\techo \"Eh? three should still have CRLF\"\n+\t\tfalse\n+\t}\n+'\n+\n+test_expect_success 'in-tree .gitattributes (4)' '\n+\n+\trm -rf tmp one dir .gitattributes patch.file three &&\n+\tgit read-tree --reset HEAD &&\n+\tgit checkout-index -u one dir/two three &&\n+\tgit checkout-index -u .gitattributes &&\n+\n+\tif remove_cr one >/dev/null\n+\tthen\n+\t\techo \"Eh? one should not have CRLF\"\n+\t\tfalse\n+\telse\n+\t\t: happy\n+\tfi &&\n+\tremove_cr three >/dev/null || {\n+\t\techo \"Eh? three should still have CRLF\"\n+\t\tfalse\n+\t}\n+'\n+\n test_done\n-- \n1.5.3.rc4.89.g18078\n"}]}