Page MenuHomePhabricator

phan issue due to a composer dev dependency
Closed, ResolvedPublic

Description

Since phan got migrated to use Quibble, @Umherirrender reported:

It is possible that now the dev dependency are part of the vendor folder?
includes/RemoteSchema.php:9 PhanRedefinedInheritedInterface \RemoteSchema inherits abstract Interface \JsonSerializable declared at internal:0 which is also declared at ../../vendor/jakub-onderka/php-parallel-lint/src/JsonSerializable.php:4. This may lead to confusing errors.
That was not before, it was breaking Echo - I1cffc88a3f4ea4290892c0624a6f32e852cde1dd

Related to the migration of phan to Quibble and to use of composer instead of vendor

https://gerrit.wikimedia.org/r/#/c/integration/config/+/503362/
https://gerrit.wikimedia.org/r/#/c/integration/config/+/508320/

Event Timeline

hashar created this task.May 15 2019, 5:57 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 15 2019, 5:57 PM
hashar added a comment.EditedMay 15 2019, 6:15 PM

Before switching to Quibble, we were using docker-registry.wikimedia.org/releng/ci-src-setup:0.2.9 and its entrypoint setup-mwext.sh:

dockerfiles/ci-src-setup/setup-mwext.sh
#!/bin/bash -eu

set -euxo pipefail

umask 002

echo $ZUUL_PROJECT > /tmp/deps.txt
echo -e "${EXT_DEPENDENCIES:-}" >> /tmp/deps.txt
echo -e "${SKIN_DEPENDENCIES:-}" > /tmp/deps_skins.txt

cd /src

zuul-cloner --version
zuul-cloner \
            --color \
            --verbose \
            --map /srv/deployment/integration/slave-scripts/etc/zuul-clonemap.yaml \
            --workspace /src \
            --cache-dir /srv/git \
            https://gerrit.wikimedia.org/r/p \
            mediawiki/core \
            mediawiki/vendor \
            $(cat /tmp/deps.txt) \
            $(cat /tmp/deps_skins.txt)

find extensions skins -maxdepth 2 \
            -name .gitmodules \
            -execdir bash -xe -c '
                git submodule foreach git clean -xdff -q
                git submodule update --init --recursive
                git submodule status
                ' \;

echo $ZUUL_PROJECT > /tmp/extensions_load.txt
echo -e "${EXT_DEPENDENCIES:-}" >> /tmp/extensions_load.txt

set -u

composer --ansi validate --no-check-publish
/srv/deployment/integration/slave-scripts/bin/mw-create-composer-local.py "/tmp/extensions_load.txt" composer.local.json
composer update --ansi --no-progress --prefer-dist --profile --no-dev
cd /src/$THING_SUBNAME
composer update --ansi --no-progress --prefer-dist --profile

In short:

  • clone mediawiki/vendor
  • create the composer.local.json for the composer merge plugin
  • From root of mediawiki/core, run composer update --no-dev # EDIT: --no-dev!
  • From root of the extension run composer update

The last step brings eg jakub-onderka/php-parallel-lint or mediawiki/mediawiki-phan-config

The switch to Quibble is https://gerrit.wikimedia.org/r/#/c/integration/config/+/503362/6/jjb/macro-docker.yaml , it drops the use of setup-mwext.sh script from above and instead does:

  • clone mediawiki/vendor
  • create the composer.local.json for the composer merge plugin

Those steps are identical.

Quibble then does:

  • From root of mediawiki/core, run composer update

It does not pass --no-dev and thus the development dependencies of mediawiki/core are fectched.

The commands to run then does the final step: From root of the extension run composer update.

Eventually I have encountered an issue with sqlite when MediaWiki installed for some extension so in https://gerrit.wikimedia.org/r/#/c/integration/config/+/508320/2/jjb/macro-docker.yaml I have switched to use MySQL and replaced mediawiki/vendor.git with composer.

So I guess the failure is that with the switch to composer, the mediawiki dev dependencies are now installed. That conflicts with the dev dependencies installed with the final step From root of the extension run composer update.

So I guess Quibble should just use vendor. To be confirmed since I can not remember whether Quibble would also install mediawiki/core dev dependencies. But that can be looked up in the build failure of Echo.

https://integration.wikimedia.org/ci/job/mwext-php70-phan-docker/28380/consoleFull has the PhanRedefinedInheritedInterface error.

There is no mediawiki/vendor:

00:00:13.559 INFO:quibble.cmd:Projects: mediawiki/core, mediawiki/skins/Vector, mediawiki/extensions/EventLogging

