Introduce .gitattributes to end the CRLF/LF and binary diff horror, and detect/auto-fix whitespace errors

How about adding the following?

.gitignore export-ignore

Then we could close #1170538: Rename core .gitignore file to example.gitignore and add explanatory comments as a duplicate.

Question. Since #917492: Add a sensible .gitignore file was well-intentioned, but ended up royally screwing a number of people by enforcing incorrect assumptions on them, how likely is this addition to do the same? Is it fairly common that someone deploying sites from Git would have their own custom .gitattributes doing their own special things, which we would then obliterate by adding this change?

Well, if you're deploying by git, then you can also maintain a local branch and merge with origin. That's what git is for, no?

Well, I think people must have different workflows they use, at least one of which collides very badly with core adding files like this.

See the discussion at #1170538: Rename core .gitignore file to example.gitignore and add explanatory comments as well as http://randyfay.com/node/102

It might be that .gitattributes is a much less-used file and doesn't suffer from these kinds of potential complications, but I'll just say I'm leery about core forcing any assumptions until some of these workflow issues become more smoothed out.

In particular, we could specify the file type for certain extensions (like *.module is a PHP file) so that the hunk headers are more useful.

@bfroehle: It would be great if you or someone else can provide an example about how different the git diff hunks will be, so we can take an informed decision.

Better title :)

Stumbling over Mind the End of your Line, the new .gitattributes can do a lot more for the benefit of our project's contributors.

I carefully crafted a .gitattributes file for Drupal core, including docs that explain what each flag is doing.

The list of the file extensions is complete; it was auto-generated on HEAD with a little command line fu:

find core/ -type f ! -path '*vendor*' | awk -F'.' '{print $NF}' | sort | uniq

Adding a nice little new tag :)

This is still something people would have to add globally to cover all their various contrib repos too though? Would be worth announcing it on planet/front page for that.

oh, and to answer @marvil07's concern in #9:

By default, git diff does not use any special algorithm for detecting the proper diff context, even if the file extension is .php. I've seen countless of patches that contain hunk headers like the following, and I cannot express how awkward they are for reviewers without using explicit language...

@@ -604,6 +610,14 @@ abstract class TestBase {

With the added .gitattributes, git diff suddenly understands PHP, and you see hunk headers like this:

@@ -693,7 +701,10 @@ protected function prepareEnvironment() {

Check the patches in #56 and #57 on #1215104-56: Use the non-interactive installer in WebTestBase::setUp() to see it yourself. :)

Hm. Last time we tried to help people by pre-packaging Git configuration files like this #1170538: Rename core .gitignore file to example.gitignore and add explanatory comments, it caused all manner of problems/workarounds. :( I think if we're going to do this, it needs to be example.gitattributes ala what we did with .gitignore in D8.

Thanks for the explanatory link on 10. It totally make sense to use git to help us deal with sharing code in an easier way.

I have some minor comments:

+++ b/.gitattributes
@@ -0,0 +1,50 @@
+# @see http://schacon.github.com/git/gitattributes.html

Better a link to official docs http://git-scm.com/docs/gitattributes

+++ b/.gitattributes
@@ -0,0 +1,50 @@
+*.js      drupaltext diff=java

java means java code, not javascript, are you sure you want to set to that?

One other thing I think is missing is to exclude vendor directory. It seems like .gitattributes have only matching rules, but there seems to not be an easy way to exclude. So, I guess we need to define the the matching rules to prevent applying them to vendor directory files.

PS: I really like that new tag!

#14: On example.gitattributes vs. .gitattributes:

I doubt that's going to be necessary here. The file essentially does two things:

1) It ensures and enforces that files are checked out and in with proper line-endings on all platforms (LF vs. CRLF). This is the standard, not only for Drupal. Most git clients should actually be configured correctly by default already, but given how many patches we see in the queue that have CRLF line-endings in the mix, there still must be a couple of misconfigured git clients.

2) It configures git diff to treat the right files as text, and the right files as binary, using the proper diff algorithm, plus .patch support, for each.

#15:

- The funny and strange thing is that those official docs pages are not parsed and formatted correctly... we can change the link, but the bogus and overly fancy output makes me vote -1 on that. http://schacon.github.com/git/git.html mirrors the latest git manual pages AFAIK, and the output is identical to what people are seeing in their local git manual.

- There's no diff algorithm for javascript, so java comes syntactically closest. Definitely better than nothing.

- As mentioned in 1) above, there's no need to exclude vendor directories, since at least with regard to this, this is a widely adopted standard, especially by open-source projects. In any case, the attributes affect the local checkout only. Git stores everything in LF internally.

Thanks for the answers!

- The funny and strange thing is that those official docs pages are not parsed and formatted correctly... we can change the link, but the bogus and overly fancy output makes me vote -1 on that. http://schacon.github.com/git/git.html mirrors the latest git manual pages AFAIK, and the output is identical to what people are seeing in their local git manual.

Then, let's use kernel.org, which use also the default asciidoc theme and have more recent docs(see last updated at the bottom):
http://www.kernel.org/pub/software/scm/git/docs/gitattributes.html

- There's no diff algorithm for javascript, so java comes syntactically closest. Definitely better than nothing.

Make sense. Maybe it worths a little comment ;-)

- As mentioned in 1) above, there's no need to exclude vendor directories, since at least with regard to this, this is a widely adopted standard, especially by open-source projects. In any case, the attributes affect the local checkout only. Git stores everything in LF internally.

OK. That means for example, if we are adding one new project to vendor, we will be changing its line ending to conform our standard instead of using what they use. I just wanted to mention it, and I am fine with that, we always have git diff --ignore-all-space.

OK. That means for example, if we are adding one new project to vendor, we will be changing its line ending to conform our standard instead of using what they use. I just wanted to mention it, and I am fine with that, we always have git diff --ignore-all-space.

I guess we ultimately won't notice, regardless of what the project's line-endings are. That is, because the .gitattributes ensure that all input and output is in one style only.

Here the two little changes mentioned on comment 17

Updated the documentation for js java diff usage.

#20: gitattributes.20.patch queued for re-testing.

Awesome patch. Wish I'd actually read it when I retested it.

To address @webchick's concerns, here's the text from the summary of #1170538: Rename core .gitignore file to example.gitignore and add explanatory comments:

Today, I decided to update my project's code, so I ran the drush make file again (which downloads projects latest versions, except for those we specified a version number). Among them, it downloads Drupal 7.2, and its new .gitignore file, overwriting ours.
If I hadn't taken care during my next commit, I would have commited ALL project files (except the ones ignored by Drupal 7.2 .gitignore file) ! Which is what I called Git bombing on the issue title.

Moreover, it also means that now, as .gitignore is part of the Drupal core project, having one's own .gitignore file could be considered as a core hack (as it implies manual maintenance and ports over Drupal versions).

Here's why I think this is a different situation:

• We are not backporting this to a production version of Drupal. The issue from #1170538: Rename core .gitignore file to example.gitignore and add explanatory comments was caused when the new .gitignore overwrote production code. We're not doing that here.
• #1170538: Rename core .gitignore file to example.gitignore and add explanatory comments altered the behavior of the repository toward the files it contained. In contrast, all this issue does is refine diff generation and ensure that whitespace and binary files are handled correctly when diffing. I can't think of any reason to have core repos that don't do those things.

For the sake of completeness, to get this in, let's have some folks do some manual testing and document the before-and-after. We don't even need to worry about the case where the user has a .gitattributes in the repo before we add this, because we're going to assume D8 ships with this.

Testing for any platform

