diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..cac481a4 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,2 @@ +# The CRLF compat fixtures must keep their line endings byte for byte. +tests/compat/*/crlf_*/** -text diff --git a/src/binary/mod.rs b/src/binary/mod.rs index 26f96548..1b5cea35 100644 --- a/src/binary/mod.rs +++ b/src/binary/mod.rs @@ -15,6 +15,9 @@ use alloc::vec::Vec; use core::fmt; use core::ops::Range; +use crate::utils::LineIter; +use crate::utils::Text; + /// Cap preallocation when the size comes from untrusted input. /// /// This prevents instant OOM from a bogus header. @@ -394,16 +397,9 @@ impl<'a> BinaryParser<'a> { } fn next_line(&mut self) -> Option<&'a [u8]> { - let rest = &self.input[self.offset..]; - if rest.is_empty() { - return None; - } - let (line, skip) = match rest.iter().position(|&b| b == b'\n') { - Some(pos) => (&rest[..pos], pos + 1), - None => (rest, rest.len()), - }; - self.offset += skip; - Some(line.strip_suffix(b"\r").unwrap_or(line)) + let line = LineIter::new(&self.input[self.offset..]).next()?; + self.offset += line.len(); + Some(line.strip_line_ending().unwrap_or(line)) } } @@ -474,10 +470,7 @@ fn parse_binary_block<'a>(parser: &mut BinaryParser<'a>) -> Option Result, BinaryPatchParseError> { // A rough estimate: In Base85, 5 chars -> 4 bytes let mut result = Vec::with_capacity(data.len() * 4 / 5); - for line in data.split(|&b| b == b'\n') { - let line = line.strip_suffix(b"\r".as_slice()).unwrap_or(line); + for line in LineIter::new(data) { + let line = line.strip_line_ending().unwrap_or(line); if line.is_empty() { continue; } diff --git a/src/patch/format.rs b/src/patch/format.rs index 8f172d32..b6d79830 100644 --- a/src/patch/format.rs +++ b/src/patch/format.rs @@ -237,14 +237,20 @@ impl + ?Sized> HunkDisplay<'_, T> { if self.f.with_color { write!(w, "{}", style::FUNCTION_CONTEXT)?; } - write!(w, " ")?; w.write_all(ctx.as_ref())?; #[cfg(feature = "color")] if self.f.with_color { write!(w, "{:#}", style::FUNCTION_CONTEXT)?; } } - writeln!(w)?; + // The function context keeps the line ending it was parsed with. + let has_line_ending = self + .hunk + .function_context + .is_some_and(|ctx| ctx.as_ref().ends_with(b"\n")); + if !has_line_ending { + writeln!(w)?; + } for line in &self.hunk.lines { self.f.write_line_into(line, &mut w)?; @@ -272,13 +278,20 @@ impl Display for HunkDisplay<'_, str> { if self.f.with_color { write!(f, "{}", style::FUNCTION_CONTEXT)?; } - write!(f, " {ctx}")?; + write!(f, "{ctx}")?; #[cfg(feature = "color")] if self.f.with_color { write!(f, "{:#}", style::FUNCTION_CONTEXT)?; } } - writeln!(f)?; + // The function context keeps the line ending it was parsed with. + let has_line_ending = self + .hunk + .function_context + .is_some_and(|ctx| ctx.ends_with('\n')); + if !has_line_ending { + writeln!(f)?; + } for line in &self.hunk.lines { write!(f, "{}", self.f.fmt_line(line))?; diff --git a/src/patch/mod.rs b/src/patch/mod.rs index a12c4c56..a7245922 100644 --- a/src/patch/mod.rs +++ b/src/patch/mod.rs @@ -302,7 +302,13 @@ impl<'a, T: ?Sized> Hunk<'a, T> { self.new_range } - /// Returns the function context (if any) for the hunk + /// Returns the function context (if any) for the hunk. + /// + /// This is the text after the closing `@@` of the hunk header, such as + /// the enclosing function's signature added by `diff -p` or `git diff`. + /// It excludes the space before it but keeps the hunk header's line + /// ending (`\n` or `\r\n`), if there is one, so formatting the hunk + /// writes the header back unchanged. pub fn function_context(&self) -> Option<&T> { self.function_context } @@ -390,6 +396,12 @@ impl fmt::Display for HunkRange { /// /// A `Line` contains the terminating newline character `\n` unless it is the final /// line in the file and the file does not end with a newline character. +/// +/// Line endings are kept exactly as they appear in the patch: a line parsed +/// from a patch with CRLF line endings ends in `\r\n`, so applying the patch +/// only matches lines that also end in `\r\n`. This matches `git apply`, +/// which treats the `\r` as part of the line's content. (GNU patch instead +/// strips the `\r` from every line of a patch with CRLF line endings.) #[derive(Debug, PartialEq, Eq)] pub enum Line<'a, T: ?Sized> { /// A line providing context in the diff which is present in both the old and new file diff --git a/src/patch/parse.rs b/src/patch/parse.rs index 624d7564..d8d6f55a 100644 --- a/src/patch/parse.rs +++ b/src/patch/parse.rs @@ -178,16 +178,11 @@ fn parse_filename<'a, T: Text + ?Sized>(prefix: &str, line: &'a T) -> Result filename, + None => line + .strip_line_ending() + .ok_or(ParsePatchErrorKind::FilenameUnterminated)?, }; let filename = escaped_filename(filename)?; diff --git a/src/patch/tests.rs b/src/patch/tests.rs index d56d0736..c43d06c4 100644 --- a/src/patch/tests.rs +++ b/src/patch/tests.rs @@ -804,3 +804,110 @@ mod error_display { assert_eq!(err.kind, ParsePatchErrorKind::InvalidHunkHeader); } } + +/// A patch written by `diff -u -p` (GNU diffutils 3.12). `git diff` writes the +/// same hunk header: one space after the closing `@@`, then the enclosing +/// function. +const FUNCTION_CONTEXT_PATCH: &str = "\ +--- a/short.c ++++ b/short.c +@@ -4,6 +4,6 @@ int main(void) + line3; + line4; + line5; +- line6; ++ CHANGED; + line7; + line8; +"; + +// The function context excludes the space before it but keeps the line +// ending, so formatting writes the hunk header back unchanged. +#[test] +fn function_context_roundtrip() { + let s = FUNCTION_CONTEXT_PATCH; + let p = parse(s).unwrap(); + assert_eq!(p.hunks()[0].function_context(), Some("int main(void)\n")); + assert_eq!(p.to_string(), s); + + let b = parse_bytes(s.as_ref()).unwrap(); + assert_eq!( + b.hunks()[0].function_context(), + Some(&b"int main(void)\n"[..]) + ); + assert_eq!(b.to_bytes(), s.as_bytes()); +} + +// A CRLF hunk header keeps its `\r\n` through a round trip. The formatter +// writes its own `---`/`+++` lines, which end in `\n`. +#[test] +fn function_context_crlf() { + let s = "\ +--- a/f\r ++++ b/f\r +@@ -1 +1 @@ fn foo()\r +-old\r ++new\r +"; + let p = parse(s).unwrap(); + assert_eq!(p.hunks()[0].function_context(), Some("fn foo()\r\n")); + assert_eq!( + p.to_string(), + "--- a/f\n+++ b/f\n@@ -1 +1 @@ fn foo()\r\n-old\r\n+new\r\n" + ); +} + +// With nothing after the closing `@@` there is no function context. With only +// a space after it, the function context is just the line ending, and +// formatting keeps the space. +#[test] +fn function_context_absent_or_empty() { + let absent = "--- a/f\n+++ b/f\n@@ -1 +1 @@\n-old\n+new\n"; + let p = parse(absent).unwrap(); + assert_eq!(p.hunks()[0].function_context(), None); + assert_eq!(p.to_string(), absent); + + let empty = "--- a/f\n+++ b/f\n@@ -1 +1 @@ \n-old\n+new\n"; + let p = parse(empty).unwrap(); + assert_eq!(p.hunks()[0].function_context(), Some("\n")); + assert_eq!(p.to_string(), empty); +} + +// A hunk header at the end of the input has no line ending, so formatting +// adds one. +#[test] +fn function_context_without_line_ending() { + let s = "--- a/f\n+++ b/f\n@@ -0,0 +0,0 @@ fn foo()"; + let p = parse(s).unwrap(); + assert_eq!(p.hunks()[0].function_context(), Some("fn foo()")); + assert_eq!(p.to_string(), format!("{s}\n")); +} + +// Color only wraps parts of the output in escape sequences. +#[cfg(feature = "color")] +#[test] +fn function_context_with_color() { + use super::PatchFormatter; + + fn strip_escapes(s: &str) -> alloc::string::String { + let mut out = alloc::string::String::new(); + let mut rest = s; + while let Some(start) = rest.find("\x1b[") { + out.push_str(&rest[..start]); + let len = rest[start..].find('m').expect("unterminated escape"); + rest = &rest[start + len + 1..]; + } + out.push_str(rest); + out + } + + let s = FUNCTION_CONTEXT_PATCH; + let f = PatchFormatter::new().with_color(); + let p = parse(s).unwrap(); + assert_eq!(strip_escapes(&f.fmt_patch(&p).to_string()), s); + + let b = parse_bytes(s.as_ref()).unwrap(); + let mut bytes = alloc::vec::Vec::new(); + f.write_patch_into(&b, &mut bytes).unwrap(); + assert_eq!(strip_escapes(core::str::from_utf8(&bytes).unwrap()), s); +} diff --git a/src/patch_set/parse.rs b/src/patch_set/parse.rs index 8facc78c..bf7c6f72 100644 --- a/src/patch_set/parse.rs +++ b/src/patch_set/parse.rs @@ -24,13 +24,20 @@ const MODIFIED_PREFIX: &str = "+++ "; /// Path used to indicate file creation or deletion. const DEV_NULL: &str = "/dev/null"; -/// Separator between commit message and patch in git format-patch output. -const EMAIL_PREAMBLE_SEPARATOR: &str = "\n---\n"; +/// Line separating the commit message from the patch in git format-patch +/// output. +const EMAIL_PREAMBLE_SEPARATOR: &str = "---"; /// Streaming iterator for parsing patches one at a time. /// /// Created by [`PatchSet::parse`] or [`PatchSet::parse_bytes`]. /// +/// # Line endings +/// +/// Lines of patch syntax, such as headers and separators, may end in either +/// `\n` or `\r\n`. Hunk lines keep their line endings as content; see +/// [`Line`](crate::Line). +/// /// # Example /// /// ``` @@ -194,7 +201,7 @@ fn find_patch_start(input: &T) -> Option { /// Strips email preamble (headers and commit message) from `git format-patch` output. /// -/// Returns the content after the first `\n---\n` separator. +/// Returns the content after the first `---` separator line. /// /// ## Observed git behavior /// @@ -202,6 +209,9 @@ fn find_patch_start(input: &T) -> Option { /// as the separator between commit message and patch content. /// It does not check if `diff --git` follows or there are more `---` lines. /// +/// `git mailsplit` (also used by `git am`) strips the `\r` from lines ending +/// in `\r\n` unless `--keep-cr` is given, so a `---\r\n` line separates too. +/// /// From [`git format-patch`] manpage: /// /// > The log message and the patch are separated by a line with a three-dash line. @@ -213,13 +223,18 @@ fn strip_email_preamble(input: &T) -> &T { return input; } - match input.find(EMAIL_PREAMBLE_SEPARATOR) { - Some(pos) => { - let (_, rest) = input.split_at(pos + EMAIL_PREAMBLE_SEPARATOR.len()); - rest + let mut offset = 0; + for line in input.lines() { + offset += line.len(); + let is_separator = line + .strip_line_ending() + .is_some_and(|line| line.as_bytes() == EMAIL_PREAMBLE_SEPARATOR.as_bytes()); + if is_separator { + let (_, rest) = input.split_at(offset); + return rest; } - None => input, } + input } fn next_gitdiff_patch<'a, T: Text + ?Sized>( @@ -423,7 +438,7 @@ impl<'a, T: Text + ?Sized> GitHeader<'a, T> { let mut consumed = 0; for line in input.lines() { - let trimmed = strip_line_ending(line); + let trimmed = line.strip_line_ending().unwrap_or(line); if let Some(rest) = trimmed.strip_prefix("diff --git ") { // Only accept the first `diff --git` line. @@ -820,13 +835,3 @@ fn extract_file_op_unidiff<'a, T: Text + ?Sized>( } } } - -/// Strips the trailing `\n` from a line yielded by [`Text::lines`]. -/// -/// [`Text::lines`] includes line endings; strip for matching. -fn strip_line_ending(line: &T) -> &T { - // TODO: GNU patch strips trailing CRs from CRLF patches automatically. - // We should consider adding compat tests for GNU patch. - // And `git apply` seems to reject. Worth adding tests as well. - line.strip_suffix("\n").unwrap_or(line) -} diff --git a/src/patch_set/tests.rs b/src/patch_set/tests.rs index 8d35809c..e1503269 100644 --- a/src/patch_set/tests.rs +++ b/src/patch_set/tests.rs @@ -6,8 +6,85 @@ use alloc::vec::Vec; use super::FileOperation; use super::ParseOptions; +use super::PatchKind; use super::PatchSet; use super::error::PatchSetParseErrorKind; +use crate::Patch; +use crate::binary::BinaryBlockKind; +use crate::binary::BinaryPatch; +use crate::patch::HunkRange; +use crate::patch::Line; + +/// Asserts that `lf` parses to the same patches after every `\n` in it is +/// replaced by `\r\n`. +/// +/// Hunk lines are compared without their line endings, because CRLF hunk +/// lines keep their `\r` as content (see `crlf::hunk_lines_keep_cr`). +fn assert_crlf_equivalent(lf: &str, opts: ParseOptions) { + let crlf = lf.replace('\n', "\r\n"); + let lf_patches = PatchSet::parse(lf, opts.clone()) + .collect::, _>>() + .unwrap(); + let crlf_patches = PatchSet::parse(&crlf, opts) + .collect::, _>>() + .unwrap_or_else(|e| panic!("the CRLF form of the input failed to parse: {e}")); + + assert_eq!(lf_patches.len(), crlf_patches.len(), "patch counts differ"); + for (lf, crlf) in lf_patches.iter().zip(&crlf_patches) { + assert_eq!(lf.operation(), crlf.operation()); + assert_eq!(lf.old_mode(), crlf.old_mode()); + assert_eq!(lf.new_mode(), crlf.new_mode()); + match (lf.patch(), crlf.patch()) { + (PatchKind::Text(lf), PatchKind::Text(crlf)) => { + assert_eq!(lf.original(), crlf.original()); + assert_eq!(lf.modified(), crlf.modified()); + assert_eq!( + hunks_without_line_endings(lf), + hunks_without_line_endings(crlf) + ); + } + // Binary payloads are slices of the input that keep their line + // endings, so compare what the headers declare. + (PatchKind::Binary(lf), PatchKind::Binary(crlf)) => { + assert_eq!(binary_blocks(lf), binary_blocks(crlf)); + } + (lf, crlf) => panic!("patch kinds differ: {lf:?} vs {crlf:?}"), + } + } +} + +type HunkWithoutLineEndings<'a> = (HunkRange, HunkRange, Option<&'a str>, Vec>); + +fn hunks_without_line_endings<'a>(patch: &'a Patch<'_, str>) -> Vec> { + let trim = |s: &'a str| s.trim_end_matches(['\r', '\n']); + patch + .hunks() + .iter() + .map(|hunk| { + let lines = hunk + .lines() + .iter() + .map(|line| match *line { + Line::Context(s) => Line::Context(trim(s)), + Line::Delete(s) => Line::Delete(trim(s)), + Line::Insert(s) => Line::Insert(trim(s)), + }) + .collect(); + let function_context = hunk.function_context().map(trim); + (hunk.old_range(), hunk.new_range(), function_context, lines) + }) + .collect() +} + +fn binary_blocks(patch: &BinaryPatch<'_>) -> Option<[(BinaryBlockKind, u64); 2]> { + match patch { + BinaryPatch::Full { forward, reverse } => Some([ + (forward.kind, forward.data.size), + (reverse.kind, reverse.data.size), + ]), + BinaryPatch::Marker => None, + } +} mod file_operation { use super::*; @@ -473,7 +550,10 @@ In a hole in the ground there lived a hobbit mod patchset_gitdiff { use super::*; + + /// Parses `input`, and also checks that its CRLF form parses the same. fn parse_gitdiff(input: &str) -> Vec> { + assert_crlf_equivalent(input, ParseOptions::gitdiff()); PatchSet::parse(input, ParseOptions::gitdiff()) .collect::, _>>() .unwrap() @@ -796,7 +876,6 @@ diff --git a/foo b/foo mod patchset_unidiff_bytes { use super::*; - use crate::patch::Line; #[test] fn single_file_bytes() { @@ -896,3 +975,131 @@ mod patchset_unidiff_bytes { ); } } + +/// Patches with CRLF line endings, as written by Windows tools or by a git +/// checkout with `core.autocrlf` enabled. +mod crlf { + use super::*; + use crate::patch_set::FileMode; + + /// A `git format-patch` email whose commit message has a line starting + /// with `diff --git`. Only the `---` line ends the commit message. + const FORMAT_PATCH: &str = "\ +From 1234567890abcdef1234567890abcdef12345678 Mon Sep 17 00:00:00 2001 +From: Gandalf +Date: Mon, 25 Mar 3019 00:00:00 +0000 +Subject: [PATCH] fix the diff header parser + +This message line is not a patch header: +diff --git a/phantom.rs b/phantom.rs +--- + src/frodo.rs | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/frodo.rs b/src/frodo.rs +index 1111111..2222222 100644 +--- a/src/frodo.rs ++++ b/src/frodo.rs +@@ -1 +1 @@ +-finger ++peace +-- +2.40.0 +"; + + #[test] + fn format_patch_gitdiff() { + let input = FORMAT_PATCH.replace('\n', "\r\n"); + let patches = PatchSet::parse(&input, ParseOptions::gitdiff()) + .collect::, _>>() + .unwrap(); + assert_eq!(patches.len(), 1); + assert_eq!( + patches[0].operation(), + &FileOperation::Modify { + original: "a/src/frodo.rs".into(), + modified: "b/src/frodo.rs".into(), + } + ); + assert_crlf_equivalent(FORMAT_PATCH, ParseOptions::gitdiff()); + } + + #[test] + fn format_patch_unidiff() { + assert_crlf_equivalent(FORMAT_PATCH, ParseOptions::unidiff()); + } + + /// Hunk lines keep their `\r` as content, matching `git apply`. The + /// `\ No newline at end of file` marker removes only the `\n`, so the `\r` + /// before it stays too: `git apply` matches such a line against a file + /// ending in `old\r`. + #[test] + fn hunk_lines_keep_cr() { + let input = "\ +--- a/f ++++ b/f +@@ -1,2 +1,2 @@ + keep +-old +\\ No newline at end of file ++new +\\ No newline at end of file +" + .replace('\n', "\r\n"); + let patches = PatchSet::parse(&input, ParseOptions::unidiff()) + .collect::, _>>() + .unwrap(); + let patch = patches[0].patch().as_text().unwrap(); + assert_eq!( + patch.hunks()[0].lines(), + [ + Line::Context("keep\r\n"), + Line::Delete("old\r"), + Line::Insert("new\r"), + ] + ); + } + + #[test] + fn rename_with_mode_change_bytes() { + let input = b"\ +diff --git a/old.sh b/new.sh\r +old mode 100644\r +new mode 100755\r +similarity index 100%\r +rename from old.sh\r +rename to new.sh\r +"; + let patches = PatchSet::parse_bytes(input, ParseOptions::gitdiff()) + .collect::, _>>() + .unwrap(); + assert_eq!(patches.len(), 1); + assert_eq!( + patches[0].operation(), + &FileOperation::Rename { + from: b"old.sh".as_slice().into(), + to: b"new.sh".as_slice().into(), + } + ); + assert_eq!(patches[0].old_mode(), Some(&FileMode::Regular)); + assert_eq!(patches[0].new_mode(), Some(&FileMode::Executable)); + } + + /// A `\r` that no `\n` follows is not a line ending, so it stays part of + /// the header value. + #[test] + fn lone_cr_is_not_a_line_ending() { + let input = "\ +diff --git a/x b/x +old mode 100644 +new mode 100755\r"; + let err = PatchSet::parse(input, ParseOptions::gitdiff()) + .next() + .unwrap() + .unwrap_err(); + assert_eq!( + err.kind, + PatchSetParseErrorKind::InvalidFileMode("100755\r".to_owned()) + ); + } +} diff --git a/src/utils.rs b/src/utils.rs index d94d56b7..be13d513 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -150,6 +150,24 @@ pub trait Text: Eq + Hash + ToOwned { fn as_bytes(&self) -> &[u8]; fn lines(&self) -> LineIter<'_, Self>; + /// Strips the line ending from a line yielded by [`LineIter`]. + /// + /// Both `\n` and `\r\n` are line endings, so patch syntax written with + /// Windows line endings parses the same as with Unix ones. A `\r` is only + /// part of the line ending when `\n` follows it. Stripping it never + /// truncates a header value: git quotes any path containing a `\r` (see + /// [`byte_needs_quoting`]), and no other header value can contain one. + /// + /// Use this for patch syntax only. Hunk lines keep their line endings as + /// content, matching `git apply`. + /// + /// Returns `None` if the line has no line ending, which happens only for + /// the final line of input that doesn't end with a newline. + fn strip_line_ending(&self) -> Option<&Self> { + let line = self.strip_suffix("\n")?; + Some(line.strip_suffix("\r").unwrap_or(line)) + } + /// Converts raw bytes into `Self::Owned`. /// /// Returns `None` if the bytes are not valid for this type diff --git a/tests/compat/common.rs b/tests/compat/common.rs index 014f9ddf..2ec066fa 100644 --- a/tests/compat/common.rs +++ b/tests/compat/common.rs @@ -39,6 +39,11 @@ pub struct Case<'a> { expect_diffy_error: Option, /// Inline snapshot for external tool's stderr on failure. expect_external_error: Option, + /// Whether output files must match byte for byte (default: false). + /// + /// snapbox treats `\r\n` and `\n` as equal when comparing text files, + /// which hides whether a line kept its `\r`. + exact_line_endings: bool, } impl<'a> Case<'a> { @@ -52,6 +57,7 @@ impl<'a> Case<'a> { expect_compat: true, expect_diffy_error: None, expect_external_error: None, + exact_line_endings: false, } } @@ -65,6 +71,7 @@ impl<'a> Case<'a> { expect_compat: true, expect_diffy_error: None, expect_external_error: None, + exact_line_endings: false, } } @@ -109,6 +116,15 @@ impl<'a> Case<'a> { self } + /// Compare output files byte for byte, including their line endings. + /// + /// Write the `out/` files by hand: `SNAPSHOTS=overwrite` normalizes their + /// line endings. + pub fn exact_line_endings(mut self) -> Self { + self.exact_line_endings = true; + self + } + /// Run the test case. pub fn run(self) { let case_dir = self.case_dir(); @@ -175,6 +191,9 @@ impl<'a> Case<'a> { // verify outputs match if diffy_result.is_ok() && external_result.is_ok() && self.expect_compat { snapbox::assert_subset_eq(&external_output, &diffy_output); + if self.exact_line_endings { + assert_subset_bytes_eq(&external_output, &diffy_output); + } } // Verify agreement/disagreement based on expectation @@ -197,7 +216,34 @@ impl<'a> Case<'a> { // Compare against expected snapshot (only for success cases) if self.expect_success { snapbox::assert_subset_eq(case_dir.join("out"), &diffy_output); + if self.exact_line_endings { + assert_subset_bytes_eq(&case_dir.join("out"), &diffy_output); + } + } + } +} + +/// Asserts that every file under `expected_root` exists under `actual_root` +/// with the same bytes. +fn assert_subset_bytes_eq(expected_root: &Path, actual_root: &Path) { + for entry in fs::read_dir(expected_root).unwrap() { + let expected_path = entry.unwrap().path(); + let actual_path = actual_root.join(expected_path.file_name().unwrap()); + if expected_path.is_dir() { + assert_subset_bytes_eq(&expected_path, &actual_path); + continue; } + let expected = fs::read(&expected_path) + .unwrap_or_else(|e| panic!("failed to read {}: {e}", expected_path.display())); + let actual = fs::read(&actual_path) + .unwrap_or_else(|e| panic!("failed to read {}: {e}", actual_path.display())); + assert_eq!( + expected.escape_ascii().to_string(), + actual.escape_ascii().to_string(), + "bytes differ: {} vs {}", + expected_path.display(), + actual_path.display(), + ); } } diff --git a/tests/compat/git/crlf_create_file/in/foo.patch b/tests/compat/git/crlf_create_file/in/foo.patch new file mode 100644 index 00000000..608d6c88 --- /dev/null +++ b/tests/compat/git/crlf_create_file/in/foo.patch @@ -0,0 +1,8 @@ +diff --git a/new.txt b/new.txt +new file mode 100644 +index 0000000..1111111 +--- /dev/null ++++ b/new.txt +@@ -0,0 +1,2 @@ ++hello ++world diff --git a/tests/compat/git/crlf_create_file/out/new.txt b/tests/compat/git/crlf_create_file/out/new.txt new file mode 100644 index 00000000..23eb407b --- /dev/null +++ b/tests/compat/git/crlf_create_file/out/new.txt @@ -0,0 +1,2 @@ +hello +world diff --git a/tests/compat/git/crlf_mode_only/in/foo.patch b/tests/compat/git/crlf_mode_only/in/foo.patch new file mode 100644 index 00000000..c8840975 --- /dev/null +++ b/tests/compat/git/crlf_mode_only/in/foo.patch @@ -0,0 +1,3 @@ +diff --git a/script.sh b/script.sh +old mode 100644 +new mode 100755 diff --git a/tests/compat/git/crlf_mode_only/in/script.sh b/tests/compat/git/crlf_mode_only/in/script.sh new file mode 100644 index 00000000..44a04582 --- /dev/null +++ b/tests/compat/git/crlf_mode_only/in/script.sh @@ -0,0 +1 @@ +echo hi diff --git a/tests/compat/git/crlf_mode_only/out/script.sh b/tests/compat/git/crlf_mode_only/out/script.sh new file mode 100644 index 00000000..44a04582 --- /dev/null +++ b/tests/compat/git/crlf_mode_only/out/script.sh @@ -0,0 +1 @@ +echo hi diff --git a/tests/compat/git/crlf_modify/in/file.txt b/tests/compat/git/crlf_modify/in/file.txt new file mode 100644 index 00000000..b87108ab --- /dev/null +++ b/tests/compat/git/crlf_modify/in/file.txt @@ -0,0 +1,3 @@ +line1 +line2 +line3 diff --git a/tests/compat/git/crlf_modify/in/foo.patch b/tests/compat/git/crlf_modify/in/foo.patch new file mode 100644 index 00000000..074c34f6 --- /dev/null +++ b/tests/compat/git/crlf_modify/in/foo.patch @@ -0,0 +1,9 @@ +diff --git a/file.txt b/file.txt +index 1111111..2222222 100644 +--- a/file.txt ++++ b/file.txt +@@ -1,3 +1,3 @@ + line1 +-line2 ++changed + line3 diff --git a/tests/compat/git/crlf_modify/out/file.txt b/tests/compat/git/crlf_modify/out/file.txt new file mode 100644 index 00000000..7f2f58ef --- /dev/null +++ b/tests/compat/git/crlf_modify/out/file.txt @@ -0,0 +1,3 @@ +line1 +changed +line3 diff --git a/tests/compat/git/crlf_rename_with_mode_change/in/foo.patch b/tests/compat/git/crlf_rename_with_mode_change/in/foo.patch new file mode 100644 index 00000000..75e73ba1 --- /dev/null +++ b/tests/compat/git/crlf_rename_with_mode_change/in/foo.patch @@ -0,0 +1,13 @@ +diff --git a/old.sh b/new.sh +old mode 100644 +new mode 100755 +similarity index 80% +rename from old.sh +rename to new.sh +index 1111111..2222222 +--- a/old.sh ++++ b/new.sh +@@ -1,2 +1,2 @@ + #!/bin/sh +-echo old ++echo new diff --git a/tests/compat/git/crlf_rename_with_mode_change/in/old.sh b/tests/compat/git/crlf_rename_with_mode_change/in/old.sh new file mode 100644 index 00000000..c7fb8334 --- /dev/null +++ b/tests/compat/git/crlf_rename_with_mode_change/in/old.sh @@ -0,0 +1,2 @@ +#!/bin/sh +echo old diff --git a/tests/compat/git/crlf_rename_with_mode_change/out/new.sh b/tests/compat/git/crlf_rename_with_mode_change/out/new.sh new file mode 100644 index 00000000..2bb564bc --- /dev/null +++ b/tests/compat/git/crlf_rename_with_mode_change/out/new.sh @@ -0,0 +1,2 @@ +#!/bin/sh +echo new diff --git a/tests/compat/git/mod.rs b/tests/compat/git/mod.rs index 91cb9a3c..4cbf8dfa 100644 --- a/tests/compat/git/mod.rs +++ b/tests/compat/git/mod.rs @@ -334,3 +334,49 @@ error: file.bin: patch does not apply fn junk_between_files() { Case::git("junk_between_files").strip(1).run(); } + +// Patches with CRLF line endings, as written by Windows tools or by a git +// checkout with `core.autocrlf` enabled. +// +// `git apply` treats the `\r` in hunk lines as content, and so does diffy: +// the patched files keep their CRLF line endings. + +// Observed: `git apply` creates the file with CRLF line endings (and warns +// about trailing whitespace). +#[test] +fn crlf_create_file() { + Case::git("crlf_create_file") + .strip(1) + .exact_line_endings() + .run(); +} + +#[test] +fn crlf_modify() { + Case::git("crlf_modify").strip(1).exact_line_endings().run(); +} + +// The `\r` is not part of the `rename from`/`rename to` paths or the modes. +#[test] +fn crlf_rename_with_mode_change() { + Case::git("crlf_rename_with_mode_change") + .strip(1) + .exact_line_endings() + .run(); +} + +// With no `---`/`+++` lines, the paths come from the `diff --git` line. +// diffy strips the `\r` from it, like GNU patch; `git apply` keeps it, so +// the two paths differ and it rejects the patch. +#[test] +fn crlf_mode_only() { + Case::git("crlf_mode_only") + .strip(1) + .exact_line_endings() + .expect_compat(false) + .expect_external_error(snapbox::str![[r#" +error: git diff header lacks filename information when removing 1 leading pathname component at :4 + +"#]]) + .run(); +} diff --git a/tests/compat/gnu_patch/crlf_patch_crlf_file/in/file.txt b/tests/compat/gnu_patch/crlf_patch_crlf_file/in/file.txt new file mode 100644 index 00000000..b87108ab --- /dev/null +++ b/tests/compat/gnu_patch/crlf_patch_crlf_file/in/file.txt @@ -0,0 +1,3 @@ +line1 +line2 +line3 diff --git a/tests/compat/gnu_patch/crlf_patch_crlf_file/in/foo.patch b/tests/compat/gnu_patch/crlf_patch_crlf_file/in/foo.patch new file mode 100644 index 00000000..ae174add --- /dev/null +++ b/tests/compat/gnu_patch/crlf_patch_crlf_file/in/foo.patch @@ -0,0 +1,7 @@ +--- file.txt ++++ file.txt +@@ -1,3 +1,3 @@ + line1 +-line2 ++changed + line3 diff --git a/tests/compat/gnu_patch/crlf_patch_crlf_file/out/file.txt b/tests/compat/gnu_patch/crlf_patch_crlf_file/out/file.txt new file mode 100644 index 00000000..7f2f58ef --- /dev/null +++ b/tests/compat/gnu_patch/crlf_patch_crlf_file/out/file.txt @@ -0,0 +1,3 @@ +line1 +changed +line3 diff --git a/tests/compat/gnu_patch/crlf_patch_lf_file/in/file.txt b/tests/compat/gnu_patch/crlf_patch_lf_file/in/file.txt new file mode 100644 index 00000000..83db48f8 --- /dev/null +++ b/tests/compat/gnu_patch/crlf_patch_lf_file/in/file.txt @@ -0,0 +1,3 @@ +line1 +line2 +line3 diff --git a/tests/compat/gnu_patch/crlf_patch_lf_file/in/foo.patch b/tests/compat/gnu_patch/crlf_patch_lf_file/in/foo.patch new file mode 100644 index 00000000..ae174add --- /dev/null +++ b/tests/compat/gnu_patch/crlf_patch_lf_file/in/foo.patch @@ -0,0 +1,7 @@ +--- file.txt ++++ file.txt +@@ -1,3 +1,3 @@ + line1 +-line2 ++changed + line3 diff --git a/tests/compat/gnu_patch/mod.rs b/tests/compat/gnu_patch/mod.rs index f57c1603..369656a7 100644 --- a/tests/compat/gnu_patch/mod.rs +++ b/tests/compat/gnu_patch/mod.rs @@ -231,3 +231,31 @@ fn fail_both_devnull() { .expect_external_error(snapbox::str!["GNU patch failed with status exit status: 1: "]) .run(); } + +// Patches with CRLF line endings, as written by Windows tools or by a git +// checkout with `core.autocrlf` enabled. +// +// GNU patch strips the `\r` from every line of a patch whose header lines +// end in CRLF, so it patches LF files and rejects CRLF ones. diffy keeps the +// `\r` in hunk lines as content, like `git apply`, so the results are the +// opposite. (`--binary` turns off GNU patch's stripping.) + +#[test] +fn crlf_patch_lf_file() { + Case::gnu_patch("crlf_patch_lf_file") + .expect_success(false) + .expect_compat(false) + .expect_diffy_error(snapbox::str!["apply error: error applying hunk #1"]) + .run(); +} + +#[test] +fn crlf_patch_crlf_file() { + Case::gnu_patch("crlf_patch_crlf_file") + .exact_line_endings() + .expect_compat(false) + .expect_external_error(snapbox::str![ + "GNU patch failed with status exit status: 1: " + ]) + .run(); +}