Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal for adding a composer update lookahead to resolve root conflicts #18

Merged

Conversation

pdohogne-magento
Copy link
Contributor

With Magento 2.3, we needed to add additional steps to the upgrade process due to incompatibilities with dependencies in the root composer.json file. Without the fixed requirements, the composer update command fails to install the new version of Magento because it has conflicts with the root requirements, and 2.3 code can't fix those requirements before it is installed because it would need to be installed in order to run the code.

In the future, we are likely to encounter this issue again, especially during upgrades where we drop support for old PHP versions (PHP 7.1 will soon stop receiving security patches, for example). To avoid needing to add more manual steps for each version, we should add hooks to the upgrade process so any installations that start at 2.3 or higher check the target version for changes that need to be made before it is installed.

See the changed file for the proposed solution.

@pdohogne-magento
Copy link
Contributor Author

Review requested: @buskamuza

@buskamuza buskamuza self-requested a review August 17, 2018 15:36
@pdohogne-magento
Copy link
Contributor Author

This is all possible using a Composer plugin; I have a proof-of-concept attaching custom parameters and composer modification functionality to the update command running locally.

Question: Is it necessary to address non-composer.json changes? The decision to include magento/updater in the 2.3 upgrade script was not a requirement, and I'm not sure trying to include that functionality in this plugin would make sense (and it would make the implementation much more complicated than handling just composer.json itself). Non-composer changes shouldn't break any upgrades unless they are in autoloaded files, and in our current project packages the only non-composer.json content we have is magento/updater, which isn't autoloaded.

@pdohogne-magento
Copy link
Contributor Author

I think this should live in its own git repo and be versioned independently, as it should have no problems being compatible with earlier Magento versions, plus it's not an actual Magento module. Do you see any issue with that? By necessity it doesn't have any actual composer requirements on any other Magento packages.

Copy link
Contributor

@buskamuza buskamuza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposal looks good in general.

Please include details about being installed as Composer plugin. Or how else a user can skip this tool all together.

@@ -0,0 +1,48 @@
# Add Lookahead for Pre-`composer update` Changes
> _Related JIRA ticket: [MAGETWO-94153](https://jira.corp.magento.com/browse/MAGETWO-94153)_
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't reference internal resources here as they're not accessible for outside people.

# Add Lookahead for Pre-`composer update` Changes
> _Related JIRA ticket: [MAGETWO-94153](https://jira.corp.magento.com/browse/MAGETWO-94153)_
## Context
With Magento 2.3, we needed to [add additional steps to the upgrade process](https://devdocs.magento.com/guides/v2.3/comp-mgr/cli/cli-upgrade.html) due to incompatibilities with dependencies in the root `composer.json` file. Without the fixed requirements, the `composer update` command fails to install the new version of Magento because it has conflicts with the root requirements, and 2.3 code can't fix those requirements before it is installed because it would need to be installed in order to run the code.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please include link to the specific section?

## Implementation Strategy
There are a few hiccups in the actual implementation.
- The upgrade needs to be able to handle starting at any previous Magento version, each of which could have different values that need to be added, changed, or removed.
- There are potentially non-Composer updates that need to be made, such as `magento/updater`, which exists in the top-level Magento directory but is not its own composer package.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, take into account a case when Updater application is absent.

@buskamuza buskamuza requested a review from szurek August 20, 2018 15:22
@buskamuza buskamuza merged commit 3ec4dc8 into magento:master Aug 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants