Page MenuHomePhabricator
Create Task
Maniphest T181940

Remove composer dump-autoload --optimize from mw-fetch-composer-dev.sh and Quibble
Closed, DeclinedPublic
Actions

Description

DONE mw-fetch-composer-dev.sh has

# FIXME: integration/composer is outdated and breaks autoloader
# Once we're on composer 1.0.0-alpha11 or higher this might not
# be needed anymore
composer dump-autoload --optimize

Quibble has

# FIXME integration/composer used to be outdated and broke the
# autoloader. Since composer 1.0.0-alpha11 the following might not
# be needed anymore.
subprocess.check_call([
    'composer', 'dump-autoload', '--optimize'],
    cwd=vendor_dir)

https://github.com/wikimedia/integration-composer has composer 1.1.3...

Can we remove this dump-autoload --optimize call?

Can we use a newer version of composer still?

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Remove deprecated dump-autoloadintegration/quibblemaster+0 -7
Remove `composer dump-autoload --optimize`integration/jenkinsmaster+0 -5
Customize query in gerrit

Related Objects

Event Timeline

Reedy created this task.Dec 3 2017, 4:48 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 3 2017, 4:48 PM
Reedy added a comment.Dec 3 2017, 6:05 PM

I can't work out what is special about 1.0.0-alpha11... https://github.com/composer/composer/blob/1.0.0-alpha11/CHANGELOG.md

gerritbot subscribed.Dec 4 2017, 2:07 AM

Change 394907 had a related patch set uploaded (by Reedy; owner: Reedy):
[integration/jenkins@master] Remove composer dump-autoload --optimize

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

gerritbot added a project: Patch-For-Review.Dec 4 2017, 2:07 AM
Reedy added a comment.Dec 4 2017, 2:08 AM
$ php ../composer.phar dumpautoload --optimize
Generating optimized autoload files

Seems to be a noop on an up to date repo...

Paladox subscribed.Dec 4 2017, 2:14 AM
hashar added a project: Quibble.Jul 31 2018, 10:22 PM
hashar subscribed.
# FIXME integration/composer used to be outdated and broke the
# autoloader. Since composer 1.0.0-alpha11 the following might not
# be needed anymore.
subprocess.check_call([
    'composer', 'dump-autoload', '--optimize'],
    cwd=vendor_dir)

Note: most of the logic has been ported to Quibble as is. So that is an issue in Quibble as well.

gerritbot added a comment.Sep 6 2018, 11:25 PM

Change 394907 merged by jenkins-bot:
[integration/jenkins@master] Remove composer dump-autoload --optimize

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

hashar renamed this task from Address fixme in mw-fetch-composer-dev.sh to Remove composer dump-autoload --optimize from mw-fetch-composer-dev.sh and Quibble.Sep 7 2018, 8:18 PM
hashar triaged this task as Low priority.
hashar updated the task description. (Show Details)
hashar updated the task description. (Show Details)
hashar removed projects: Patch-For-Review, Continuous-Integration-Infrastructure.Aug 26 2019, 9:52 AM
gerritbot added a comment.Apr 19 2020, 8:56 AM

Change 590273 had a related patch set uploaded (by Awight; owner: Awight):
[integration/quibble@master] Remove deprecated dump-autoload

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

gerritbot added a project: Patch-For-Review.Apr 19 2020, 8:56 AM
awight moved this task from Backlog to In progress on the Quibble board.Apr 19 2020, 8:57 AM
gerritbot added a comment.May 3 2020, 11:33 PM

Change 590273 merged by jenkins-bot:
[integration/quibble@master] Remove deprecated dump-autoload

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

Krinkle awarded a token.May 4 2020, 12:06 AM
hashar added a subscriber: awight.May 6 2020, 12:27 PM

We had to revert because, and I quote @awight:

FileImporter's Hamcrest function is not working with Quibble 0.0.43 somehow?

