Problem/Motivation

Updater uses Stage::getActiveComposer(), which returns a ComposerUtility. #3316368: Remove our runtime dependency on composer/composer: remove ComposerUtility is removing that.

Proposed resolution

Refactor it to use ComposerInspector instead.

Remaining tasks

Do it!

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork automatic_updates-3345754

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Wim Leers created an issue. See original summary.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Related issues: +#3345771: VersionPolicyValidator should use ComposerInspector instead of ComposerUtility

This blocks #3345771: VersionPolicyValidator should use ComposerInspector instead of ComposerUtility

yash.rode’s picture

yash.rode commented
Assigned: Unassigned » yash.rode
Status: Active » Needs work

yash.rode opened merge request !760

yash.rode’s picture

yash.rode commented
Assigned: yash.rode » Unassigned

Starting with package_manager issue for now.

yash.rode’s picture

yash.rode commented
Assigned: Unassigned » yash.rode
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
  1. Needs work for phpcs failures.
  2. Reviewed anyway so we can get this to RTBC ASAP 😊
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Looks great, but tests are not yet passing!

yash.rode’s picture

yash.rode commented

Tests are passing on local, It's just the PHP temp directory (/tmp) does not exist or is not writable to Composer error on DrupalCI, running tests again.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

#9: that's not what I see on DrupalCI — I see this:


Drupal\Tests\package_manager\Kernel\ComposerPatchesValidatorTest
fail: [Other] Line 0 of sites/default/files/simpletest/phpunit-52.xml:
PHPUnit Test failed to complete; Error: PHPUnit 9.5.28 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\package_manager\Kernel\ComposerPatchesValidatorTest
...F............                                                  16 / 16 (100%)

Time: 01:41.117, Memory: 6.00 MB

There was 1 failure:

1) Drupal\Tests\package_manager\Kernel\ComposerPatchesValidatorTest::testErrorDuringPreCreate with data set "VALID: absent" (0, array())
Failed asserting that an array is not empty.
yash.rode’s picture

yash.rode commented
Assigned: yash.rode » wim leers
Status: Needs work » Needs review

The tests are passing now. The test mentioned in #10 was also passing on my local.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » Unassigned
Status: Needs review » Reviewed & tested by the community

🤯

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Reviewed & tested by the community » Needs work

Some relatively minor stuff. I'm not sure about naming the new property of InstalledPackage $isDevRequirement, because these aren't requirements per se.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » phenaproxima

because these aren't requirements per se

https://getcomposer.org/doc/04-schema.md#require-dev literally uses this terminology, that's why I suggested that instead of the is_dev that was there before.

But I guess you're worried about how lengthy it is?

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Assigned: phenaproxima » tedbow
Status: Needs work » Needs review

This is looking promising. Over to @tedbow for review...

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » phenaproxima
Status: Needs review » Needs work

see MR

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Assigned: phenaproxima » tedbow
Status: Needs work » Needs review
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Needs review » Needs work
Issue tags: +Needs change record

Looks good but needs a Change Record. Should be made as draft for now

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
Issue tags: -Needs change record

Done.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » Unassigned
Status: Needs review » Reviewed & tested by the community

🎉

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.