Page MenuHomePhabricator
Create Task
Maniphest T11813

"Can't parse an empty diff!" in Differential when ignoring most whitespace changes
Closed, DuplicatePublic
Actions

Description

I'm seeing unexpected behavior when viewing Differential revisions to Mercurial repositories. Specifically, a large number of revisions simply display a pop-up with the following message:

Unhandled Exception ("Exception")

Can't parse an empty diff!

Interestingly, when I change the whitespace policy from "Ignore Most" to "Show All," I don't receive any error message and the diff displays correctly.

Please see the attached screenshots, which include a specific revision that illustrates this behavior.

Screen Shot 2016-11-03 at 7.54.37 AM.png (1×2 px, 200 KB)
Screen Shot 2016-11-03 at 7.54.44 AM.png (1×2 px, 262 KB)
Screen Shot 2016-11-03 at 7.55.04 AM.png (1×2 px, 277 KB)

I can also reproduce this behavior on a number of other revisions although I haven't tried to create a minimal test case against an empty repository (yet).

Version Information

stable branch, updated this morning

ApplicationVersionDate
phabricatorbd256e9f3fdbb74db78ff994925402cee9d26759Fri, Oct 28
arcaniste17fe43ca3fe6dc6dd0b5ce056f56310ea1d3d51Fri, Oct 21
phutil1961d5518822d88e74c2635e4374fc1e6be8160aFri, Oct 28

Related Objects

Event Timeline

jmc7tp created this task.Nov 3 2016, 1:51 PM
epriestley edited projects, added Bug Report (Needs Information); removed Bug Report.Nov 3 2016, 6:27 PM
epriestley added a subscriber: epriestley.

I can't reproduce this by following the steps provided. Here's what I did:

  • I created a diff against a Mercurial repository.
  • I viewed the diff with "Whitespace Changes" set to "Ignore Most":
  • Everything worked fine:

Screen Shot 2016-11-03 at 11.26.20 AM.png (502×1 px, 67 KB)

We need more information about how to reproduce this issue before we can move forward. See Providing Reproduction Steps for details.

jmc7tp added a comment.Nov 5 2016, 3:40 PM

Thanks for the quick feedback. Perhaps it's something unique to my environment because I can reproduce this behavior consistently across multiple repositories.

Here's a series of commits that I used to verify the unexpected behavior:

  1. Create new Mercurial repository (using the web interface, if that makes any difference)
  2. Clone repository
  3. Configure Arcanist by adding .arcconfig
diff -r 000000000000 -r 6c80a79b6943 .arcconfig
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/.arcconfig	Sat Nov 05 09:52:49 2016 -0400
@@ -0,0 +1,3 @@
+{
+  "phabricator.uri": /* STRIPPED FROM EXAMPLE */
+}
  1. Push changeset to remote repository
  2. Add .hgignore file and commit
diff -r 6c80a79b6943 -r fc07a2bca3d9 .hgignore
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/.hgignore	Sat Nov 05 09:53:15 2016 -0400
@@ -0,0 +1,3 @@
+syntax: glob
+
+*.orig
  1. Use arc diff to create revision
  2. Push changeset to remote repository (which closes prior revision)
  3. Modify .hgignore by adding a new line and commit
diff -r fc07a2bca3d9 -r 7976041da85c .hgignore
--- a/.hgignore	Sat Nov 05 09:53:15 2016 -0400
+++ b/.hgignore	Sat Nov 05 09:55:19 2016 -0400
@@ -1,3 +1,4 @@
 syntax: glob
 
+.*.swp
 *.orig
  1. Use arc diff to create revision

The revision for Step 6 displays as expected in Differential -- i.e., no errors and diff matches that shown above. The revision for Step 9 displays the error messages shown previously when "Ignore All" or "Ignore Most" is selected in the "Whitespace Changes" dropdown list box.

Here's a backtrace from the server log:

[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46] [2016-11-05 10:57:36] 
EXCEPTION: (Exception) Can't parse an empty diff! at [<arcanist>/src/parser/ArcanistDiffParser.php:191]
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46] arcanist(head=stable, ref.master=296250485541, ref.stable=e17fe43ca3fe), phabricator(head=stable, ref.master=754397c4e75d, ref.stable=bd256e9f3fdb), phutil(head=stable, ref.master=50cd143e07ca, ref.stable=1961d5518822)
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46]   #0 <#2> ArcanistDiffParser::parseDiff(string) called at [<phabricator>/src/infrastructure/diff/PhabricatorDifferenceEngine.php:165]
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46]   #1 <#2> PhabricatorDifferenceEngine::generateChangesetFromFileContent(string, string) called at [<phabricator>/src/applications/differential/parser/DifferentialChangesetParser.php:710]
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46]   #2 <#2> DifferentialChangesetParser::process() called at [<phabricator>/src/applications/differential/parser/DifferentialChangesetParser.php:645]
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46]   #3 <#2> DifferentialChangesetParser::tryCacheStuff() called at [<phabricator>/src/applications/differential/parser/DifferentialChangesetParser.php:859]
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46]   #4 <#2> DifferentialChangesetParser::render(NULL, NULL, array) called at [<phabricator>/src/applications/differential/parser/DifferentialChangesetParser.php:74]
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46]   #5 <#2> DifferentialChangesetParser::renderChangeset() called at [<phabricator>/src/applications/differential/controller/DifferentialChangesetViewController.php:256]
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46]   #6 <#2> DifferentialChangesetViewController::handleRequest(AphrontRequest) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:257]
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46]   #7 phlog(Exception) called at [<phabricator>/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php:41]
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46]   #8 PhabricatorDefaultRequestExceptionHandler::handleRequestException(AphrontRequest, Exception) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:671]
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46]   #9 AphrontApplicationConfiguration::handleException(Exception) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:262]
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46]   #10 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:169]
[Sat Nov 05 10:57:36 2016] [error] [client 108.18.197.46]   #11 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:17]

Incidentally, this Phabricator installation is running on Solaris (Solaris 11.3) if that makes any difference.

epriestley added a comment.Nov 5 2016, 3:42 PM

Incidentally, this Phabricator installation is running on Solaris (Solaris 11.3) if that makes any difference.

It may, the default diff on Solaris is nonstandard. See a note in T4124.

Can you reproduce this on a non-Solaris system? For example, use arc diff --conduit-uri=https://secure.phabricator.com/ to send a diff to this system. (Don't send anything sensitive!)

jmc7tp added a comment.EditedNov 5 2016, 3:47 PM

Yes, I think that it's the same issue as T4124: Test the capabilities of `diff` better, especially for Solaris, as I just found the -L options in generateRawDiffFromFileContent.

I will try the command that you specified and update momentarily.

epriestley added a comment.Nov 5 2016, 3:48 PM

We've only seen that one other time, so it hasn't been a priority to fix. I believe the other install which hit the issue was able to install GNU diff to resolve it.

jmc7tp added a comment.EditedNov 5 2016, 3:55 PM

I'm quite certain that it's the -L option. Let me talk to the system administrator about installing the GNU diffutils -- I'm guessing that it won't be a problem.

Thanks for the help!

epriestley added a comment.Nov 5 2016, 3:56 PM

Cool. I'll merge this into T4124 and retitle that to be a little more specific.

epriestley closed this task as a duplicate of T4124: Test the capabilities of `diff` better, especially for Solaris.Nov 5 2016, 3:56 PM
cspeckmim mentioned this in T4124: Test the capabilities of `diff` better, especially for Solaris.Feb 1 2017, 9:27 PM
Phabricator · Built by Phacility