Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Line endings should be LF across repo and not CRLF #10119

Merged
merged 1 commit into from
Feb 10, 2021
Merged

Conversation

LysandreJik
Copy link
Member

Up to now we've had quite a few issues with users on Windows having their line endings be "CRLF" by default, while Linux users have "LF" line endings by default.

Problem

This can be problematic in the following scenarios where no handling of the issue has been done on the user's side:

  • When a user runs make style, their line endings will switch from LF to CRLF in all files, essentially rewriting the entire file
  • When a user adds a new file to the repository, it will be in the "CRLF" format and will be committed as such.

Resolution

The resolution is either to have the user handle that, or to handle that ourselves. Handling it ourselves is simple as it only requires adding a .gitattributes file at the root of the repository which will specify the line endings we're looking for, thus this is what this PR is proposing. On the other hand, we had issues handling it on the user side with the proposed git core.autocrlf as it seemed to have different results according to the setup.

Additionally, if users already have files in CRLF mode, then an additional command is required to convert these files to LF: git add --renormalize .. I believe this only impacts users that created files previous to this PR, as newly created files will already benefit from the .gitattributes file.


This PR completely reformats two files: examples/research_projects/bertology/run_prune_gpt.py and tests/test_modeling_deberta.py. These files had CRLF line endings, and will now have LF line endings.


Further readings:

cc @NielsRogge

@LysandreJik LysandreJik mentioned this pull request Feb 10, 2021
10 tasks
Copy link
Contributor

@jplu jplu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Super cool that we can now handle EOL that easily!

@NielsRogge
Copy link
Contributor

Thank you @LysandreJik, this will alleviate much of the pain Windows users had with git in the past!

@julien-c
Copy link
Member

To hack my way up the contributors list, can I change all line endings to CRLF then revert? 😂

Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for diving into this!

@LysandreJik LysandreJik merged commit 0d8e554 into master Feb 10, 2021
@LysandreJik LysandreJik deleted the gitattributes branch February 10, 2021 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants