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

Bad constraint "symfony/yaml": "^2.3||^3.3" #404

Conversation

@vkerkhoff
Copy link
Contributor

commented Jan 7, 2019

Bad constraint "symfony/yaml": "^2.3||^3.3"

Description

vendor/magento/ece-tools/src/Config/Environment/Reader.php on line 58 uses Yaml::PARSE_CONSTANT, but this constant is only introduced in Symphony Yaml v3.2
With the current constraints, composer allows us to run Yaml v2.8 due to a different module requiring "^2.0", but the build breaks on vendor/bin/m2-ece-build

Solutions it to remove ^2.3 from the ece-tools constraint.

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)

vkerkhoff and others added some commits Dec 12, 2018

Bad constraint `"symfony/yaml": "^2.3||^3.3"`
vendor/magento/ece-tools/src/Config/Environment/Reader.php on line 58 uses Yaml::PARSE_CONSTANT, but this constant is only introduced in Symphony Yaml v3.2
With the current constraints, composer allows us to run Yaml v2.8 due to a different module requiring "^2.0", but the build breaks on `vendor/bin/m2-ece-build`

Solutions it to remove ^2.3 from the ece-tools constraint.

@vkerkhoff vkerkhoff changed the title Feature/wrong composer version symfony yaml Bad constraint "symfony/yaml": "^2.3||^3.3" Jan 7, 2019

@shiftedreality

This comment has been minimized.

Copy link
Member

commented Jan 7, 2019

@vkerkhoff Thanks for your contribution!

Unfortunately, this change will break compatibility with n98-magerun2 utility https://github.com/netz98/n98-magerun2/blob/develop/composer.json#L33

@vkerkhoff

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2019

But currently, it breaks the build if it is installed with symfony/yaml <3.2, I would rather receive an error message during composer install that it has a conflict. Also, most of the time you will install the phar of magerun2 and not include it in your project, so that won't cause a conflict then. And most important, ece-tools isn't compatible with symfony/yaml <3.2 so it shouldn't be listed as a dependency.

@YPyltiai

This comment has been minimized.

Copy link
Contributor

commented Jan 8, 2019

@vkerkhoff actually #389 should have fixed this. It will be in .16 release (.15 now has the bug mentioned).

@shiftedreality

This comment has been minimized.

Copy link
Member

commented Jan 10, 2019

@vkerkhoff please check Yevhenii's comment. Is it still actual after #389 was delivered?

@vkerkhoff

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2019

@shiftedreality I guess it solves the problem, but not sure if this is really a clean solution as it can give unpredictive results (with >3.2 installed it might give a different result compared to <3.2). I think that just removing the 2.3 dependencies would be better as my previous comment.

@YPyltiai

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2019

@cmuench , would it be possible to add ^3.3 support to https://github.com/netz98/n98-magerun2 ? I was pinging you on Slack previously but didn't remember getting a response.

@YPyltiai

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2019

Opened an issue: netz98/n98-magerun2#430

@YPyltiai

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2019

Based on the information from netz98/n98-magerun2#430 - we further agree that this utility should be used in form of phar as per official recommendations. Outdated symfony/yaml version can be removed from ECE-Tools dependencies.

@shiftedreality shiftedreality added approved and removed onhold review labels Jan 23, 2019

@shiftedreality shiftedreality merged commit a035029 into magento:develop Jan 23, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
licence/cla Contributor License Agreement is signed.
Details
@contribution-survey

This comment has been minimized.

Copy link

commented Jan 23, 2019

Hi @vkerkhoff, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.