Check if drupal/core package exists before running scaffolding

[T2L]

[greg-1-anderson]

Thanks for this; however, I think that this PR runs the risk of doing more harm than good. Currently, if there is no core available, then you get a fatal error, and the build breaks on the bench. With this PR, this situation instead silently skips scaffolding, and you get a build that reports success, but that is incomplete and therefore will not work.

Perhaps a better change here would be to detect the fatal error, and then throw an exception with a better description of what is wrong.

[webflo]

Yeah i think @greg-1-anderson is right.

@T2L Are you using drupal-scaffold without drupal/core? Could you explain your idea? Thanks!

[greg-1-anderson]

@webflo I think that @T2L is probably using drupal-scaffold with Pantheon. Currently, our process is to include drupal/core directly, using the version provided by drupal.org; however, we pull our scaffold files from pantheon-systems/drops-8. The problem occurs when a new version of drupal/core comes out; in this instance, Composer will pull in, say, version 8.2.6 of drupal/core, but pantheon-systems/drops-8 is only available through version 8.2.5. In this instance, drupal-scaffold fails.

Perhaps Pantheon should make an exact clone of drupal/core, only for the purpose of delaying the addition of the latest tag until said tag also exists on pantheon-systems/drops-8.

[T2L]

Actually my wordflow was a little bit different. I was working on a composer project template for our company. Accidently I've added dependency on drupal/drupal (instead of drupal/core). composer install has run without any glitch. Then I spotted my mistake and tried to do composer remove drupal/drupal and faced aforementioned issue. I don't think it should break composer in such way.

My vision:

1. We clearly have a bug (uncaught exception). It must be fixed
2. @greg-1-anderson - you are talking about "running a build", but imo composer is not a building tool, but dependency manager. However, some other building tool (maybe Phing) might use composer as a step in a bulding pipeline. Also, I do believe that it's a building tool's responsibility to verify build integrity.
3. I understand why there is no hard dependency on drupal/core (because you want to support different flavours of Drupal core, like Drops)
4. Seems like we need some kind of soft dependency on a Drupal core-compatible code. Obviously, this is not possible with plain composer
5. Throwing an exception when no core-compatible code were found could work, but let's think about responsibility
6. Ideally, we need a hard dependency on core. One possible options is to divide this package:
   1. drupal-composer/drupal-scaffold (no dependecies, as it's now)
   2. drupal-composer/drupal-scaffold-drupal (dependency on drupal-composer/drupal-scaffold and drupal/core)
   3. drupal-composer/drupal-scaffold-drops (dependency on drupal-composer/drupal-scaffold and pantheon-systems/drops-8)
   4. ... do we need more?

[derhasi]

To support a different core package, we could make that configurable in Handler::getCorePackage and Handler::getDrupalCorePackage.

In addition we should improve Plugin::scaffold() to throw an explicit exception or use iO to display an error. We definitely should not fail silently there.

[T2L]

Huh. Code explicitly depends on drupal/core, but the package itself does not. Maybe just and a dependency drupal/core: "*"?

ADDED Even project description states "Composer Plugin for updating the Drupal scaffold files when using drupal/core"

[greg-1-anderson]

The semantics of what composer is are immaterial here; if a tool of any type fails to do it's job, it should return with a non-zero exit code. An inscrutable exception is preferable to a silent failure, and a rational error message is better than an inscrutable exception.

Related issue: https://github.com/pantheon-systems/drops-8/issues/168

[webflo]

+1 on adding an exception