composer update on mediawiki/core does install the dev dependencies and php-parallel-lint v0.9.2:

00:00:27.284 INFO:quibble.cmd:Running "composer update for mediawiki/core
00:00:38.521 [279.1MB/11.06s]  Extracting archive[279.1MB/11.10s]   - Installing jakub-onderka/php-parallel-lint (v0.9.2): [279.1MB/11.10s] Loading from cache[279.1MB/11.10s]

composer update for the extension installs the same dev dependency, albeit at a different version:

00:02:36.213 INFO:test.commands:cd "$THING_SUBNAME" && composer update --ansi --no-progress --prefer-dist --profile
00:02:38.759 [181.8MB/2.37s]   - Installing jakub-onderka/php-parallel-lint (v1.0.0): [181.9MB/2.37s] Loading from cache[181.9MB/2.37s]

PhanRedefinedInheritedInterface is emitted.


Looking at another build that uses mediawiki/vendor https://integration.wikimedia.org/ci/job/mwext-php70-phan-docker/27859/consoleFull

00:00:22.777 INFO:quibble.cmd:Projects: mediawiki/core, mediawiki/skins/Vector, mediawiki/vendor, mediawiki/extensions/EntitySchema
00:00:47.071 INFO:quibble.cmd:vendor.git used. Requiring composer dev dependencies
00:00:52.598  Extracting archive  - Installing jakub-onderka/php-parallel-lint (v0.9.2): Loading from cache

00:01:43.785 INFO:test.commands:cd "$THING_SUBNAME" && composer update --ansi --no-progress --prefer-dist --profile

00:01:45.841 [181.4MB/1.91s]   - Installing jakub-onderka/php-parallel-lint (v1.0.0): [181.6MB/1.92s] Loading from cache[181.6MB/1.92s]

Both have PHAN_VERSION=1.2.6 and mediawiki/mediawiki-phan-config 0.5.0.

So I dont know :-/

In T223397#5185404 I have missed the fact that dockerfiles/ci-src-setup/setup-mwext.sh ran composer update from mediawiki/core with --no-dev whereas Quibble runs without the parameter and thus default to install mediawiki/core development dependencies.

Change 511447 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] phan: do not install mediawiki dev dependencies

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

hashar claimed this task.May 20 2019, 3:19 PM
hashar triaged this task as Normal priority.

Change 511448 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/extensions/Echo@master] Revert "Make phan ignore JsonSerializable redefinition"

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

The workaround was to do:

// @phan-suppress-next-line PhanRedefinedInheritedInterface
abstract class EchoEventPresentationModel implements JsonSerializable {

Done by https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Echo/+/508702/

I reverted it, deployed a Jenkins job based on https://gerrit.wikimedia.org/r/511447 and the new job pass just fine :]

Change 511449 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/EventLogging@master] build: Remove PhanRedefinedInheritedInterface suppression

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

I will update the job configuration on Tuesday. Then I guess we have to remove the various @phan-suppress-next-line PhanRedefinedInheritedInterface from the code.

I have spotted: Echo, EventLogging and GrowthExperiments all due to JsonSerializable.

For EventLogging I have uploaded a patch set

For GrowthExperiments you can revert https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/GrowthExperiments/+/507994/

Thank you @Umherirrender . I will do the update tomorrow :]

Mentioned in SAL (#wikimedia-releng) [2019-05-21T08:24:36Z] <hashar> Updated phan jobs to no more install mediawiki development dependencies (potentially conflicting with the extensions code) https://gerrit.wikimedia.org/r/#/c/integration/config/+/511447/ # T223397

Change 511447 merged by jenkins-bot:
[integration/config@master] phan: do not install mediawiki dev dependencies

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

Change 511646 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/extensions/GrowthExperiments@master] Revert "Fix phan job: ignore line using JsonSerializable"

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

Change 511449 merged by jenkins-bot:
[mediawiki/extensions/EventLogging@master] build: Remove PhanRedefinedInheritedInterface suppression

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

Change 511448 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Revert "Make phan ignore JsonSerializable redefinition"

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

Change 511646 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Revert "Fix phan job: ignore line using JsonSerializable"

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

Change 512950 had a related patch set uploaded (by Sbisson; owner: Hashar):
[mediawiki/extensions/GrowthExperiments@wmf/1.34.0-wmf.6] Revert "Fix phan job: ignore line using JsonSerializable"

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

Change 512950 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@wmf/1.34.0-wmf.6] Revert "Fix phan job: ignore line using JsonSerializable"

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