• Add a whitespace error to a file and do git diff --color before applying the patch. Apply the patch and repeat.
• Make a small change to a method in a class file and do a git diff before applying the patch. Apply the patch and repeat. You can post before-and-after code snippets to illustrate the difference for reviewers.
• Change a binary file and do a git diff before applying the patch. Apply the patch and repeat. (You can code snippets, but truncate the binary data to just a couple lines. We don't need an entire binary file in ASCII in an issue comment.) :)
• Create a patch that adds whitespace errors to a file. Reset, apply the patch from #20, and then try to apply your whitespace-error-adding patch. Document the result.
• With #20 applied, try to create a new patch with new whitespace errors. Document the result.
• (Advanced) Add a global .gitattributes file to your .gitconfig, and add some rules to it that are different from the ones in the patch. Document the results of using this with the patch.

Windows testing

• Add proper newlines to files before and after the patch and document the result. Ensure the patch is still generated correctly.
• With #20 applied, add CRLF line endings to a file and try to create a patch. Document the result.
• Without the patch applied, create a patch with CRLF line endings. Upload it to this issue (named something like crlf-do-not-test.patch) for other Windows and non-Windows users to test with.

The fact that most of the issues w/ .gitignore were because it was in 7.2 vs. 7.0 is a good point. It's also a good point that this is generally speaking, git attributes affect much less destructive stuff than .gitignore.

I would nevertheless still love to see at least one report back on that "advanced" test case before committing this.

I am on windows and thanks to Eclipse I face newline issues constantly. Will perform the tests in #22 and report.

Tested whitespace errors before and after patch, and because my work environment already highlights whitespace errors in diffs there really isn't much visual change here, however xjm told me this was still relevant and that I should post it. I suppose the thought is that since nothing changed between what my default settings are and what the patch did that the patch is working as expected there.

I have some more tests that I did where I created a patch with whitespace errors before and after the patch presented in 20 which I'll post later as soon as I confirm that I get the same result more than once.

On testing a binary file, should I just modify a jpg or something to git diff?

Attached is the before and after of a git diff --color filename before/after patch in #20 within my working environment.

Windows XP, git version 1.7.11.msysgit.0

Note: I had to convert newlines from CRLF to LF just to stage the .gitattributes file locally, otherwise:

Error: "fatal: CRLF would be replaced by LF in .gitattributes."

Also commenting my global autocrlf/safecrlf settings (both TRUE) worked.

• Add a whitespace error to a file and do git diff --color before applying the patch. Apply the patch and repeat.

  Before: trailing whitespaces outlined on terminal.
  After: trailing whitespaces outlined on terminal.

• Make a small change to a method in a class file and do a git diff before applying the patch. Apply the patch and repeat. You can post before-and-after code snippets to illustrate the difference for reviewers.

  Before:

  Patch snippet:

  @@ -190,6 +190,7 @@ class EntityFormController implements EntityFormControllerInterface {
      */
     public function save(array $form, array &$form_state) {
       // @todo Perform common save operations.
  +    // @todo Is 'save' a proper name for a submit handler?
     }
  

  After:

  Terminal output:

  warning: CRLF will be replaced by LF in core/lib/Drupal/Core/Entity/EntityFormController.php.
  The file will have its original line endings in your working directory.
  

  Patch snippet:

  @@ -190,6 +190,7 @@ public function submit(array $form, array &$form_state) {
      */
     public function save(array $form, array &$form_state) {
       // @todo Perform common save operations.
  +    // @todo Is 'save' a proper name for a submit handler?
     }
  

• Change a binary file and do a git diff before applying the patch. Apply the patch and repeat. (You can code snippets, but truncate the binary data to just a couple lines. We don't need an entire binary file in ASCII in an issue comment.) :)

  Before:

  Patch snippet:

  diff --git a/core/misc/tree.png b/core/misc/tree.png
  index 89ea235..2cedc27 100644
  Binary files a/core/misc/tree.png and b/core/misc/tree.png differ
  

  After:

  Patch created, cannot paste/attach.

• Create a patch that adds whitespace errors to a file. Reset, apply the patch from #20, and then try to apply your whitespace-error-adding patch. Document the result.

  Before:

  Terminal output:

  patch\work.patch:10: trailing whitespace.
     *
  warning: 1 line adds whitespace errors.
  

  Patch applied. Whitespace error introduced.

  After:

  Same result.

• Exposed with git diff --check.

  Before:

  Problematic line outlined on terminal.

  Terminal output:

  warning: LF will be replaced by CRLF in core/lib/Drupal/Core/Entity/EntityFormController.php.
  The file will have its original line endings in your working directory.
  

  After:

  Same result but without the warning.

• Applied with --whitespace=error-all option

  Before:

  Terminal output:

  patch\work.patch:10: trailing whitespace.
     *
  fatal: 1 line adds whitespace errors.
  

  Patch not applied.

  After:

  Same result.

• Applied with --whitespace=error-all option

  Before:

  Terminal output:

  patch\work.patch:10: trailing whitespace.
     *
  fatal: 1 line adds whitespace errors.
  

  Patch not applied.

  After:

  Same result.

• Fixed with git apply --whitespace=fix

  Before:

  Terminal output:

  patch\work.patch:10: trailing whitespace.
     *
  warning: 1 line applied after fixing whitespace errors.
  

  After:

  Same result.

• With #20 applied, try to create a new patch with new whitespace errors. Document the result.

  Patch created with whitespaces in it, no warning.

• (Advanced) Add a global .gitattributes file to your .gitconfig, and add some rules to it that are different from the ones in the patch. Document the results of using this with the patch.

  The local file takes precedence, global attributes are overridden.

• Add proper newlines to files before and after the patch and document the result. Ensure the patch is still generated correctly.

  Before:

  Checked out index.php (CRLF). Changed index.php. Patch created.
  Converted modified index.php to LF. Patch created. Terminal output:

  warning: LF will be replaced by CRLF in index.php.
  The file will have its original line endings in your working directory.
  

  LF patch applied with no warning.

  After:

  Checked out index.php (LF). Changed index.php. Patch created. Terminal output:

  warning: CRLF will be replaced by LF in index.php.
  The file will have its original line endings in your working directory.
  

  Converted modified index.php to CFLF. Patch created. Terminal output:

  warning: CRLF will be replaced by LF in index.php.
  The file will have its original line endings in your working directory.
  

  LF patch applied with no warning.
  Converted patch to CRLF. Tried to apply patch. Terminal output:

  patch\work.patch:9: trailing whitespace.
  

Awesome testing, thanks @plach!

I'll have to finish the rest of the testing requests another night, here's what I got through at tonight's core office hours session:

Add a whitespace error to a file and do git diff --color before applying the patch. Apply the patch and repeat.

Before the patch:
@see sh1-1.png attached.

After the patch:
@see sh2-2.png attached.

Make a small change to a method in a class file and do a git diff before applying the patch. Apply the patch and repeat. You can post before-and-after code snippets to illustrate the difference for reviewers.

Before the patch

	@@ -266,6 +266,12 @@ class EntityFormController implements EntityFormControllerInterface {
    * Implements Drupal\Core\Entity\EntityFormControllerInterface::getOperation().
    */
   public function getOperation() {
-    return $this->operation;
+    if($scalpel !== 'dropped') {
+      return $this->operation;
+    }
+    else {
+      $message = 'You are horrible at being a doctor.';
+      return $message;
+    }
   }
 }

After the patch

@@ -266,6 +266,12 @@ protected function prepareEntity(EntityInterface $entity) {
    * Implements Drupal\Core\Entity\EntityFormControllerInterface::getOperation().
    */
   public function getOperation() {
-    return $this->operation;
+    if($scalpel !== 'dropped') {
+      return $this->operation;
+    }
+    else {
+      $message = 'You are horrible at being a doctor.';
+      return $message;
+    }
   }
 }

Change a binary file and do a git diff before applying the patch. Apply the patch and repeat. (You can code snippets, but truncate the binary data to just a couple lines. We don't need an entire binary file in ASCII in an issue comment.) :)

Before the patch

	diff --git a/core/themes/bartik/screenshot.png b/core/themes/bartik/screenshot.png
	index 34734ef..2c7d8e1 100644
	Binary files a/core/themes/bartik/screenshot.png and b/core/themes/bartik/screenshot.png differ

After the patch

	diff --git a/core/themes/bartik/screenshot.png b/core/themes/bartik/screenshot.png
index 34734ef..00ca2ab 100644
--- a/core/themes/bartik/screenshot.png
+++ b/core/themes/bartik/screenshot.png
@@ -1,4 +1,4 @@
-<89>PNG
+<89>^M
 ^Z
 ^@^@^@^MIHDR^@^@^A&^@^@^@<DB>^H^C^@^@^@<F7>a̛^@^@^C^@PLTE<EC><EC><EB><B4><C6><D2>4<9B><D9>'''<F5><F5>
 <80>0^PC<D1><DC><FF><C6>I<A6>*<8A><82>T(^E^Q7y<D0>͇<E9>"<A8>    ^AO<88>^Of<CA>L<99><89><87>=<C9><D6><FE>̓
@@ -70,4 +70,4 @@ K
 Y!<BB>8<CD><EC><9E><C7><E0>^Z<95><EE><B3><CB>
 <A4>$<FF><B6><CC><D4>=^AZ^K<E4>O_<9F>E<F3><87>^Y<AA><BD><CC>t<8F><8F><FA>L^Z<EC><F5>a<91><E1><8F>+$3
 Ҝ<B9><80>"<DC>i<E4><<C2><CE>^@<B6>^Q<C9><E8>Z&<9F>I6<82>}5bR<FA><8F>><C8>jz<AC><BD>秹<DE><D6><EC>^O<91>
-^_<DB>N^_<FF>B<8E>w^@<8C><E7>o<A3><82><<BE><8B><FD><B4>Ct<84>/<84><EC>R<BC><B0>^]<AF>^Cš<F0><FE>Q<F0>H:
\ No newline at end of file
+^_<DB>N^_<FF>B<8E>w^@<8C><E7>o<A3><82><<BE><8B><FD><B4>Ct<84>/<84><EC>R<BC><B0>^]<AF>^Cš<F0><FE>Q<F0>H:

Just to confirm because xjm asked me to, I ran the binary file test again and here's the output.

Before applying patch in #20, clean branch of 8.x

diff --git a/core/themes/bartik/screenshot.png b/core/themes/bartik/screenshot.png
index 34734ef..2c7d8e1 100644
Binary files a/core/themes/bartik/screenshot.png and b/core/themes/bartik/screenshot.png differ 

After applying patch in #20

diff --git a/core/themes/bartik/screenshot.png b/core/themes/bartik/screenshot.png<br />
index 34734ef..2c7d8e1 100644
--- a/core/themes/bartik/screenshot.png
+++ b/core/themes/bartik/screenshot.png
@@ -1,4 +1,4 @@
-<89>PNG
+<89>JPG
 ^Z
 ^@^@^@^MIHDR^@^@^A&^@^@^@<DB>^H^C^@^@^@<F7>a̛^@^@^C^@PLTE<EC><EC><EB><B4><C6><D2>4<9B><D9>'''<F5><F5><F3><E4><E4>䴴<B4>C<A6>⼼<BC><C2><C2><C2><CC><CC>̬<AC>
 <80>0^PC<D1><DC><FF><C6>I<A6>*<8A><82>T(^E^Q7y<D0>͇<E9>"<A8>    ^AO<88>^Of<CA>L<99><89><87>=<C9><D6><FE>̓/<83><C2>Q<F9><FB><A3><F7>g<E0><83><AA><BA>^R`G
@@ -70,4 +70,4 @@ K
 Y!<BB>8<CD><EC><9E><C7><E0>^Z<95><EE><B3><CB>
 <A4>$<FF><B6><CC><D4>=^AZ^K<E4>O_<9F>E<F3><87>^Y<AA><BD><CC>t<8F><8F><FA>L^Z<EC><F5>a<91><E1><8F>+$3<F0>^P<91>^M<F4> 7R^P^P<91><C4>^E<A3>M<8D><FF>0<93>T
 Ҝ<B9><80>"<DC>i<E4><<C2><CE>^@<B6>^Q<C9><E8>Z&<9F>I6<82>}5bR<FA><8F>><C8>jz<AC><BD>秹<DE><D6><EC>^O<91>.on^\<D4><C3><^U+Gb<U+E955>]%<D1>^L<C6>(5<E5><8E>%
-^_<DB>N^_<FF>B<8E>w^@<8C><E7>o<A3><82><<BE><8B><FD><B4>Ct<84>/<84><EC>R<BC><B0>^]<AF>^Cš<F0><FE>Q<F0>H:i^]<86><86>Ŷ<AB>t<FB>e^W<AB><EB><9D>Q*<CC>c<F1>
\ No newline at end of file
+^_<DB>N^_<FF>B<8E>w^@<8C><E7>o<A3><82><<BE><8B><FD><B4>Ct<84>/<84><EC>R<BC><B0>^]<AF>^Cš<F0><FE>Q<F0>H:i^]<86><86>Ŷ<AB>t<FB>e^W<AB><EB><9D>Q*<CC>c<F1>

Steps to produce

1. checkout drupal 8
2. branch to 6497658-core-mentoring-issue
3. vim core/themes/bartik/screenshot.png
4. change PNG to JPG on line 1
5. git diff 8.x -- see above output
6. git reset --hard
7. wget patch
8. git apply -v patch
9. vim core/themes/bartik/screenshot.png
10. change PNG to JPG on line 1
11. git diff 8.x -- see above output

Thanks @carwin for confirming the result with the binary.

I'd be interested to see what this does with a patch that updates the db dumps, e.g. #1778986: drupal-7.filled.standard_all test db dump has invalid widget info.

@xjm: How many more confirmations do you want for the simple answer of:

You get a binary diff by default, because that's what we're telling git to do.

? ;)

Smart aleck. It actually seems like a UX regression to me from "Binary file changed" with no garbage, which is why I asked carwin to double-check, because it seemed like the opposite of what we should be doing.

For clarity, my comment in #30 is that I was curious how a binary diff of a tarball would differ. If it bears any relationship to the size of the change, fine, but with the file compressed I wouldn't expect it to. So I guess I don't really see the benefit.

Binary diffs of tarballs and zip files are currently only possible by supplying the additional --binary flag to git diff. That's the only way you can currently submit a patch to such issues in the first place.

Making them less of an edge-case is one of the primary purposes of this patch:

Stop the edge-cases. Don't make me ask. Just diff.

Okay, I can buy that. I misunderstood the intent of that configuration, then.

My remaining concerns are then from @plach's testing:

Exposed with git diff --check.

Before:

Problematic line outlined on terminal.

Terminal output:

warning: LF will be replaced by CRLF in core/lib/Drupal/Core/Entity/EntityFormController.php.
The file will have its original line endings in your working directory.
After:

Same result but without the warning.

Seems backwards to me that the warning would go away, but I guess that's again the expected behavior and I'm just misunderstanding.

And:

(Advanced) Add a global .gitattributes file to your .gitconfig, and add some rules to it that are different from the ones in the patch. Document the results of using this with the patch.

The local file takes precedence, global attributes are overridden.

So here, the configuration Drupal enforces overrides whatever is configured globally. I'd call that by design, though when I spoke to @webchick about this issue in IRC, she brought up the case of Silly Company X that mandates some particular configuration, say, ten-space indentation. I think it's such an edge case to be a nonissue. Consider the scenario:

1. Our local file overrides their global one.
2. So they hack the local one, er, locally.
3. Core gets updated. Like .htaccess, the .gitattributes gets overwritten.

BUT! They could work around this by using git, and presumably since they're using git, they're using git. git it? ;)

