acquia / lightning-project

A Composer-based installer for the Lightning distribution of Drupal 8. Support ended on November 2, 2021 and this project is no longer maintained.
133 stars 60 forks source link

Switch from drupal-composer/drupal-scaffold to drupal/core-composer-scaffold #112

Closed balsama closed 4 years ago

balsama commented 4 years ago

drupal-composer/drupal-scaffold has been deprecated in favor of drupal/core-composer-scaffold. This PR replaces the old with the new.

Since the Travis CI config triggers the scaffold plugin twice (once during the first install, then again on an update) it hits Issue #3091285. Line 50 of .travis.yml works around that bug.

phenaproxima commented 4 years ago

Looks great! Merge when green. :)

danepowell commented 4 years ago

BLT also hit the permission hardening issue. We worked around it using the file-mapping key to exclude the affected files from updates: https://github.com/acquia/blt-project/blob/11.x/composer.json#L42

Users of Lightning will hit that bug and probably not know what's going on. A Travis CI workaround won't help them :smile:

balsama commented 4 years ago

True. It's a trivial error with a documented workaround: https://www.drupal.org/docs/develop/using-composer/starting-a-site-using-drupal-composer-project-templates#s-troubleshooting-permission-issues-prevent-running-composer

Agree that it's likely to annoy people though. I thought that adopting the new scaffold was important and didn't want to adopt another core patch to fix it (especially when the implementation details are still being debated). So here we are. We could write a blog explaining that, but it would just compete SEO-wise with the documented workaround.

Does anyone have suggestions?

phenaproxima commented 4 years ago

My preference is to revert this for now until the resolution is either RTBC or committed.

danepowell commented 4 years ago

You don't need a core patch to work around this in Lightning Project. You can have your cake and eat it too! 😄

Just exclude those two files from updates using file mapping like I mentioned above: https://github.com/acquia/blt-project/blob/11.x/composer.json#L42

The only downside to this approach is that downstream users won't get automatic updates to those files, but they can fix that by just removing the file mappings once the upstream issue is resolved. I think that's a decent compromise, but it's up to you.