Problem/Motivation

I think if you install a meta package with our fixture manipulator and then call \Drupal\package_manager\ComposerInspector::getInstalledPackagesList the InstalledPackage object will the project root as the path. It should probably be null.

this is probably a problem with getInstalledPackagesList but it could be the FixtureManipulator.

We can manually test by pointing getInstalledPackagesList at a real composer project with metapackage.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#8 Screenshot 2023-03-14 at 5.50.45 PM.png303.49 KByash.rode

Issue fork automatic_updates-3345646

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tedbow created an issue. See original summary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: InstalledPackage object for packages with no source like meta-package types should be NULL » InstalledPackage object for packages with no source like meta-package types should have `install_path === NULL`

this is probably a problem with getInstalledPackagesList but it could be the FixtureManipulator.

I'm pretty sure it's the former based on the observations I had while working on #3343827: Update FixtureManipulator to work with InstalledPackagesList, real composer show command, but of course we'll need to confirm 👍

Is this really core-mvp though?

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Issue tags: -sprint, -core-mvp +core-post-mvp

yes you are right core-post-mvp

yash.rode made their first commit to this issue’s fork.

yash.rode’s picture

yash.rode CreditAttribution: yash.rode at Acquia commented
Assigned: Unassigned » yash.rode
yash.rode’s picture

yash.rode CreditAttribution: yash.rode at Acquia commented
Status: Active » Needs work

yash.rode opened merge request !781

yash.rode’s picture

yash.rode CreditAttribution: yash.rode at Acquia commented
FileSize
Screenshot 2023-03-14 at 5.50.45 PM.png303.49 KB

I can confirm that it is not the FixtureManipulator which is causing this problem as you can see in the screen-shot
passing. So it is composer show() which is causing the problem in getInstalledPackagesList(). I tried debugging it and created a git clone of composer and added a var_dump() in comopser-testing/composer/src/Composer/Command/ShowCommand.php
when $input->getOption('path') is true but I am not able to create a meta package in that git clone.

yash.rode’s picture

yash.rode CreditAttribution: yash.rode at Acquia commented
Issue tags: +sprint
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

but I am not able to create a meta package in that git clone.

  1. The git clone you mention is of Composer itself, so of course you cannot make a meta package inside that git clone?! 🤔
  2. Why can't you create an meta package just like the one that that test created, whose Composer commands you can observe exactly and repeat in any other place? 🤔 That way you'd be able to figure this out completely independently of Package Manager's test suite!
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Issue tags: -core-post-mvp +core-mvp

phenaproxima made their first commit to this issue’s fork.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Assigned: yash.rode » phenaproxima
phenaproxima’s picture

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

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

Looks pretty good but a couple MR comments

phenaproxima’s picture

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

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Title: InstalledPackage object for packages with no source like meta-package types should have `install_path === NULL` » InstalledPackage::$path for metapackages should be NULL
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Assigned: tedbow » phenaproxima
Status: Needs review » Needs work
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Assigned: phenaproxima » Wim Leers
Status: Needs work » Needs review

🏓

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: InstalledPackage::$path for metapackages should be NULL » InstalledPackage::$path for metapackages should be NULL
Assigned: Wim Leers » phenaproxima
Issue tags: +Needs followup

If the necessary follow-ups existed, this would be RTBC. We need both an upstream Composer issue filed and an issue in our issue queue.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Assigned: phenaproxima » tedbow
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs followup

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Reviewed & tested by the community » Fixed
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Assigned: tedbow » Unassigned

Status: Fixed » Closed (fixed)

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