00:04:43.431 1) FileImporter\Tests\Html\ChangeFileInfoFormTest::testTextDisplayedInInputBox with data set #0 ('Some Input Text', 'Some Input Text\n')
00:04:43.431 === Logs generated by test case
00:04:43.431 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
00:04:43.432 [localisation] [debug] LocalisationCache using store LCStoreNull []
00:04:43.432 [localisation] [debug] LocalisationCache using store LCStoreNull []
00:04:43.432 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
00:04:43.432 [localisation] [debug] LocalisationCache::isExpired(en): cache missing, need to make one []
00:04:43.432 [objectcache] [debug] fetchOrRegenerate(wikidb-unittest_:interwiki:fileimporter\tests\html\changefileinfoformtest): miss, new value computed []
00:04:43.432 [wfDebug] [debug] IP: 127.0.0.1 {"private":false}
00:04:43.432 [wfDebug] [debug] IP: 127.0.0.1 {"private":false}
00:04:43.432 [MediaWiki\Content\ContentHandlerFactory::getContentHandler] [info] Registered handler for wikitext: WikitextContentHandler {"private":false}
00:04:43.432 [UserOptionsManager] [debug] Loading options from database {"user_id":33}
00:04:43.432 ===
00:04:43.432 Error: Call to undefined function FileImporter\Tests\Html\is()
00:04:43.432 
00:04:43.432 /workspace/src/extensions/FileImporter/tests/phpunit/Html/ChangeFileInfoFormTest.php:119
00:04:43.432 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:423
00:04:43.432 /workspace/src/maintenance/doMaintenance.php:105

Hamcrest is installed into /vendor earlier in the build; is it accidentally getting wiped out?

The failure is caused by 8191974690e7a84344664cfdc6b95864e473eeab which removed composer dump-autoload --optimize.

My guess is that the global function isn't autoloaded correctly, probably solved by replacing is with the namespaced version \Hamcrest\Core\Is::is. In the meantime, we can revert the quibble autoload patch, since it's just a cleanup and nothing is dependent.

I should clarify: I was able to reproduce the problem locally, and reverting the "dump-autoload" quibble change fixed it for me. I verified that the Hamcrest library is present in vendor either way, the "git clean" change doesn't damage the libs.

hashar added a comment.May 6 2020, 1:00 PM

I cannot reproduce the issue locally using just mediawiki/extensions/FileImporter. Do we have a build log from CI?

I did tests using composer 1.10.5 and I can not tell any I also can not tell any difference when Quibble does or does not run composer dump-autoload --optimize.

Moreover, the tests are passing in both case for me using:

ZUUL_PROJECT=mediawiki/extensions/FileImporter quibble --git-cache=/home/hashar/projects --db=sqlite --packages-source=composer --run=phpunit --phpunit-testsuite=extensions
Reedy added a comment.May 6 2020, 1:30 PM

My guess is that the global function isn't autoloaded correctly, probably solved by replacing is with the namespaced version \Hamcrest\Core\Is::is.

I think that's probably just good practice. I'll file a task about doing that

Reedy mentioned this in T252014: Replace hamcrest global functions with namespaced class static functions.May 6 2020, 1:32 PM
awight added a comment.May 6 2020, 4:36 PM
In T181940#6112465, @hashar wrote:
ZUUL_PROJECT=mediawiki/extensions/FileImporter quibble --git-cache=/home/hashar/projects --db=sqlite --packages-source=composer --run=phpunit --phpunit-testsuite=extensions

I was using --packages-source=vendor, looks like that would make the difference.

awight added a comment.May 6 2020, 4:38 PM
In T181940#6112567, @Reedy wrote:

My guess is that the global function isn't autoloaded correctly, probably solved by replacing is with the namespaced version \Hamcrest\Core\Is::is.

I think that's probably just good practice. I'll file a task about doing that

Looking at the usage, there are a lot of global functions pulled in for Hamcrest, and test code might become hard to read with all the namespaces. Even with use statements, it would look like Is::is. Maybe there's another way, where we just leverage composer's newer functionality? I see that composer require has a flag to include optimized autoload dumping, for example...

hashar added a comment.May 6 2020, 10:27 PM
In T181940#6113498, @awight wrote:
In T181940#6112465, @hashar wrote:
ZUUL_PROJECT=mediawiki/extensions/FileImporter quibble --git-cache=/home/hashar/projects --db=sqlite --packages-source=composer --run=phpunit --phpunit-testsuite=extensions

I was using --packages-source=vendor, looks like that would make the difference.

Indeed, I can reproduce it now :)

