Welcome to the Splat alpha!

Interesting. It looks like the problem was that the file didn't have a newline at the
end and that confused our code. We ended up generating a diff without a trailing
newline. We should fix this.

• Tags:

  -	Priority-Medium
  +	Priority-High
  +	Milestone-Release1.0
  +	Component-DiffParser

• Summary:

  +	Files without a trailing newline break the diff parser.

• Assigned to:

  +	chipx86

hit this error (see 359 marked as duplicate).

Symptoms are that it raises an exception while trying to patch a non-existing
old file (because the file we want to check in is new/ marked for add on perforce).
maybe some checks to see if oldfile exists would be relevant.

Steps to reproduce:
1. create a new file (e.g. perl script) that is open for add.
2. no trailing new line (actually i tried to upload diffs again with trailing newline
and still fails)
3. post-review

e.g. of diff:
      1 --- Perl/userworld.pl  //depot/non-framework/userworld.pl#1
      2 +++ Perl/userworld.pl  08-01-28 15:58:51
      3 @@ -0,0 +1,150 @@
    ...

post-review,l.322:
        f = os.popen('diff -urNp "%s" "%s"' % (old_file, new_file), 'r')
        dl = f.readlines()
        f.close()

this generate broken diff in my case, old_file is empty file. I noticed I had
carriage return at the beginning of the line causing diff to misbehave.

someperl.pl:
      1 #!/bin/perl
      2 # Copyright (C) blablah
      3 
      4 
      5 ^Msub main;
      6 
      7 main;
      8 
      9 sub main
     10 {
     11         return 0;
     12 }
     13 

this is the file it outputs:
      1 --- /dev/null   2008-01-21 14:43:44.512730151 -0800
      2 +++ someperl.pl 2008-02-06 18:18:56.007370000 -0800
      3 @@ -0,0 +1,13 @@
      4 +#!/bin/perl
      5 +# Copyright (C) blablah
      6 +
      7 +
      8 +^Msub main;
      9 +
     10 +main;
     11 +
     12 +sub main
     13 +{
     14 +       return 0;
     15 +}
     16 +


but this is how the script treats it:
--- /dev/null   2008-01-21 14:43:44.512730151 -0800
+++ someperl.pl 2008-02-06 18:18:56.007370000 -0800
@@ -0,0 +1,13 @@
+#!/bin/perl
+# Copyright (C) blablah
+
+
sub main;
+
+main;
+
+sub main
+{
+       return 0;
+}
+

if Ryan also has ^M issues, then we should rename this thread as ^M breaks diff parser.

These are actually two completely separate issues. One involves the lack of trailing
newlines breaking a diff (due to how we concatenate the per-file diffs) and the other
involves how we handle conversion to a standard newline across files.

We should probably have a separate bug filed for your issue.

bug 359? Is 359 really a duplicate or was it the same issue that I hit?

I've been trying to figure out the exact problem and I think I've got it. The problem
is that the files without a trailing newline come to us from Perforce as having \r at
the end. We translate this to \r\n, which breaks patches.

There's a couple ways we may be able to fix this.

There are some things we can do here, but it'll be interesting to see if they manage
to break anything.

One thing we can do is change our convert_line_endings function to check if the last
character in the file is "\r" and, if so, preserve that. This should fix the patches.

Another is to normalize when it comes out of Perforce by stripping the last \r if it
exists. If the file uses \r all throughout, though, then this is technically wrong.

Since GNU diff sees \r at the end of a file as the file not having a newline (as
indicated by the "\ No newline at end of file" markers), even if \r is used instead
of \n or \r\n throughout the file, then we can probably go ahead and preserve that
last \r.

The first suggestion there seems to be the least bad.

Okay, this should be fixed in SVN now. I'll update the VMware Review Board server
later this week and verify it's taken care of there in the reported cases.

• Status:

  +	Fixed

I am facing similar issue too. Could you please provide us the patched file which fixed the problem.

This was fixed 4 years ago. You're likely hitting a brand new issue, and will need to open a new bug. We don't have any patched file that fixes your problem, or it would be part of the codebase.