1591015 - Submitting via moz-phab with --no-arc fails to handle windows line endings properly

Status: RESOLVED FIXED

(Conduit :: moz-phab, defect, P1)

Description

[Mark Banner (:standard8)]

I've been trying to get bug 1556854 submitted, and I've been getting various errors from the code review bots:

patching file dom/media/test/test_eme_pssh_in_moof.html
Hunk #1 FAILED at 108
Hunk #2 FAILED at 137
2 out of 2 hunks FAILED -- saving rejects to file dom/media/test/test_eme_pssh_in_moof.html.rej
abort: patch failed to apply

and from lando:

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. applying /tmp/tmp_3uxMQ dom/media/test/test_eme_pssh_in_moof.html Hunk #1 FAILED at 109 (different line endings). Hunk #2 FAILED at 138 (different line endings). 

I tried this from both git and hg repositories, and I eventually realised that this is due to the using --no-arc when calling moz-phab submit. I have just successfully submitted without --no-arc.

STR

1. Make a change to a file in the tree that has windows line endings, e.g. dom/media/test/test_eme_pssh_in_moof.html
2. Submit to phabricator with --no-arc

Expected Results

• The patch can have code analysis run on it and be applied via lando.

Actual Results

• Analysis and Lando fail to apply the patches, also moz-phab patch fails.

There's probably not too many windows line-endings left in the tree, but it feels like this should work anyway.

Comment 1

[Piotr Zalewa [:zalun]]

Confirmed (submit/patch)

$ DEBUG=1 mozphab patch D1501
[...]
hg.exe --config extensions.rebase= --pager never import 'C:\Users\mozilla\AppData\Local\Temp\tmp67jfc8vq' --quiet -l 'C:\Users\mozilla\AppData\Local\Temp\tmp6hsublj9' -u 'User Name <[e-mail]>' -d 2019-10-24T14:07:15
abort: bad hunk #1 @@ -4,4 +4,7 @@
 (5 4 7 7)
[...]

The patch returned from Phabricator:

$ moz-phab patch D1501 --raw
diff --git a/notepad document.txt b/notepad document.txt
--- a/notepad document.txt
+++ b/notepad document.txt
@@ -4,4 +4,7 @@

 with the last line empty

-there will be some more lines
\ No newline at end of file
+there will be some more lines
+
+
+

Comment 2

[Piotr Zalewa [:zalun]]

Attachment: Bug 1591015 - Temporarily switch off --no-arc on Windows. r=glob

Comment 3

[Mark Banner (:standard8)]

I'm running on Mac...

Comment 4

[Piotr Zalewa [:zalun]]

git is not printing the CR character when it is removed from the file

File created:

$ git diff HEAD~
diff --git a/test_clrf b/test_clrf
new file mode 100644
index 0000000..0e7d2a2
--- /dev/null
+++ b/test_clrf
@@ -0,0 +1 @@
+line^M

removed the ^M, commited

$ git diff HEAD~
diff --git a/test_clrf b/test_clrf
index 0e7d2a2..a999a0c 100644
--- a/test_clrf
+++ b/test_clrf
@@ -1 +1 @@
-line
+line

Comment 5

[Piotr Zalewa [:zalun]]

Tried --no-textconv, -c core.autocrlf=false with no luck

Comment 6

[Piotr Zalewa [:zalun]]

It's the Python's line normalization removing the \r characters.
Finishing the patch.

Comment 7

[Piotr Zalewa [:zalun]]

Attachment: Bug 1591015 - CRLF allowed in git and hg r=nika,glob!

Python was stripping the \r from the diff response.

Warning:
Please take a look at detecting the \\ No new line at end of file\n
string. There is a change as we now have the \n character at the end
of this line. We might want to change it.

Warning:
There is no test for update yet. Creation was working fine even before
the fix.

Note:
recognizing lack of the \n at the end of the file was broken.