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

Fails to stage hunk on Windows when file uses LF (unix) line endings #176

Open
brownts opened this issue Oct 14, 2020 · 3 comments · May be fixed by #210
Open

Fails to stage hunk on Windows when file uses LF (unix) line endings #176

brownts opened this issue Oct 14, 2020 · 3 comments · May be fixed by #210
Labels
bug a feature isn't working

Comments

@brownts
Copy link
Contributor

brownts commented Oct 14, 2020

In a repository which uses LF line termination, using git-gutter on Windows appears to be problematic for staging hunks (and possibly other operations).

Scenario:

  1. Source file (LF line termination -- unix style) on Windows 10
  2. M-x git-gutter:stage-hunk
  3. *git-gutter:diff* buffer shows CRLF end-of-line formatting
  4. "Stage current hunk? (y or n)" y
  5. Application of patch fails (i.e., "Failed: stating this hunk")

I investigated this a bit and discovered that the temporary "patch" file created in git-gutter:do-stage-hunk ends up using CRLF line endings when written to the file system. Note that this patch file is different than if you just performed the git diff command directly at the command line as the patch file contains CRLF and the git diff command contains only LF endings. Thus, it appears the internal manipulation of the diff output within the Emacs buffers by git-gutter is likely what is causing the line-endings to be changed to CRLF.

This trips up the subsequent git apply as that is the command which fails (with "patch failed" and "patch does not apply" output from git). I modified the source to prevent the temporary file from being deleted, so that I could experiment with attempting to apply the patch directly with git from the console. I then performed a conversion of the line endings of the patch file (i.e., dos2unix -U <temp file>) and repeated the git apply command. The subsequent git apply then succeeds. So it appears that Windows git is sensitive to the line endings of the patch file created by git-gutter. I looked at the --whitespace=fix and --ignore-whitespace options for git apply, but neither of those make sense in this scenario.

I also attempted to replicate the reverse behavior on Linux. Starting with a CRLF EOL file and performed a git-gutter:stage-hunk on that. The *git-gutter:diff* buffer showed the line ending as being LF, but the subsequent git apply succeeded. Thus, this appears to be problematic only on Windows.

Steps to reproduce (on Windows):

  1. Create new git repo (git init)
  2. Create file and save it with LF line endings.
  3. Commit file as initial version
  4. Modify file in Emacs, save file, and attempt to use git-gutter:stage-hunk to stage the change.
  5. The application of the patch should fail with "Failed: stating this hunk".

These were the versions of tools I used:

  • git-gutter-20200326.1814
  • Windows git 2.28.0.windows.1
  • Emacs 27.1
  • Windows 10
@syohex
Copy link
Contributor

syohex commented Oct 14, 2020

Could you try #177 patch ?

@brownts
Copy link
Contributor Author

brownts commented Oct 14, 2020

@syohex, I tried this patch, and it partially worked, but it lead to a discovery of a more fundamental problem. It appears the format of the patch generated by git diff has line termination dependent on the internal (i.e., repository) storage format of the files. Thus, if the file is stored in the repository with LF line termination, the patch will have LF termination (regardless of the line termination of the workspace file). However, if the file is stored in the repository with CRLF line termination, the patch will have CRLF termination. Therefore the line termination of the patch is not dependent on the workspace file, but on the format of the repository file (at least when trying to match the original portion of the patch).

What I realized is that I had core.autocrlf=true set in my Windows git configuration for the repositories I had previously experimented with. That meant that internally the files would be stored with LF line termination (whether the workspace file had LF or CRLF line termination) and thus the patch generated by git diff and expected by git apply required LF line termination to match the representation of the internally stored file. When I configured a new repository on Windows with core.autocrlf=false and created a source file with CRLF line termination, the patch generated by git diff and expected by git apply has CRLF line termination (again to match the representation of the internally stored file).

In summary, what I observed with Windows git:

  • When core.autocrlf is used (true), git diff generates a patch which uses LF line termination, regardless of the workspace file format.
  • When core.autocrlf is not used (or false), git diff generates a patch where the hunk uses the line termination of the file.

Therefore, it doesn't appear to be enough to base the line termination of the patch file on the workspace file, as that may not be how it is internally represented. Additionally, it appears problematic to attempt to force the line termination a specific way, and it is probably best to preserve the line termination as originally generated by git diff, else that risks causing "patch does not apply" failures.

With the currently proposed #177 patch, I can make this fail on Linux in the following manner:

  • Create git repository (git init)
  • Configure repository for autocrlf (git config --local core.autocrlf true)
  • Create source file with CRLF line termination and commit file.
  • Modify source file and save it.
  • Attempt to stage hunk (M-x git-gutter:stage-hunk)
  • Staging fails. This fails because internally the file is saved with LF line termination, but the temporary patch file is created with CRLF line termination.

I think if the temporary patch file generation can be changed so that it doesn't perform line ending conversions from the output that git diff generates (i.e., preserves the line termination as generated by git diff), this would likely take care of the issue.

@jcs090218 jcs090218 added the bug a feature isn't working label Oct 28, 2020
@jcs090218
Copy link
Collaborator

jcs090218 commented Oct 28, 2020

Mark with bug label for now! Thanks for reporting this issue to us!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug a feature isn't working
Development

Successfully merging a pull request may close this issue.

3 participants