drupal-composer / drupal-scaffold

:construction: Composer Plugin for updating the Drupal scaffold files when using drupal/core
190 stars 46 forks source link

Error in post install script #79

Closed CaptainQuirk closed 6 years ago

CaptainQuirk commented 6 years ago

:bulb: Context

:ballot_box_with_check: Process

:thumbsdown: Actual result

Installation runs smoothly but crashes after generating autoload files with message

Fatal error: Uncaught TypeError: Argument 3 passed to DrupalComposer\DrupalScaffold\PrestissimoFileFetcher::__construct() must implement interface Composer\IO\IOInterface, array given

:thumbsup: Expected result

:smiling_imp: A wholly successful installation !

webflo commented 6 years ago

I looked at the code that calls PrestissimoFileFetcher, but I couldn't find a bug. The callstack/backtrace would be helpful. Thanks!

labboy0276 commented 6 years ago

I am seeing a similar error as well after updating to 2.5.0

PHP Fatal error:  Uncaught TypeError: Argument 3 passed to DrupalComposer\DrupalScaffold\PrestissimoFileFetcher::__construct() must implement interface Composer\IO\IOInterface, array given, called in /app/vendor/drupal-composer/drupal-scaffold/src/Handler.php on line 117 and defined in /app/vendor/drupal-composer/drupal-scaffold/src/PrestissimoFileFetcher.php:25
Stack trace:
#0 /app/vendor/drupal-composer/drupal-scaffold/src/Handler.php(117): DrupalComposer\DrupalScaffold\PrestissimoFileFetcher->__construct(Object(Composer\Util\RemoteFilesystem), 'http://cgit.dru...', Array, Object(Composer\IO\ConsoleIO), Object(Composer\Config))
#1 /app/vendor/drupal-composer/drupal-scaffold/src/Handler.php(92): DrupalComposer\DrupalScaffold\Handler->downloadScaffold()
#2 /app/vendor/drupal-composer/drupal-scaffold/src/Plugin.php(66): DrupalComposer\DrupalScaffold\Handler->onPostCmdEvent(Object(Composer\Script\Event))
#3 [internal function]: DrupalComposer\DrupalScaffold\Plugin->postCmd(Object(Composer\Script\Event))
#4 phar:///usr/local/bin/ in /app/vendor/drupal-composer/drupal-scaffold/src/PrestissimoFileFetcher.php on line 25
johandenhollander commented 6 years ago

Same here.

webflo commented 6 years ago

But it happens only once? I think, the plugin class has been loaded before the package update.

johandenhollander commented 6 years ago

No it happens all the time. I locked the version to 2.4.0 now the composer install goes without errors.

mikran commented 6 years ago

Same thing here, all build scripts are failing to this error with 2.5.0.

webflo commented 6 years ago

I think this related to the update path from 2.4.0 to 2.5.0 only. Once your install is on 2.5.0 this should not happen anymore. Can anyone confirm that?

webflo commented 6 years ago

composer require --no-plugins --no-scripts drupal-composer/drupal-scaffold:^2.5.0 should work without errors.

webflo commented 6 years ago

Related issue https://github.com/composer/composer/issues/6408#issuecomment-300128504

mikran commented 6 years ago

Once your install is on 2.5.0 this should not happen anymore. Can anyone confirm that?

Yes, retry after failure works.

ebeyrent commented 6 years ago

I ran composer require --no-plugins --no-scripts drupal-composer/drupal-scaffold:^2.5.0, committed composer.json and composer.lock to my repo. However, I'm getting these exceptions still in CircleCI. Any thoughts?

webflo commented 6 years ago

@ebeyrent could clear the CircleCI caches? Or add rm -rf vendor/drupal-composer/drupal-scaffold before running composer install. You can remove again once all your builds use drupal-composer/drupal-scaffold 2.5.0