And also? They'd have to be committing their wild-and-crazy ten-space tabs in core, in which case they have a lot more to worry about when updating core than the .gitattributes itself. ;) Any custom module could have its own .gitattributes for the rebel tabs, thus circumventing the problem.

So I'm going to take the plunge and call this RTBC, and not just because @sun is a smartypants.

P.S.: This is my favorite thing:

@@ -266,6 +266,12 @@ protected function prepareEntity(EntityInterface $entity) {

Method names in class diffs, yay!

Ok, awesome. Thanks for the thorough testing on this, folks.

Committed and pushed to 8.x. Hopefully this will help increase our contributor velocity going forward.

Let's get a change notice for this, please. (Quickly, if at all possible, since we're bumping up against thresholds.)

A Symfony file has incorrect line endings, and this is causing it to throw errors, and apparently is wreaking havoc on the testbots.

warning: CRLF will be replaced by LF in core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/ProjectWithXsdExtensionInPhar.phar.
The file will have its original line endings in your working directory.

shouldn't this be backported to D7?

Interesting enough, it only affects D7 (and D6 I suppose), because for D8 tests it doesn't have to change branches.
http://qa.drupal.org/pifr/test/27332

[18:53:08] Command [git clone 'git://git.drupal.org/project/drupal.git' 'checkout' --reference /var/cache/git/reference 2>&1] succeeded.
[18:53:09] Command [git --work-tree='checkout' --git-dir='checkout/.git' checkout '7.x' 2>&1] failed
  Duration 1 seconds
  Directory [/var/lib/drupaltestbot/sites/default/files]
  Status [1]
 Output: [error: You have local changes to 'core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/ProjectWithXsdExtensionInPhar.phar'; cannot switch branches.].

