{"thread":{"id":"44214","subject":"Merge conflicts in .gitattributes can cause trouble","startedAt":"2016-10-04T10:19:18Z","lastAt":"2016-10-18T17:30:22Z","messageCount":6,"participants":["Lars Schneider","Duy Nguyen","Johannes Schindelin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"303197","messageId":"248A6E81-8D5C-4183-9756-51A0D5193E3E@gmail.com","threadId":"44214","inReplyTo":null,"subject":"Merge conflicts in .gitattributes can cause trouble","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-10-04T10:19:09Z","receivedAt":"2016-10-04T10:19:18Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"Hi,\n\n\nIf there is a conflict in the .gitattributes during a merge then it looks \nlike as if the attributes are not applied (which kind of makes sense as Git \nwould not know what to do). As a result Git can treat e.g. binary files \nas text and they can end up with changed line endings in the working tree. \nAfter resolving the conflict in .gitattributes all files would be marked \nas binary, again, and the user can easily commit the wrongly changed line \nendings.\n\nConsider this script on Windows:\n\n$ git init .\n$ touch first.commit\n$ git add .\n$ git commit -m \"first commit\"\n\n$ git checkout -b branch\n$ printf \"*.bin binary\\n\" >> .gitattributes\n$ git add .\n$ git commit -m \"tracking *.bin files\"\n\n$ git checkout master\n$ printf \"binary\\ndata\\n\" > file.dat # <-- Unix line ending!\n$ printf \"*.dat binary\\n\" >> .gitattributes # <-- Tell Git to keep Unix line ending!\n$ git add .\n$ git commit -m \"tracking *.dat files\"\n$ git cat-file -p :file.dat | od -c\n0000000   b   i   n   a   r   y  \\n   d   a   t   a  \\n \n                                ^^^^                ^^^^  <-- Correct!\n$ git checkout branch\n$ git merge master # <-- Causes merge conflict!\n$ printf \"*.bin binary\\n*.dat binary\\n\" > .gitattributes # <-- Fix merge conflict!\n$ git add .\n$ git commit -m \"merged\"\n$ git cat-file -p :file.dat | od -c\n0000000   b   i   n   a   r   y  \\r  \\n   d   a   t   a  \\r  \\n\n                                ^^^^^^^^                ^^^^^^^^  <-- Wrong!\n\nPossible solutions:\n\n1. We could print an appropriate warning if we detect a merge conflict \n   in .gitattributes\n\n2. We could disable all line ending conversions in case of a merge conflict\n   (I am not exactly sure about all the implications, though)\n\n3. We could salvage what we could of the .gitattributes file, \n   perhaps by using the version from HEAD (or more likely, the ours stage of\n   the index) -- suggested by Peff on the related GitHub issue mentioned below\n\nThoughts?\n\nThanks,\nLars\n\n\nPS: I noticed that behavior while working with Git LFS and started a discussion\nabout it here: https://github.com/github/git-lfs/issues/1544 "},{"id":"303201","messageId":"CACsJy8CSHepObhcSYHOYgY0oev6wEeNgpy+x1_c_QhLtLA8Xzg@mail.gmail.com","threadId":"44214","inReplyTo":"248A6E81-8D5C-4183-9756-51A0D5193E3E@gmail.com","subject":"Re: Merge conflicts in .gitattributes can cause trouble","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-10-04T11:26:30Z","receivedAt":"2016-10-04T11:27:06Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 4, 2016 at 5:19 PM, Lars Schneider <larsxschneider@gmail.com> wrote:\n> Hi,\n>\n>\n> If there is a conflict in the .gitattributes during a merge then it looks\n> like as if the attributes are not applied (which kind of makes sense as Git\n> would not know what to do). As a result Git can treat e.g. binary files\n> as text and they can end up with changed line endings in the working tree.\n> After resolving the conflict in .gitattributes all files would be marked\n> as binary, again, and the user can easily commit the wrongly changed line\n> endings.\n>\n> Consider this script on Windows:\n>\n> $ git init .\n> $ touch first.commit\n> $ git add .\n> $ git commit -m \"first commit\"\n>\n> $ git checkout -b branch\n> $ printf \"*.bin binary\\n\" >> .gitattributes\n> $ git add .\n> $ git commit -m \"tracking *.bin files\"\n>\n> $ git checkout master\n> $ printf \"binary\\ndata\\n\" > file.dat # <-- Unix line ending!\n> $ printf \"*.dat binary\\n\" >> .gitattributes # <-- Tell Git to keep Unix line ending!\n> $ git add .\n> $ git commit -m \"tracking *.dat files\"\n> $ git cat-file -p :file.dat | od -c\n> 0000000   b   i   n   a   r   y  \\n   d   a   t   a  \\n\n>                                 ^^^^                ^^^^  <-- Correct!\n> $ git checkout branch\n> $ git merge master # <-- Causes merge conflict!\n> $ printf \"*.bin binary\\n*.dat binary\\n\" > .gitattributes # <-- Fix merge conflict!\n> $ git add .\n> $ git commit -m \"merged\"\n> $ git cat-file -p :file.dat | od -c\n> 0000000   b   i   n   a   r   y  \\r  \\n   d   a   t   a  \\r  \\n\n>                                 ^^^^^^^^                ^^^^^^^^  <-- Wrong!\n>\n> Possible solutions:\n>\n> 1. We could print an appropriate warning if we detect a merge conflict\n>    in .gitattributes\n\nThis is good regardless, to encourage people to resolve conflicts in\n.gitattributes first. A good place for this warning may be \"git\nstatus\"?\n\n> 2. We could disable all line ending conversions in case of a merge conflict\n>    (I am not exactly sure about all the implications, though)\n>\n> 3. We could salvage what we could of the .gitattributes file,\n>    perhaps by using the version from HEAD (or more likely, the ours stage of\n>    the index) -- suggested by Peff on the related GitHub issue mentioned below\n\nWe already have code to fall back to index version in some cases,\nadding \"fall back on merge conflicts\" (and probably updating the index\nlookup code too because it looks for stage 0 now) sounds reasonable\n(especially with the warning in #1).\n\nBTW whoever fixes this probably should do the same for .gitignore files.\n-- \nDuy\n"},{"id":"304279","messageId":"alpine.DEB.2.20.1610171803400.197091@virtualbox","threadId":"44214","inReplyTo":"248A6E81-8D5C-4183-9756-51A0D5193E3E@gmail.com","subject":"Re: Merge conflicts in .gitattributes can cause trouble","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-17T16:11:00Z","receivedAt":"2016-10-17T16:11:26Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Lars,\n\nOn Tue, 4 Oct 2016, Lars Schneider wrote:\n\n> If there is a conflict in the .gitattributes during a merge then it looks \n> like as if the attributes are not applied\n\nI tried to replicate this behavior, to the point where I wrote a patch\nthat demonstrates the breakage so I could single-step in a debugger to\nfind out where things go wrong, and fix them.\n\nAlas, I found out that the .gitattributes are read *before* any merge\nconflict arises in the case you demonstrated. Which kind of makes sense,\nbecause the gitattributes decide over which merge driver to use, among\nother things.\n\nSo in your example:\n\n> Consider this script on Windows:\n> \n> $ git init .\n> $ touch first.commit\n> $ git add .\n> $ git commit -m \"first commit\"\n> \n> $ git checkout -b branch\n> $ printf \"*.bin binary\\n\" >> .gitattributes\n> $ git add .\n> $ git commit -m \"tracking *.bin files\"\n> \n> $ git checkout master\n> $ printf \"binary\\ndata\\n\" > file.dat # <-- Unix line ending!\n> $ printf \"*.dat binary\\n\" >> .gitattributes # <-- Tell Git to keep Unix line ending!\n> $ git add .\n> $ git commit -m \"tracking *.dat files\"\n> $ git cat-file -p :file.dat | od -c\n> 0000000   b   i   n   a   r   y  \\n   d   a   t   a  \\n \n>                                 ^^^^                ^^^^  <-- Correct!\n> $ git checkout branch\n\nAt this point, the .gitattributes list only .bin files as binary. That is\nthe revision of the .gitattributes used by this command:\n\n> $ git merge master # <-- Causes merge conflict!\n\nAnd as a consequence, the .gitattributes do not tell Git that it should\nhandle .dat files as binary. Which means that...\n\n> $ printf \"*.bin binary\\n*.dat binary\\n\" > .gitattributes # <-- Fix merge conflict!\n> $ git add .\n> $ git commit -m \"merged\"\n> $ git cat-file -p :file.dat | od -c\n> 0000000   b   i   n   a   r   y  \\r  \\n   d   a   t   a  \\r  \\n\n>                                 ^^^^^^^^                ^^^^^^^^  <-- Wrong!\n\n... this is actually expected! Why? Because the .gitattributes that were\nin effect when the user asked to perform a merge said so.\n\nIf you adjust .gitattributes *before* merging `master`, it works as you\nwould expect: the line endings are not changed.\n\nThe reason to do it this way: we want to respect the .gitattributes as per\nthe current worktree. We go even so far that we respect uncommitted\nchanges to said file...\n\n> Possible solutions:\n> \n> 1. We could print an appropriate warning if we detect a merge conflict \n>    in .gitattributes\n> \n> 2. We could disable all line ending conversions in case of a merge conflict\n>    (I am not exactly sure about all the implications, though)\n> \n> 3. We could salvage what we could of the .gitattributes file, \n>    perhaps by using the version from HEAD (or more likely, the ours stage of\n>    the index) -- suggested by Peff on the related GitHub issue mentioned below\n\nI would vote for:\n\n4. We keep letting Git read in the *current* version of .gitattributes\n   *before* the merge, and apply those attributes while performing the\n   merge.\n\nCiao,\nDscho\n"},{"id":"304291","messageId":"xmqqvawqvp6y.fsf@gitster.mtv.corp.google.com","threadId":"44214","inReplyTo":"alpine.DEB.2.20.1610171803400.197091@virtualbox","subject":"Re: Merge conflicts in .gitattributes can cause trouble","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-17T17:51:49Z","receivedAt":"2016-10-17T17:51:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> I would vote for:\n>\n> 4. We keep letting Git read in the *current* version of .gitattributes\n>    *before* the merge, and apply those attributes while performing the\n>    merge.\n\nEven though this needs a major surgery to the way the attr subsystem\nreads from these files, I think it is conceptually the cleanest.\n"},{"id":"304335","messageId":"alpine.DEB.2.20.1610181437180.197091@virtualbox","threadId":"44214","inReplyTo":"xmqqvawqvp6y.fsf@gitster.mtv.corp.google.com","subject":"Re: Merge conflicts in .gitattributes can cause trouble","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-18T12:39:04Z","receivedAt":"2016-10-18T12:39:29Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 17 Oct 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > I would vote for:\n> >\n> > 4. We keep letting Git read in the *current* version of .gitattributes\n> >    *before* the merge, and apply those attributes while performing the\n> >    merge.\n> \n> Even though this needs a major surgery to the way the attr subsystem\n> reads from these files, I think it is conceptually the cleanest.\n\nTo the contrary. As far as I can see, when calling `git merge`, Git\ncurrently *does* read .gitattributes from the file, and if that fails,\nfalls back to reading that file from the index.\n\nIn other words, option 4. is the current behavior no change required.\n\nCiao,\nDscho\n"},{"id":"304352","messageId":"xmqqmvi1pntm.fsf@gitster.mtv.corp.google.com","threadId":"44214","inReplyTo":"alpine.DEB.2.20.1610181437180.197091@virtualbox","subject":"Re: Merge conflicts in .gitattributes can cause trouble","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-18T17:30:13Z","receivedAt":"2016-10-18T17:30:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> To the contrary. As far as I can see, when calling `git merge`, Git\n> currently *does* read .gitattributes from the file, and if that fails,\n> falls back to reading that file from the index.\n\nHmph.\n\nAssuming that the merge always goes in the index order, I think you\nare right.  When we need to merge path/to/dir/.gitattributes, we\nwould need to know all the .gitattributes files that may affect that\npath, so .gitattributes, path/.gitattributes, path/to/.gitattributes\nand the file being merged are all read into core before anything\nhappens and these are kept in attr_stack while merging anything\nunderneath path/to/dir/ hierarchy without being re-read from the\nfilesystem.  The original contents of path/to/dir/.gitattributes\ncached in the attr_stack will be discarded when we start to merge\nthings outside path/to/dir (e.g. merging path/to/another/file), but\nat that point the contents of path/to/dir/.gitattributes no longer\nmatters to the path being merged, so unless the merge somehow jumps\naround it is OK.\n\nIt may be a fragile assumption in the longer term that the merge\nalways goes in the index order, but I think the assumption holds in\nthe current codebase, and the update planned immediately in the\nfuture.\n\n"}]}