DiffFormatter.php
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	• Tgr
	Sep 27 2019, 1:08 PM

Description

I ran into this on Vagrant with the Echo role enabled. Don't see this error in production so it might be specific to Vagrant somehow, although it seems unlikely.

Notice: Undefined property: Diff::$edits in /vagrant/mediawiki/includes/diff/DiffFormatter.php on line 74
#0  {main}( ) .../api.php:0
#1  require( '/vagrant/mediawiki/api.php' ) .../api.php:5
#2  MediaWiki->doPostOutputShutdown( ) .../api.php:124
#3  MediaWiki->{closure:/vagrant/mediawiki/includes/MediaWiki.php:764-771}( ) .../MediaWiki.php:789
#4  MediaWiki->restInPeace( ) .../MediaWiki.php:766
#5  DeferredUpdates::doUpdates( ) .../MediaWiki.php:942
#6  DeferredUpdates::handleUpdateQueue( ) .../DeferredUpdates.php:149
#7  DeferredUpdates::run( ) .../DeferredUpdates.php:226
#8  DeferredUpdates::attemptUpdate( ) .../DeferredUpdates.php:281
#9  MWCallableUpdate->doUpdate( ) .../DeferredUpdates.php:383
#10 EchoHooks::{closure:/vagrant/mediawiki/extensions/Echo/includes/EchoHooks.php:535-542}( ) .../MWCallableUpdate.php:38 
#11 EchoDiscussionParser::generateEventsForRevision( ) .../EchoHooks.php:541
#12 EchoDiscussionParser::getChangeInterpretationForRevision( ) .../DiscussionParser.php:40
#13 EchoDiscussionParser::getMachineReadableDiff( ) .../DiscussionParser.php:480
#14 EchoDiffParser->getChangeSet( ) .../DiscussionParser.php:925
#15 UnifiedDiffFormatter->format( ) .../EchoDiffParser.php:89

The relevant code is

$diffs = new Diff( explode( "\n", $left ), explode( "\n", $right ) );
$format = new UnifiedDiffFormatter();
$diff = $format->format( $diffs );

and Diff::$edits is a public property so I don't understand what's going on here.

Details

	Project	Branch	Lines +/-	Subject
	mediawiki/services/parsoid	master	+474 -453	Remove phpspec

Customize query in gerrit

Related Objects

Mentioned Here: T166010: The Great Namespaceization and Reorg

Event Timeline

• Tgr created this task.Sep 27 2019, 1:08 PM

Restricted Application added a project: Growth-Team. · View Herald TranscriptSep 27 2019, 1:08 PM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Ok, apparently this is a namespace conflict: MediaWiki and phpspec (used by Parsoid) both have an un-namespaced Diff class, and the phpspec one is loaded via Composer so it takes precedence. So there is no production impact since you'd have to install MediaWiki in dev mode, but also it does not seem Vagrant-specific (other than that Vagrant installs Parsoid/PHP due to various weird dependency chains), affects all kinds of diff code and could break CI once Parsoid gets enabled there.

I'm not sure what could be done about it, other than getting rid of phpspec and/or wrapping up T166010: The Great Namespaceization and Reorg.

Upstream report: phpspec/phpspec#1262

On Vagrant, this breaks the GrowthExperiments question poster functionality (probably among other things) because the question poster triggers the Echo notification which triggers the error above, and that results in xdebug output being added to the API response.

We should get rid of phpspec since that was a short-lived experiment and looks like we've only used phpunit since. Do you want to port over those two test files? :-)

ssastry edited projects, added Parsoid-PHP; removed Parsoid.Sep 27 2019, 2:16 PM

ssastry triaged this task as Medium priority.Sep 27 2019, 4:13 PM

Change 540440 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/services/parsoid@master] Remove phpspec

https://gerrit.wikimedia.org/r/540440

gerritbot added a project: Patch-For-Review.Oct 2 2019, 4:41 PM

ssastry assigned this task to • Tgr.Oct 7 2019, 8:13 PM

@Tgr, please test with master version of Parsoid after the above patch merges and resolve this ticket if the issue is resolved.

Change 540440 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Remove phpspec

https://gerrit.wikimedia.org/r/540440

I am going to mark this resolved. Please reopen / refile if there is anything else that needs fixing.

Aklapper removed a project: Patch-For-Review.Apr 10 2020, 9:43 AM

Undefined property: Diff::$edits in includes/diff/DiffFormatter.phpClosed, ResolvedPublicActions

Description

Details

Related Objects

Event Timeline

Undefined property: Diff::$edits in includes/diff/DiffFormatter.php
Closed, ResolvedPublic
Actions