So we can either revert this whole patch, or just strip out the line endings in that file and move forward (since future commits will be automatically fixed).

JavaScript is nothing like java, the diff algorithm is now giving useless method information in the diff. Please remove the diff=java from the javascript line.

#16 How is having java better than nothing? "nothing" gives better informations and a less fragmented patch that is easier to review: here are two version of the same patch as example: #1799594: Update to jQuery 1.9.

We've temporarily switched the default branch as an attempt to fix this until a new commit can be made.
#1799620: Temporarily switch the default git branch for Drupal core to 7.x

Retesting, will post back with results.

EDIT: This has temporarily fixed the problem with the D7 testbots.

Well that'll learn me to commit patches before getting on planes. :P~

Reverted.

Now, can someone explain how in the hell I broke *D7* by committing this patch?? And also why testbot didn't freak out before now?

D8 current has a file with CRLF line endings in it. This .gitattributes file prevents CRLF from being committed. Therefore, it flagged that file as having changes. This prevents you from checking out another branch, which is why it didn't break D8, but it broke D7.

All new files from now on will not be able to be added with CRLF, so we should never have this problem again.

This patch is the same as above, but replaces the CRLF with LF.

phar are archives, so I guess yes.

After adding phar as binary, the hunk about changing the phar file should not be needed anymore.