Sorry i don`t have a better workaround atm. Fixing the update path is hard.

potterme commented 6 years ago

Since my related issue was closed, can somebody summarize what the specific fix or workaround for this issue is? I didn't get any of the error messages shown above and I was building a fresh new Drupal 8 site. I wasn't even aware of 2.5.0 until I couldn't get "composer drupal-scaffold" to work and then compared my composer.lock with a different working site which was on 2.4.0. When I reverted back to 2.4.0 then it worked.

A list of the specific composer commands needed to get a site working with 2.5.0 would be appreciated. I hate to pin all of our sites to 2.4.0.

mlncn commented 6 years ago

Same error happening to a bunch of folks:

  [ErrorException]                                                                              
  Argument 3 passed to DrupalComposer\DrupalScaffold\PrestissimoFileFetcher::__construct() mus  
  t implement interface Composer\IO\IOInterface, array given, called in /vagrant/vendor/drupal  
  -composer/drupal-scaffold/src/Handler.php on line 117 and defined                             

Repeated runs of composer update fixed it for me and others there.

mlncn commented 6 years ago

OK, the fact that i upgraded drupal/core (8.5.1 => 8.5.4) with no scaffolding files changing means that the scaffolding script never ran; subsequent runs of composer update just didn't trigger it because Drupal core had already been updated.

So in addition to this bug needing to be fixed... any advice for running scaffolding updates retroactively?

webflo commented 6 years ago

@mlncn run "composer drupal:scaffold"

mlncn commented 6 years ago

Thanks @webflo ! That's better than the workaround i was coming to report— adding a patch to Drupal core causes it to be uninstalled and reinstalled, and the scaffolding ran successfully (no error).

Running composer drupal:scaffold worked also. So this is a temporary / easily-worked-around error. Thank you!

greg-1-anderson commented 6 years ago

I briefly looked into fixing this to make things easier on folks who are upgrading from 2.4.0. However, putting the constructor back the way it was would break everyone who already updated to 2.5.0+

If I understand correctly, the problem here is that Drupal Scaffold is causing dependency hell with itself, because it doesn't load some of its classes until after it updates itself. The result is that Drupal Scaffold is extremely sensitive / fragile to API changes in these classes.

@webflo What do you think about the idea of pre-loading all of Drupal Scaffold's classes immediately prior to doing an update? We could maybe look at the data structures Composer maintains (maybe not good, as these are internal, and I'm not sure there's an API), or perhaps we could simply walk the filesystem at Drupal Scaffold's src directory and infer namespace / classname from the filepath -- or better yet, skip the autoloader and include or require the files ourselves. If we did this, then Drupal Scaffold would continue to use all of its old code until the update was finished.

This would not help 2.4.0 users, but it would prevent this from happening again. If you think this precaution is worthwhile, I'll work on a PR.

greg-1-anderson commented 6 years ago

I guess it's not Drupal Scaffold that's causing this problem; it's Composer that is deciding to load some Drupal Scaffold classes prior to the update, and then continues to call the already-loaded code after updating self-same extension. I guess the fault here is really in Composer. I'd suggest that it be fixed there; however, Composer can't really know what classes a plugin might need post update, and it would be ridiculous to load everything in the vendor directory any time composer update ran in the presence of a plugin.

Maybe we could assume, though, that this sort of upgrade problem only ever happens with internal APIs, and SemVer will protect any cross-project API calls that a plugin might make. If that's the case, then maybe this could be solved in Composer, by having composer update pre-load all of the class files for any plugin that installs any hooks that run after the update. (Or just pre-load all plugin classes.)

Thoughts?

greg-1-anderson commented 6 years ago

According to https://github.com/composer/composer/issues/7298#issuecomment-393796348, composer plugins are not supposed to use the autoloader.

We could therefore fix this in Drupal Scaffold as suggested in https://github.com/drupal-composer/drupal-scaffold/issues/79#issuecomment-395499318.

webflo commented 6 years ago

@greg-1-anderson Thanks for your help. Much appreciated.

The idea in https://github.com/drupal-composer/drupal-scaffold/issues/79#issuecomment-395499318 for not using the autoloader and rely on includes is unusual, but the right thing to do here.

I thought about the opposite direction, delaying the execution until its updated. But i head no luck with this approach.

Please submit a PR. Thanks!

greg-1-anderson commented 6 years ago

@webflo I was thinking about this some more, and remembered that one of the early implementations of Drupal Scaffold used a RoboFile to define the scaffold commands. If we went back to this model, then the various Composer plugin hooks could do nothing but exec the appropriate scripts that would do the actual operations. Each script would then autoload only with the target application's autoloader, and would have no cross-talk with Composer's vendor directory at all.

This seems more standard to me than avoiding the autoloader, which could STILL cause us to run into problems if we use any third party libraries. At the moment, we use only composer/semver, but even this could potentially cause Drupal Scaffold to blow up if Composer updates to a newer / internal-API-incompatible version of composer/semver. That one might be pretty safe, though, as it might be possible that any invocation of their API might load all of their classes. We can't guarentee that, though, and if Composer ever called us with a partially-loaded composer/semver, then we wouldn't be able to guarentee consistency.

So, what do you think? We could also use exec to simply call php drupalscaffold.php if you wanted to minimize dependencies.

alphex commented 6 years ago

Just a note : composer drupal:scaffold overwrites any changes you may have made to sites/development.services.yml (as it downloads a bunch of files for your project.

- .gitattributes (https://raw.githubusercontent.com/pantheon-systems/drops-8-scaffolding/8.5.4/.gitattributes): Downloading (100%)
- .ht.router.php (https://raw.githubusercontent.com/pantheon-systems/drops-8-scaffolding/8.5.4/.ht.router.php): Downloading (100%)
- index.php (https://raw.githubusercontent.com/pantheon-systems/drops-8-scaffolding/8.5.4/index.php): Downloading (100%)    - robots.txt (https://raw.githubusercontent.com/pantheon-systems/drops-8-scaffolding/8.5.4/robots.txt): Downloading (100%)
- sites/default/default.services.yml (https://raw.githubusercontent.com/pantheon-systems/drops-8-scaffolding/8.5.4/sites/default/default.services.yml): Downloading (100%)
- sites/default/default.settings.php (https://raw.githubusercontent.com/pantheon-systems/drops-8-scaffolding/8.5.4/sites/default/default.settings.php): Downloading (100%)
- sites/development.services.yml (https://raw.githubusercontent.com/pantheon-systems/drops-8-scaffolding/8.5.4/sites/development.services.yml): Downloading (100%)
- sites/example.settings.local.php (https://raw.githubusercontent.com/pantheon-systems/drops-8-scaffolding/8.5.4/sites/example.settings.local.php): Downloading (100%)
- sites/example.sites.php (https://raw.githubusercontent.com/pantheon-systems/drops-8-scaffolding/8.5.4/sites/example.sites.php):Downloading (100%)         )
- update.php (https://raw.githubusercontent.com/pantheon-systems/drops-8-scaffolding/8.5.4/update.php): Downloading (100%)
- sites/default/default.services.pantheon.preproduction.yml (https://raw.githubusercontent.com/pantheon-systems/drops-8-scaffolding/8.5.4/sites/default/default.services.pantheon.preproduction.yml): Downloading (100%) .
- sites/default/settings.pantheon.php (https://raw.githubusercontent.com/pantheon-systems/drops-8-scaffolding/8.5.4/sites/default/settings.pantheon.php): Downloading (100%)
greg-1-anderson commented 6 years ago

This issue is not fixed for folks who are still on 2.4.x, but it is fixed by #82 for those users who successfully used a workaround to get to 2.5.0 or later.

greg-1-anderson commented 6 years ago

@alphex It seems to me that it would be a good idea to move some of the files to the "initial files" list:

      'sites/default/default.settings.php',
      'sites/default/default.services.yml',
      'sites/development.services.yml',
      'sites/example.settings.local.php',
      'sites/example.sites.php',
Getekid commented 6 years ago

For anyone that might get here, another way to get past this error is to first update scaffold and then anything else you might want, i.e. composer update drupal-composer/drupal-scaffold --with-dependencies composer update

This way everything went smoothly and after the update the scaffold run as usual.

lauriskuznecovs commented 6 years ago

For anyone that might get here, another way to get past this error is to first update scaffold and then anything else you might want, i.e. composer update drupal-composer/drupal-scaffold --with-dependencies composer update

This way everything went smoothly and after the update the scaffold run as usual.

Thanks, it worked.