Move enlightn/security-checker to "suggest" #5078
Conversation
|
Looks like the one failing test is caused by updating dependencies. The test passes locally with the composer.lock from the 11.x branch, but fails after running |
|
Failing test is caused by: This combination breaks expansion of |
|
Failing test fixed by grasmash/expander#13 |
|
Most tests green with the update to grasmash/expander 2.0.1. Now, it looks like the latest Drush 10.0.x dev requires symfony/process ^6, and consolidation/site-process supports ^5 at the most. Will need some more releases on consolidation/*, it looks like. |
|
I made a 5.x-dev branch of consolidation/site-process to bring in symfony/process ^6. I think this should work, but now we need to upgrade league/container to ^4, because that version series is necessary to be compatible with psr/container ^2, which Drupal 10 now requires. |
|
Now we need a new Robo release that allows symfony/process ^6. |
greg-1-anderson
force-pushed
the
suggest-security-checker
branch
from
bb71fc9
to
0931c51
Compare
|
The Composer dependencies resolve with Drupal 10 now; however, it seems that there have been some changes in the Drupal bootstrap to adjust to Symfony 6; similar changes will be needed in Drush. The error we are getting now is: |
|
Related drupal.org Symfony 6 issue: https://www.drupal.org/project/drupal/issues/3252757 |
|
Now it looks like we're just down to the |
…al 9.4, and will be removed in Drupal 10.0.0. See https://www.drupal.org/node/3220952 for more information. This commit implements a call to ModuleHandler::loadInclude() instead.
…ecker tests on Drupal 10.
…placement update hook registry service is available.
|
Getting really close. The integration tests have several errors. CircleCI does not report anything; running locally, I get: Not sure why it's running out of memory; infinite recursion or something? Needs further investigation. Functional and unit tests are all passing. |
tests/functional/UpdateDBTest.php
Outdated
| @@ -70,7 +77,7 @@ public function testFailedUpdate($last_successful_update, $expected_status_repor | |||
| $this->drush('php-script', ['updatedb_script'], ['script-path' => __DIR__ . '/resources']); | |||
|
|
|||
| // Force re-run of woot_update_8101(). | |||
| $this->drush('php:eval', array('drupal_set_installed_schema_version("woot", ' . $last_successful_update . ')'), $options); | |||
| $this->drush('php:eval', array('\Drupal::service("update.update_hook_registry")->setInstalledVersion("woot", ' . $last_successful_update . ')'), $options); | |||
There was a problem hiding this comment.
Does anyone know what's up with backslashes and Windows PHP? This line works in Circle, but fails in Appveyor.
There was a problem hiding this comment.
@greg-1-anderson just remove the leading \ from '\Drupal::service(...)
There was a problem hiding this comment.
That might help. I think that the issue might be that backslashes are getting doubled up in a place where that's not necessary. But I'm in favor of avoidance at this stage. 😄
|
Hey, much better -- Appvayor is green, and we're down to a dozen integration test failures. I didn't realize there was going to be so much to fix here. |
|
Down to just some MigrateRunnerTest failures in the integration tests. |
|
Migrate tests have been failing for a while in d10. Feel free to merge without them. See consolidation/site-process#56 for windows bug. |
|
Great, thanks. This will be better for Drupal 10 users, but Drupal 9 users lose enlightn/security-checker. Maybe that package will become compatible & we can add it back in before the next release. Or maybe temporarily add it in just for the release tag, and then take it out again so that Drupal 10 users can continue using the dev release of Drush. |
|
@greg-1-anderson @weitzman Symfony 6 is now supported via enlightn/security-checker#27, so can we revert this PR? |
|
We'll need a new PR to re-add enlightn/security-checker; this PR fixed quite a few other issues at the same time, so we shouldn't do a literal revert. Re-enabling the security checks will be easy, though. |
|
Partial revert at #5081 |
We can see how this works vis-a-vis tests. Not a great solution, but might be better than some of the alternatives.