Removed the diff=java and comment for the js line.

Both of those changes are good, and adding *.phar is certainly better than my fix.

Sorry webchick, for all of the confusion!

I still contend that the new .gitattributes file should be backported to D7...

Agree with #51.

Hm. Last time I committed a dot-git-anything to D7, I got angry blog posts, emails, tweets, IRC lashes, and issue queue rage. Definitely not looking forward to that again. :P~ Fortunately, it's not my call anymore, and you can try asking David if he wants to take the risk 18 months into a stable release. ;)

For 8.x though, this seems fine, as long as it doesn't destroy the universe again.

Committed and pushed to 8.x again. Fingers crossed...

Posted http://drupal.org/node/1803766, please review.

> It is recursive for all sub-directories that do not contain their own .gitattributes file.
So if a sub-directory contains a .gitattributes file, this one is completely ignored? Or can the one below extend/change what has been set?

Can this be clarified? looks good otherwise.

Good point @Berdir. I clarified it.

http://drupal.org/node/1803766/revisions/view/2385820/2386704

Change notice looks good, thanks @tim.plunkett!

Lets see what David thinks of backporting ...

If Drupal core is installed in a subdirectory of a repo, the .gitattributes causes git status to complain, and even report phar files as modified. For example if my hosting provider requires the DocumentRoot to be a subdirectory of my Git repository, and I want to maintain Drupal using subtree merges here's what happens when I try to set that up:

Example A:

# setup
$ git clone example.com.git
$ cd example.com/

# add Drupal as a remote
$ git remote add -f drupal http://git.drupal.org/project/drupal.git
$ git checkout 8.x

# merge the 8.x branch into the path "docroot" on master; immediately get "not allowed" errors
$ git checkout master
$ git read-tree --prefix=docroot/ -u 8.x
[attr]drupaltext    text eol=lf whitespace=blank-at-eol,-blank-at-eof,-space-before-tab,tab-in-indent,tabwidth=2 not allowed: docroot/.gitattributes:13
[attr]drupalbinary  -text diff not allowed: docroot/.gitattributes:18

# git status repeats the "not allowed" and also shows the phar file as modified
$ git status
[attr]drupaltext    text eol=lf whitespace=blank-at-eol,-blank-at-eof,-space-before-tab,tab-in-indent,tabwidth=2
 not allowed: docroot/.gitattributes:13
[attr]drupalbinary  -text diff
 not allowed: docroot/.gitattributes:18
# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#	new file:   docroot/.editorconfig
#	new file:   docroot/.gitattributes
#	new file:   docroot/.htaccess
### SNIP ###
#
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#	modified:   docroot/core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/ProjectWithXsdExtensionInPhar.phar
#

So Git thinks I've modified ProjectWithXsdExtensionInPhar.phar? Huh???

It's not just an issue with git read-tree. Using rsync or similarly copying Drupal into the docroot folder of a Git repo causes a ton of worrisome messages about changing the contents of binary files as soon as .gitattributes is added:

Example B:

$ rsync -a --exclude=.git drupal/ example2.com/docroot
$ cd example2.com
$ git add docroot
[attr]drupaltext    text eol=lf whitespace=blank-at-eol,-blank-at-eof,-space-before-tab,tab-in-indent,tabwidth=2
 not allowed: docroot/.gitattributes:13
[attr]drupalbinary  -text diff
 not allowed: docroot/.gitattributes:18
warning: CRLF will be replaced by LF in docroot/core/assets/vendor/ckeditor/plugins/about/dialogs/hidpi/logo_ckeditor.png.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in docroot/core/assets/vendor/ckeditor/plugins/about/dialogs/logo_ckeditor.png.
The file will have its original line endings in your working directory.

Yikes!

I think it might be wise to get this into the change record, so that it doesn't trip people up. For me, moving .gitattributes into the top-level folder of the repo caused the complaining to stop; but I'm not certain that is the best practice.

@Jonathon Now that #2166013: Remove macros from .gitattributes to avoid Git errors. has removed the macros, do you still get those errors? If so, open a follow-up issue to discuss.

Doesn't this need to go to 7.x for David to decide if it should be backported?

Follow up issue for D8: #2333243: Relax .gitattributes global rule

I can't seem to get this file to not copy from the scaffolds

"drupal-scaffold": {
            "locations": {
                "web-root": "docroot/"
            },
            "file-mapping": {
                "[web-root]/.gitattributes": false
            }
        },

Does anyone know if this for some reason does not work?
https://www.drupal.org/docs/develop/using-composer/using-drupals-compose...

This is what I see from my CLI, its like it can't ignore the .gitattributes
Scaffolding files for drupal/core:
- Copy [project-root]/.gitattributes from assets/scaffold/files/gitattributes
- Skip [web-root]/.htaccess: overridden in drupal/recommended-project
* Homepage: https://www.drupal.org/project/drupal
* Support:

@NicholasS This issue is 10 years old. Please create a new issue with your request / problem.

Ad D7 backport: D7 planned EOL is on 5.1.2025. I do not think it would be good to introduce this in the current D7 phase, as it would have a minimal effect (and potentially can affect existing developers with different settings). Therefore closing this as fixed for D8 (and fixing credits). Thanks all!