rm -f src/LocalSettings.php; quibble --git-cache=$HOME/projects --db=sqlite --packages-source=vendor --run=phpunit --phpunit-testsuite=extensions -- mediawiki/extensions/FileImporter
hashar added a comment.May 7 2020, 12:46 PM

And here is a diff with composer dump-autoload --optimize and without it. We are missing:

  • three classes from Mediawiki includes/composer
  • hamcrest/hamcrest-php/hamcrest/Hamcrest.php'
  • wmde/hamcrest-html-matchers/src/functions.php
diff -ru vendor-ok/composer/autoload_classmap.php vendor-broken/composer/autoload_classmap.php
--- vendor-ok/composer/autoload_classmap.php	2020-05-07 14:37:13.137400874 +0200
+++ vendor-broken/composer/autoload_classmap.php	2020-05-07 00:23:06.204708885 +0200
@@ -123,9 +123,6 @@
     'Cdb\\Writer\\DBA' => $vendorDir . '/wikimedia/cdb/src/Writer/DBA.php',
     'Cdb\\Writer\\PHP' => $vendorDir . '/wikimedia/cdb/src/Writer/PHP.php',
     'Comparable' => $vendorDir . '/data-values/data-values/src/interfaces/Comparable.php',
-    'ComposerHookHandler' => $baseDir . '/../includes/composer/ComposerHookHandler.php',
-    'ComposerPhpunitXmlCoverageEdit' => $baseDir . '/../includes/composer/ComposerPhpunitXmlCoverageEdit.php',
-    'ComposerVendorHtaccessCreator' => $baseDir . '/../includes/composer/ComposerVendorHtaccessCreator.php',
     'Composer\\Semver\\Comparator' => $vendorDir . '/composer/semver/src/Comparator.php',
     'Composer\\Semver\\Constraint\\AbstractConstraint' => $vendorDir . '/composer/semver/src/Constraint/AbstractConstraint.php',
     'Composer\\Semver\\Constraint\\Constraint' => $vendorDir . '/composer/semver/src/Constraint/Constraint.php',
diff -ru vendor-ok/composer/autoload_files.php vendor-broken/composer/autoload_files.php
--- vendor-ok/composer/autoload_files.php	2020-05-07 14:37:13.137400874 +0200
+++ vendor-broken/composer/autoload_files.php	2020-05-07 00:23:06.204708885 +0200
@@ -32,6 +32,4 @@
     '801c31d8ed748cfa537fa45402288c95' => $vendorDir . '/psy/psysh/src/functions.php',
     '4d945db823e5f6ca6dd83ad1f5fbcc43' => $vendorDir . '/wikimedia/relpath/src/RelPath/RelPath.php',
     '6513700b70192b7dfd0e5e9fc8082cf3' => $vendorDir . '/wikimedia/relpath/src/Wikimedia/RelPath.php',
-    '7f3c89f3ec643bfa9bcbc28aa6156778' => $vendorDir . '/hamcrest/hamcrest-php/hamcrest/Hamcrest.php',
-    '98e5ce360aab3ada6c3c53532d1b525c' => $vendorDir . '/wmde/hamcrest-html-matchers/src/functions.php',
 );
diff -ru vendor-ok/composer/autoload_namespaces.php vendor-broken/composer/autoload_namespaces.php
--- vendor-ok/composer/autoload_namespaces.php	2020-05-07 14:37:13.137400874 +0200
+++ vendor-broken/composer/autoload_namespaces.php	2020-05-07 00:23:06.204708885 +0200
@@ -18,8 +18,5 @@
     'JsonMapper' => array($vendorDir . '/netresearch/jsonmapper/src'),
     'DataValues\\' => array($vendorDir . '/data-values/number/src', $vendorDir . '/data-values/time/src'),
     'Console' => array($vendorDir . '/pear/console_getopt'),
-    'ComposerVendorHtaccessCreator' => array($baseDir . '/../includes/composer'),
-    'ComposerPhpunitXmlCoverageEdit' => array($baseDir . '/../includes/composer'),
-    'ComposerHookHandler' => array($baseDir . '/../includes/composer'),
     '' => array($vendorDir . '/cssjanus/cssjanus/src', $vendorDir . '/pear/pear-core-minimal/src'),
 );
