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

Drush9 class mismatch with drupal #3254

Closed
weitzman opened this issue Dec 17, 2017 · 5 comments
Closed

Drush9 class mismatch with drupal #3254

weitzman opened this issue Dec 17, 2017 · 5 comments

Comments

@weitzman
Copy link
Member

weitzman commented Dec 17, 2017

I'm seeing another case where a global Drush gets an error, seemingly due to a class mismatch with Drupal core.

In your global Drush, composer update (if needed) to get to symfony/yaml 3.4. Then run drush cr using the global Drush and a naked Drupal 8 (no site local Drush). I'm seeing [error] Unable to parse at line 130 (near "{}"). which is a parse error here. Note the unusual indendentation+braces syntax in that line.

If you aren't getting the error, copy the whole default.services.yml to services.yml and rerun drush cr.

The error goes away if I downgrade to symfony/yaml:3.2 in my global Drush.

@weitzman
Copy link
Member Author

Also, make sure you dont have PECL yaml as that works around symfony/yaml AFAIK

@greg-1-anderson
Copy link
Member

Yeah, symfony/yaml is one of the packages that Drush needs early on. This leaves us vulnerable to class mismatch errors in symfony/yaml if the global Drush uses a different symfony/yaml than the global Drupal. I'm not sure if the error you are seeing is being caused by a transient error in symfony/yaml or a class mismatch error.

It seems that perhaps the safest thing to do would be to use a different yaml parser during the Drush preflight. However, this decision would impact consolidation/config, yaml/expander, etc., so I don't know that it is a practical measure we could contemplate.

@weitzman
Copy link
Member Author

weitzman commented Dec 19, 2017

This is indeed a problem - https://www.drupal.org/project/drupal/issues/2927746#comment-12390037. I wonder if we should pin our symfony/yaml so it can't exceed 3.3 (for now). Not sure if we use a Conflicts statement or just a constraint for that.

@greg-1-anderson
Copy link
Member

The short-term solution is to add another require statement for symfony/yaml in the require-dev section. This will pin the version of symfony/yaml for the global Drush, but will allow site-local Drush to float to whatever version of the package is being used by Drupal.

This is a fine short-term solution. We are going to run up on the reefs when Drupal core starts requiring Symfony 3.4, as we at that time will not be able to produce a global Drush that works with both the newer and the older Drupals. I thought we'd have a long run before we hit this problem, as I expected it wouldn't happen until Symfony 4, which Drupal won't be able to adopt for a long time due to its minimum PHP requirement of 7.1. The fact that symfony/yaml 3.4 breaks global Drush is a problem, since Drupal 8.5 is already working on adopting that version.

Perhaps what we should do is make consolidation/config use its own private yaml parser, and copy @grasmash's yaml-expander inside of it so that we don't have to worry about additional external dependencies being affected by the need for a custom yaml parser. We'd use symfony/yaml for everything else, and our custom parser only for consolidation/config (Drush configuration and alias files). Our custom yaml parser could be a re-namespaced copy of symfony/yaml, or something else.

@weitzman
Copy link
Member Author

weitzman commented Jan 5, 2018

This was fixed by Drush adding a copy of the Parser from symfony/yaml. Also required changes in config and grasmash projects. See #3269

@weitzman weitzman closed this as completed Jan 5, 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

No branches or pull requests

2 participants