Opened 11 years ago

Last modified 9 years ago

#22 new enhancement

diff, patch and EOL style

Reported by: dmik Owned by:
Priority: major Milestone: dev tools
Component: *none Version:
Severity: low Keywords:
Cc:

Description

There is a fundamental problem with diff, patch and line endings. It happens when line endings in either party do not match the system-default (CRLF in case of OS/2). With the default diff/patch behavior, the following happens:

  • everything (LF, CRLF) is accepted on input (files, patches)
  • patches apply cleanly even if there is a mismatch in EOL style
  • output is always CRLF

The worst case in this scenario is when your file's endings are LF. After doing a diff/patch cycle on such a file all line endings will be unconditionally converted to CRLF. This is annoying in many cases.

There is one solution. Both tools support the --binary option that turns off automatic CRLF<->LF conversion. As a result, you will get the following behavior:

  • all input files must have the same line ending style (either LF or CRLF)
  • patches will not apply if EOL style of the target differs from the source
  • output has the same EOL style as input

This solves the above problem with the LF file but it breaks patching if e.g. the patch file's EOL style gets accidentally changed while transferring (in an email message or in a ticket's comment).

A better solution might be to combine the above two behaviors:

  • in diff, accept any EOL style when comparing files and write a patch in system default EOL style
  • in patch, accept any EOL style of both the patch and the target file and preserve EOL style of the target on output after patching it

Change History (3)

comment:1 by dmik, 11 years ago

Type: enhancementdefect

The diff and patch code is rather complex and implementing the solution described last requires some effort beyond my current capacity. As a temporary solution, I implemented the second approach which basically means that the --binary option is forced on OS/2 (i.e. diff/patch behave as if it were set and you cannot unset it). In addition to this, diff ignores the --strip-trailing-cr option and always behaves as it were unset. This is necessary to make sure that the patch file will derive line ending style from the diffed files (and will apply cleanly by patch).

Actually, --binary was already forced for patch by Yuri. I just fixed one unpleasant bug there (r726) and added the similar thing to diff so that they behave consistently now. This means that what diff produces is guaranteed to be cleanly applied by patch provided that line ending style of the files and the patches is not changed in between. The only disadvantage of this new behavior is that if you happen to get a patch whose original EOL style changes, your patch invocation will fail and you will have to manually change the EOL style of the patch to match the target.

When the better solution is to be implemented, the current one forcing --binary must be undone by reverting the following revisions: r730, r727, r552 (only src/patch.c).

comment:2 by dmik, 10 years ago

Type: defectenhancement

comment:3 by Silvan Scherrer, 9 years ago

Component: *none
Milestone: dev tools
Severity: low
Note: See TracTickets for help on using tickets.