Improve parsing of unusual "git diff --no-index --no-prefix" diffs
Open, WishlistPublic
Actions

Assigned To

None

Authored By

	Pawka
	Jun 13 2017, 9:20 AM

Description

On some special cases ArcanistDiffParser fails to parse filenames out of the patch.

Data

For example try feed following patch to diff parser:

diff --git diff_staging/solo_score_timeout.10f178f2a16f4143aedb48f9dfaba3fe.1489777879750.json diff_staging/solo_score_timeout.10f178f2a16f4143aedb48f9dfaba3fe.1489777879750.new.json
index 9fd44d5..a3bdf2e 100644
--- diff_staging/solo_score_timeout.10f178f2a16f4143aedb48f9dfaba3fe.1489777879750.json
+++ diff_staging/solo_score_timeout.10f178f2a16f4143aedb48f9dfaba3fe.1489777879750.new.json
@@ -1,3 +1,3 @@
 {
-    "reason": "arstarst", 
+    "reason": "ienen", 
 }
\ No newline at end of file

JSON representation of the patch above:

"diff --git diff_staging\/solo_score_timeout.10f178f2a16f4143aedb48f9dfaba3fe.1489777879750.json diff_staging\/solo_score_timeout.10f178f2a16f4143aedb48f9dfaba3fe.1489777879750.new.json\r\nindex 9fd44d5..a3bdf2e 100644\r\n--- diff_staging\/solo_score_timeout.10f178f2a16f4143aedb48f9dfaba3fe.1489777879750.json\r\n+++ diff_staging\/solo_score_timeout.10f178f2a16f4143aedb48f9dfaba3fe.1489777879750.new.json\r\n@@ -1,3 +1,3 @@\r\n {\r\n-    \"reason\": \"arstarst\", \r\n+    \"reason\": \"ienen\", \r\n }\r\n\\ No newline at end of file"

Reproduction steps

Open https://secure.phabricator.com/conduit/method/differential.createrawdiff/ (ArcanistDiffParser is used in this API method)
Fill JSON representation from above to the diff field.
Submit the form.

Expected

Created a diff.

Received

An error: "#1048: Column 'filename' cannot be null"

Versions

This bug was introduced with rARCd0957c344156356123048be2219fb95a54c89a85 - every Phabricator instance with this version of Arcanist or newer has mentioned problem.
Reverting the file to previous version solves the problem.

Related Objects
Search...

		Status	Assigned	Task
		Open	None	T12664 Update diff/patch parsing to extract more metadata and parse a wider range of formats
		Open	None	T12835 Improve parsing of unusual "git diff --no-index --no-prefix" diffs

Event Timeline

Pawka created this task.Jun 13 2017, 9:20 AM

Pawka updated the task description. (Show Details)

How did you generate this diff?

@epriestley git diff --no-index --no-prefix -U2000 $old_file_path $new_file_path

Why is git diff --no-index being used instead of and diff, and why is --no-prefix being used?

--no-index - diff is created outside of repository by comparing directories.
--no-prefix - I'm not sure about this - some historical reasons and code in the service which generates diffs. But as I understand this was supported since D3744 and until rARCd0957c344156356123048be2219fb95a54c89a85.

That change fixed a lot of bugs, so we aren't going to revert it: reverting it would break all the things it fixed and added test cases for. --no-index --no-prefix diffs have never been part of the test suite and it is just coincidence that it worked before that change.

We'll fix this, but since it never occurs on any normal flow and requires explicitly selecting multiple unusual flags (--no-index, --no-prefix) it's difficult to prioritize. Omitting the --no-prefix flag is likely a workaround until the parser is updated.

epriestley renamed this task from ArcanistDiffParser fails to parse filenames to Improve parsing of unusual "git diff --no-index --no-prefix" diffs.Jul 3 2017, 10:48 AM

epriestley added a parent task: T12664: Update diff/patch parsing to extract more metadata and parse a wider range of formats.

avivey updated the task description. (Show Details)Jul 3 2017, 6:14 PM

joshuaspence added a subscriber: joshuaspence.Jul 11 2017, 2:26 PM

I'm getting the same #1048: Column 'filename' cannot be null running just regular "arc diff". It used to work and then at some point after updates started failing.

P2077

• GoogleLegacy added a subscriber: • GoogleLegacy.Feb 19 2018, 4:36 AM

epriestley moved this task from Backlog to Diff Parsing on the Arcanist board.Mar 5 2018, 2:16 PM