diff -ru vendor-ok/composer/autoload_static.php vendor-broken/composer/autoload_static.php
--- vendor-ok/composer/autoload_static.php	2020-05-07 14:37:13.137400874 +0200
+++ vendor-broken/composer/autoload_static.php	2020-05-07 00:23:06.204708885 +0200
@@ -33,8 +33,6 @@
         '801c31d8ed748cfa537fa45402288c95' => __DIR__ . '/..' . '/psy/psysh/src/functions.php',
         '4d945db823e5f6ca6dd83ad1f5fbcc43' => __DIR__ . '/..' . '/wikimedia/relpath/src/RelPath/RelPath.php',
         '6513700b70192b7dfd0e5e9fc8082cf3' => __DIR__ . '/..' . '/wikimedia/relpath/src/Wikimedia/RelPath.php',
-        '7f3c89f3ec643bfa9bcbc28aa6156778' => __DIR__ . '/..' . '/hamcrest/hamcrest-php/hamcrest/Hamcrest.php',
-        '98e5ce360aab3ada6c3c53532d1b525c' => __DIR__ . '/..' . '/wmde/hamcrest-html-matchers/src/functions.php',
     );
 
     public static $prefixLengthsPsr4 = array (
@@ -716,18 +714,6 @@
             array (
                 0 => __DIR__ . '/..' . '/pear/console_getopt',
             ),
-            'ComposerVendorHtaccessCreator' => 
-            array (
-                0 => __DIR__ . '/..' . '/../includes/composer',
-            ),
-            'ComposerPhpunitXmlCoverageEdit' => 
-            array (
-                0 => __DIR__ . '/..' . '/../includes/composer',
-            ),
-            'ComposerHookHandler' => 
-            array (
-                0 => __DIR__ . '/..' . '/../includes/composer',
-            ),
         ),
     );
 
@@ -854,9 +840,6 @@
         'Cdb\\Writer\\DBA' => __DIR__ . '/..' . '/wikimedia/cdb/src/Writer/DBA.php',
         'Cdb\\Writer\\PHP' => __DIR__ . '/..' . '/wikimedia/cdb/src/Writer/PHP.php',
         'Comparable' => __DIR__ . '/..' . '/data-values/data-values/src/interfaces/Comparable.php',
-        'ComposerHookHandler' => __DIR__ . '/..' . '/../includes/composer/ComposerHookHandler.php',
-        'ComposerPhpunitXmlCoverageEdit' => __DIR__ . '/..' . '/../includes/composer/ComposerPhpunitXmlCoverageEdit.php',
-        'ComposerVendorHtaccessCreator' => __DIR__ . '/..' . '/../includes/composer/ComposerVendorHtaccessCreator.php',
         'Composer\\Semver\\Comparator' => __DIR__ . '/..' . '/composer/semver/src/Comparator.php',
         'Composer\\Semver\\Constraint\\AbstractConstraint' => __DIR__ . '/..' . '/composer/semver/src/Constraint/AbstractConstraint.php',
         'Composer\\Semver\\Constraint\\Constraint' => __DIR__ . '/..' . '/composer/semver/src/Constraint/Constraint.php',
hashar added a comment.May 7 2020, 12:53 PM

And in mediawiki/core we have:

composer.json
"autoload": {
    "psr-0": {
        "ComposerHookHandler": "includes/composer",
        "ComposerVendorHtaccessCreator": "includes/composer",
        "ComposerPhpunitXmlCoverageEdit":"includes/composer"
    }
},
"autoload-dev": {
    "files": [
        "vendor/hamcrest/hamcrest-php/hamcrest/Hamcrest.php",
        "vendor/wmde/hamcrest-html-matchers/src/functions.php"
    ]
},

Which are the classes we are missing.

Which brings me back to T158674 and https://gerrit.wikimedia.org/r/#/c/339202/ which added for Wikibase:

composer config extra.merge-plugin.include "$MW_INSTALL_PATH/composer.json"

So I guess we should keep composer dump-autoload --optimize.

hashar closed this task as Declined.May 7 2020, 12:54 PM
Reedy added a comment.May 7 2020, 1:34 PM

Might be worth some inline docs so we don't try and remove it freely again...

hashar moved this task from In progress to Enhancement on the Quibble board.Apr 11 2026, 1:42 PM
Maintenance_bot removed a project: Patch-For-Review.Apr 11 2026, 2:31 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits