Fix magento root package identification for metapackage installation #22116

oleksii-lisovyi · 2019-04-02T13:10:04Z

Description (*)

Fix issue with failing Magento installing process (php bin/magento setup:install) for installation via metapackage, because of issue in root package defining.

Fixed Issues (if relevant)

Magento installation via metapackage: checkExtensions fails #21136: Magento installation via metapackage: checkExtensions fails, because isMagentoRoot works incorrectly for installation

Manual testing scenarios (*)

Steps to reproduce could be takes from issue.

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

m2-assistant · 2019-04-02T13:10:05Z

Hi @oleksii-lisovyi. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento-engcom-team give me test instance - deploy test instance based on PR changes
@magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

magento-engcom-team · 2019-04-02T13:29:02Z

Hi @ihor-sviziev, thank you for the review.
ENGCOM-4644 has been created to process this Pull Request

orlangur · 2019-04-02T13:44:09Z

lib/internal/Magento/Framework/Composer/ComposerInformation.php

@@ -148,7 +148,6 @@ public function getRequiredExtensions()
            /** @var CompletePackageInterface $package */
            foreach ($this->getLocker()->getLockedRepository()->getPackages() as $package) {
                $requires = array_keys($package->getRequires());
-                $requires = array_merge($requires, array_keys($package->getDevRequires()));
                $allPlatformReqs = array_merge($allPlatformReqs, $requires);


Why do change this instead of because isMagentoRoot works incorrectly for installation?

It collects all packages from composer.lock file and collects all require and require-dev dependencies. But during composer install - require-dev dependencies are getting from composer.json file only. So fix is correct.

@ihor-sviziev your statement does not answer my question.

The root issue: method $this->isMagentoRoot() returns FALSE for current Magento installation, because its regular expression /magento/magento2...?/ doesn't match magento/project-community-edition, that comes from metapackage.

Quote from the issue. Collected package information must be different depending whether it is a dev Magento installation or Composer-based.

orlangur · 2019-04-02T13:45:45Z

dev/tests/integration/testsuite/Magento/Framework/Composer/ComposerInformationTest.php

@@ -92,7 +92,7 @@ public function testGetRequiredPhpVersion($composerDir)
    public function testGetRequiredExtensions($composerDir)
    {
        $this->setupDirectory($composerDir);
-        $expectedExtensions = ['ctype', 'gd', 'spl', 'dom', 'simplexml', 'mcrypt', 'hash', 'curl', 'iconv', 'intl'];
+        $expectedExtensions = ['ctype', 'gd', 'spl', 'dom', 'simplexml', 'mcrypt', 'hash', 'curl', 'iconv'];


Does not match https://github.com/magento/magento2/blob/2.3-develop/composer.json#L30. Why do we need this test at all? How it works for outdated PHP versions?

@nmalevanec was fixing this integration test in #21137, then this forward port was created from that PR.
I think he fixed it incorrectly. I'll talk with him

ihor-sviziev

Hi,
As we discussed with @nmalevanec - he'll fix tests in correct way internally. And he already pushed some changes for fixing integration tests. I'm approving this PR.

magento-engcom-team · 2019-04-11T17:15:53Z

Hi @ihor-sviziev, thank you for the review.
ENGCOM-4644 has been created to process this Pull Request

orlangur · 2019-04-12T09:28:51Z

lib/internal/Magento/Framework/Composer/ComposerInformation.php

@@ -148,7 +148,6 @@ public function getRequiredExtensions()
            /** @var CompletePackageInterface $package */
            foreach ($this->getLocker()->getLockedRepository()->getPackages() as $package) {
                $requires = array_keys($package->getRequires());
-                $requires = array_merge($requires, array_keys($package->getDevRequires()));
                $allPlatformReqs = array_merge($allPlatformReqs, $requires);


@ihor-sviziev your statement does not answer my question.

The root issue: method $this->isMagentoRoot() returns FALSE for current Magento installation, because its regular expression /magento/magento2...?/ doesn't match magento/project-community-edition, that comes from metapackage.

Quote from the issue. Collected package information must be different depending whether it is a dev Magento installation or Composer-based.

orlangur · 2019-04-15T09:20:20Z

@ihor-sviziev thanks for sorting this out in Slack.

@oleksii-lisovyi please squash changes into a single commit so that we have perfectly clean history 😉

orlangur · 2019-04-24T20:06:12Z

All good here, waiting for @oleksii-lisovyi returning back from vacation 🉑

ihor-sviziev · 2019-05-10T20:26:57Z

Hi @oleksii-lisovyi,
Will you be able to squash changes into single commit?

oleksii-lisovyi · 2019-05-11T19:19:37Z

Hi. I can, but what is the advantage of having single commit comparing to couple?

orlangur

@oleksii-lisovyi we always must strive to as few logically separated commits as possible.

Fix integration test commit does not make sense as separate commit.

Please squash all three commits into one and force push.

engcom-Golf · 2019-06-18T10:55:32Z

@orlangur please review

magento-engcom-team · 2019-06-18T12:02:55Z

Hi @ihor-sviziev, thank you for the review.
ENGCOM-4644 has been created to process this Pull Request
✳️ @ihor-sviziev, could you please add one of the following labels to the Pull Request?

Label	Description
`Auto-Tests: Covered`	All changes in Pull Request is covered by auto-tests
`Auto-Tests: Not Covered`	Changes in Pull Request requires coverage by auto-tests
`Auto-Tests: Not Required`	Changes in Pull Request does not require coverage by auto-tests

engcom-Alfa · 2019-06-19T07:17:15Z

✔️ QA Passed

magento-engcom-team · 2019-06-19T16:44:36Z

Hi @orlangur, thank you for the review.
ENGCOM-4644 has been created to process this Pull Request

m2-assistant · 2019-06-27T14:06:50Z

Hi @oleksii-lisovyi, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

…r metapackage installation #22116

magento-engcom-team added Component: Composer Release Line: 2.3 labels Apr 2, 2019

oleksii-lisovyi mentioned this pull request Apr 2, 2019

[Backport] Fix magento root package identification for metapackage installation #21137

Closed

4 tasks

orlangur self-assigned this Apr 2, 2019

ihor-sviziev approved these changes Apr 2, 2019

View reviewed changes

ghost assigned ihor-sviziev Apr 2, 2019

ghost added the Progress: ready for testing label Apr 2, 2019

orlangur requested changes Apr 2, 2019

View reviewed changes

ghost added Progress: needs update and removed Progress: ready for testing labels Apr 2, 2019

sivaschenko changed the title ~~[Forwardport] Fix magento root package identification for metapackage installation~~ Fix magento root package identification for metapackage installation Apr 3, 2019

ihor-sviziev approved these changes Apr 11, 2019

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Apr 11, 2019

nmalevanec force-pushed the fix-magento-root-package-identification-for-metapackage-installation-2.3 branch 2 times, most recently from 684abc8 to b0a899b Compare April 12, 2019 08:37

magento-engcom-team assigned nmalevanec Apr 12, 2019

orlangur requested changes Apr 12, 2019

View reviewed changes

ghost added Progress: needs update and removed Progress: ready for testing labels Apr 12, 2019

nmalevanec force-pushed the fix-magento-root-package-identification-for-metapackage-installation-2.3 branch from b0a899b to a2ad630 Compare April 15, 2019 12:28

IvanPletnyov self-assigned this May 20, 2019

ihor-sviziev approved these changes May 21, 2019

View reviewed changes

ghost added the Progress: accept label May 24, 2019

orlangur requested changes May 24, 2019

View reviewed changes

ghost added Progress: needs update and removed Progress: accept labels May 24, 2019

Framework: remove checking on dev packages requirements

2c91e5a

ysapiga force-pushed the fix-magento-root-package-identification-for-metapackage-installation-2.3 branch from 1df503d to 2c91e5a Compare June 18, 2019 10:53

ihor-sviziev approved these changes Jun 18, 2019

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Jun 18, 2019

engcom-Alfa assigned engcom-Alfa and unassigned soleksii Jun 18, 2019

ihor-sviziev added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Jun 18, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Jun 19, 2019

ghost added Progress: extended testing and removed Progress: testing in progress labels Jun 19, 2019

engcom-Foxtrot self-assigned this Jun 19, 2019

ghost added Progress: accept and removed Progress: extended testing labels Jun 19, 2019

orlangur approved these changes Jun 19, 2019

View reviewed changes

magento-engcom-team added the Award: bug fix label Jun 20, 2019

magento-engcom-team mentioned this pull request Jun 27, 2019

Magento installation via metapackage: checkExtensions fails #21136

Closed

magento-engcom-team merged commit 2c91e5a into magento:2.3-develop Jun 27, 2019

magento-engcom-team added this to the Release: 2.3.3 milestone Jun 27, 2019

magento-engcom-team pushed a commit that referenced this pull request Jun 27, 2019

ENGCOM-4644: [Forwardport] Fix magento root package identification fo…

4364d04

…r metapackage installation #22116

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix magento root package identification for metapackage installation #22116

Fix magento root package identification for metapackage installation #22116

oleksii-lisovyi commented Apr 2, 2019

m2-assistant bot commented Apr 2, 2019

magento-engcom-team commented Apr 2, 2019

orlangur Apr 2, 2019

ihor-sviziev Apr 10, 2019

orlangur Apr 12, 2019

orlangur Apr 2, 2019

ihor-sviziev Apr 10, 2019

ihor-sviziev left a comment

magento-engcom-team commented Apr 11, 2019

orlangur Apr 12, 2019

orlangur commented Apr 15, 2019

orlangur commented Apr 24, 2019

ihor-sviziev commented May 10, 2019

oleksii-lisovyi commented May 11, 2019

orlangur left a comment •

edited

Loading

engcom-Golf commented Jun 18, 2019

magento-engcom-team commented Jun 18, 2019

engcom-Alfa commented Jun 19, 2019

magento-engcom-team commented Jun 19, 2019

m2-assistant bot commented Jun 27, 2019

Fix magento root package identification for metapackage installation #22116

Fix magento root package identification for metapackage installation #22116

Conversation

oleksii-lisovyi commented Apr 2, 2019

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Contribution checklist (*)

m2-assistant bot commented Apr 2, 2019

magento-engcom-team commented Apr 2, 2019

orlangur Apr 2, 2019

Choose a reason for hiding this comment

ihor-sviziev Apr 10, 2019

Choose a reason for hiding this comment

orlangur Apr 12, 2019

Choose a reason for hiding this comment

orlangur Apr 2, 2019

Choose a reason for hiding this comment

ihor-sviziev Apr 10, 2019

Choose a reason for hiding this comment

ihor-sviziev left a comment

Choose a reason for hiding this comment

magento-engcom-team commented Apr 11, 2019

orlangur Apr 12, 2019

Choose a reason for hiding this comment

orlangur commented Apr 15, 2019

orlangur commented Apr 24, 2019

ihor-sviziev commented May 10, 2019

oleksii-lisovyi commented May 11, 2019

orlangur left a comment • edited Loading

Choose a reason for hiding this comment

engcom-Golf commented Jun 18, 2019

magento-engcom-team commented Jun 18, 2019

engcom-Alfa commented Jun 19, 2019

magento-engcom-team commented Jun 19, 2019

m2-assistant bot commented Jun 27, 2019

orlangur left a comment •

edited

Loading