Problem/Motivation

In Slack chat, @mixologic mentions that when you use Composer to build a Symfony project, you get some output at the very end helping you move forward with the next step.

Proposed resolution

Add a 'next steps' summary to the end of the composer create-project process for Drupal.

This could be a separate plugin, so that you can turn it off using composer remove. In fact, telling you how to do that will be one of the next steps. :-)

Conversely, add a warning to the end of a composer install in a git clone of Drupal, as this could be a sign of folks trying to build a site and making a common mistake.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23 created an issue. See original summary.

AaronMcHale’s picture

Oh I like this idea!

Off the top of my head, some things which might be nice to include:

  • Links to documentation on how to get started with Drupal (obviously).
  • Information about reporting bugs and feature requests (probably just linking off to docs on the matter).
  • Information about where you can get support (either one link to a page or multiple links, e.g. forums and stack exchange).
  • Information about how to get involved with the community, both online e.g. contributing patches, but also offline e.g. local meetups and DrupalCon (again this could probably be links off to these things but some initial text explaining that would be good).

The last one about community I think is possibly one of the most important because it's really important that people (especially those using Drupal for the first time) understand what the Drupal community is and how open source works (some people installing Drupal for the first time might not understand open source). I think this would be a useful way to help grow the community and keep people within Drupal for longer. There's that tag line we use a lot: "Come for the Software, Stay for the Community".

greg.1.anderson’s picture

If we added the message to a `composer create-project` hook, then we wouldn't need to worry about removing it, because it would only run once. It would probably be a good idea to include removal instructions, though, as the site won't need it after the initial run.

The question is, though, where should we put the message? We could put it in the scaffold plugin, but I'd rather not have a very Drupal-specific message there. Beyond that, it would probably be a good idea to make it easy for, say, an installation profile to be able to add to or replace the instructions.

Perhaps a separate project is best.

AaronMcHale’s picture

Beyond that, it would probably be a good idea to make it easy for, say, an installation profile to be able to add to or replace the instructions.

Yes, that would be a very desirable. I’m involved in a number of Drupal Distributions (one under active development, and another few in the planning stages) which are for very specific purposes and mostly designed for internal use, so being able to easily customise the post-installation Composer messaging would be very advantageous.

Mile23’s picture

Status: Active » Needs review
FileSize
33.84 KB

Here's a patch.

It allows you to set up different files for different events.

Currently it only allows you to respond to post-create-project-cmd and post-install-cmd Composer events.

You can see it in action like this:

$ git checkout 8.8.x
$ rm -rf vendor
$ composer install
$ git apply 3087482_5.patch
$ composer install

[..]

                       
  You have installed!  
                       

  * Get rid of this message with composer remove drupal/core-project-message.

This is a PoC which is why it's added to drupal/drupal. A real patch would not do this, because we're not trying to tell core developers any next steps.

Check the README in the patch for more details.

greg.1.anderson’s picture

Good start. I like the configuration options. Seems to meet all of the basic needs.

johnwebdev’s picture

Yeah looks promising!

Another thing to do post-install though! :/

Does Symfony also require you to remove the plugin to get rid of the message as well?

+++ b/composer/Plugin/ProjectMessage/composer.json
@@ -0,0 +1,20 @@
+  "description": "Adds a message after Composer installation.",

To me that reads like after installing Composer, as in, Composer itself.

Mile23’s picture

Does Symfony also require you to remove the plugin to get rid of the message as well?

Under Symfony the message is displayed as part of a Flex recipe, and only when you initially create-project.

For Drupal, if you say composer install a lot, you'll want to get rid of the message.

But it will only display once for composer create-project because you only ever do that once per project.

mbaynton’s picture

Issue summary: View changes

Capturing some discussion from the bi-weekly meeting:
The "next steps with Drupal" summary should be output at the end of composer create-project. On the other hand, a potentially common error would be to git clone Drupal and run composer install, which is the correct way to do core development but the incorrect way to build a new site. So, there should be some help text in case of composer installing from the git clone as well, but it should say "hey, you're doing core dev. this is for development purposes etc"

mixologic:drupalcon:  9 minutes ago
Does this need to run on install? or just create-project? Who would run composer install?
Mile23  8 minutes ago
it can be configured either way (or both).
mixologic:drupalcon:  8 minutes ago
Oooh. what if on composer install (which you should only do if you've got a git clone and are doing core dev) we said "hey, you're doing core dev. this is for development purposes etc"
Mile23  8 minutes ago
zactly.
mixologic:drupalcon:  8 minutes ago
btw. dont try to deploy this to productoin
Mile23’s picture

Adds a message to drupal/drupal.

AaronMcHale’s picture

+1 to #9 and #10. Only one suggestion, perhaps in the message provide a sentence and link to the recommended approaches for setting up a site, in case someone didn't intend to setup a development environment.

Mile23’s picture

Trimmed down the messages a little so they're noticeable but not obtrusive.

Added a link to the templates change record.

Added a message for create-project. It's red, and the install one is blue.

greg.1.anderson’s picture

Code is looking good. Should we also have post-install instructions for drupal/recommended-project and drupal/legacy-project?

It might be good to gather some wider feedback on what the messages should say, along the lines of #2.

Mile23’s picture

This adds messages to the templates, and also adds a feature. You can specify include-keys for homepage and/or support, and these elements from the composer.json file will be included at the end of your message.

We might add name and description as well, so that other users could bypass actually writing a message and just show the links

greg.1.anderson’s picture

Status: Needs review » Needs work

This is great! I think that #14 addresses #13. We can leave further improvements to the messages as follow-on work (unless someone else jumps in with specific suggestions).

I noticed that if you have no vendor directory, then the post-install message is printed twice. This is caused by the wikimedia/composer-merge-plugin (Wikimedia\Composer\MergePlugin::onPostInstallOrUpdate()). It is unfortunately not easy to detect that the message has been printed before, e.g. with a static variable or similar, due to the way that the Wikimedia plugin calls the install event. We could, I suppose, walk up the call stack and detect the wikimedia plugin in the backtrace, but it does not seem that avoiding the repetition is worth the hackery.

  • This should be re-rolled for 8.9.x. It might be back-port-able to 8.8.x after review, but I think all new code is going on 8.9.x first now.
  • drupalci.yml should be reverted so that all of the tests can run.
  • Could use a change record.

Ready for RTBC after that.

Mile23’s picture

Issue tags: +Needs change record
Mile23’s picture

Version: 8.8.x-dev » 8.9.x-dev
Status: Needs work » Needs review
FileSize
39.68 KB

Reverts drupalci.yml, rerolled for 8.9.x, updates README.

greg.1.anderson’s picture

It's working! (The composer lock hash test)

AaronMcHale’s picture

Some great progress here!

It bothers me a little that the user guide URL has index.html at the end, but it bothers me more that the URL gives a 404 when you try to access it without index.htm, so not much we can do about that.

That being said maybe we could get some form of short URLs generated for some these links, whether that be redirects under Drupal.org or on a third-party URL shortened. Also helpful in the even that someone is using a terminal where copy and paste to a web browser is not an option.

greg.1.anderson’s picture

Status: Needs review » Needs work

Setting to needs work for #17; probably needs a reroll by now.

I'm not sure what to advise about the url shortener. I'm happy to make whatever aliases folks want under g1a.io, but that's probably not appropriate for core. Does the DA already have a short domain? Maybe 'drupal.org' is short enough, if we just make a short alias? Shrug.

Mile23’s picture

Status: Needs work » Needs review
Issue tags: +Needs followup, +Needs documentation
FileSize
39.68 KB
1.01 KB

Updated the lock file, also fixed a CS error.

I think refining the message should be a follow-up. The important thing here is to make sure people know drupal/drupal is for dev. The follow-up can be more documentation-oriented.

greg.1.anderson’s picture

Status: Needs review » Reviewed & tested by the community

I think this is ready to go in, so I am RTBC-ing it. However, my updated understanding of the current Drupal 9 patch process is that core committers want patches for both 8.9.x and 9.0.x so that they can commit both at the same time.

AaronMcHale’s picture

Happy to refine wording in a followup, as like you said the key requirements are met here.

greg.1.anderson’s picture

I added a draft change record. It should do in a pinch, but feel free to enhance.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
  1. I think this is really great - this will make it much easier for beginners
  2. +++ b/composer/Plugin/ProjectMessage/Config.php
    @@ -0,0 +1,112 @@
    +/**
    + * Determine configuration.
    + */
    +class Config {
    

    I'm not sure about this object name. Does it not represent the message - should we call it Message and the method getMessageText() -> getText()?

  3. +++ b/composer/Plugin/ProjectMessage/Config.php
    @@ -0,0 +1,112 @@
    +   * @var Composer\Package\RootPackageInterface
    

    Needs a leading slash

  4. +++ b/composer/Plugin/ProjectMessage/Config.php
    @@ -0,0 +1,112 @@
    +    $file = $this->eventName . '-message.txt';
    +    if ($config_file = $package_config['drupal-core-project-message'][$this->eventName . '-file'] ?? FALSE) {
    +      $file = $config_file;
    +    }
    

    Are we sure about supporting files at this point - it seems like we should start by adding support for one type of way to do messages and then add this after more discussion.

  5. +++ b/composer/Template/LegacyProject/composer.json
    @@ -41,6 +47,25 @@
    +        "drupal-core-project-message": {
    
    +++ b/composer/Template/RecommendedProject/composer.json
    @@ -40,6 +46,25 @@
    +        "drupal-core-project-message": {
    

    On the other hand - these messages are completely the same maybe we should put these in a single file because then they won't get out-of-sync. On this this makes me ponder is where would this file reside. If we can't answer that then maybe the file thing is not that useful?

  6. +++ b/composer/Template/RecommendedProject/composer.json
    @@ -40,6 +46,25 @@
    +                "  * Remove the plugin that prints this message:",
    +                "      composer remove drupal/core-project-message"
    

    This is really smart way of allowing people to remove the message. I like it.

Mile23’s picture

#25.4,5:

The file output part roughly follows the Symfony Flex model for post-install-output. http://fabien.potencier.org/symfony4-workflow-automation.html

If we were to use a file for the project templates, that file would also need to be in both of the project templates, leaving the same problem of maintaining two almost-same files.

Mile23’s picture

Status: Needs work » Needs review
Issue tags: -Needs change record, -Needs followup

Re: #24.4,5: As mentioned in #26, if the two messages are the same, we still have to have two different files so they live in different subtree splits. Also, they’re not the same message. :-) But most importantly: Discussion about the contents of the messages (same or different) can be in a follow-up. And here is the follow-up: #3091274: Decide on 'next steps' messages for project templates

Changed the Config class to be named Message.

Fixed CS.

Greg did the change record.

I’ll make a docs page.

Mile23’s picture

Mile23’s picture

Status: Needs review » Needs work

The last submitted patch, 28: 3087482_27.patch, failed testing. View results

Mile23’s picture

Status: Needs work » Needs review
FileSize
39.97 KB
481 bytes

Update lock file.

afeijo’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, I see the changes that Alex requested and the new doc page looks nice.

alexpott’s picture

Version: 8.9.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed b547ffe and pushed to 9.0.x. Thanks!
Committed 70addd7 and pushed to 8.9.x. Thanks!

I'll ask @catch about backporting this.

  • alexpott committed b547ffe on 9.0.x
    Issue #3087482 by Mile23, greg.1.anderson, AaronMcHale, johndevman,...

  • alexpott committed 70addd7 on 8.9.x
    Issue #3087482 by Mile23, greg.1.anderson, AaronMcHale, johndevman,...
xjm’s picture

This broke Drupal.org packaging for 9.0.x and 8.9.x. @drumm and @Mixologic are looking into it, but if they aren't able to fix it shortly we should revert this. We should definitely not backport this to 8.8.x until packaging is fixed.

xjm’s picture

@greg.1.anderson is working on a patch to remove the PHP 7-specific syntax from the plugin to hotfix this (hopefully).

greg.1.anderson’s picture

Status: Patch (to be ported) » Needs review
FileSize
1.69 KB

Here's a PHP 5.x version of the Message Plugin.

alexpott’s picture

Status: Needs review » Reviewed & tested by the community

Tested manually, without the patch on 5.6...

composer install --ignore-platform-reqs
> Drupal\Composer\Composer::ensureComposerVersion
Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Nothing to install or update
Package phpunit/phpunit-mock-objects is abandoned, you should avoid using it. No replacement was suggested.
Generating autoload files
> Drupal\Core\Composer\Composer::preAutoloadDump
Hardening vendor directory with .htaccess and web.config files.
Cleaning vendor directory.
PHP Parse error:  syntax error, unexpected '?' in /Users/alex/dev/sites/drupal8alt.dev/composer/Plugin/ProjectMessage/Message.php on line 52

Parse error: syntax error, unexpected '?' in /Users/alex/dev/sites/drupal8alt.dev/composer/Plugin/ProjectMessage/Message.php on line 52

with the patch:

composer install --ignore-platform-reqs
> Drupal\Composer\Composer::ensureComposerVersion
Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Nothing to install or update
Package phpunit/phpunit-mock-objects is abandoned, you should avoid using it. No replacement was suggested.
Generating autoload files
> Drupal\Core\Composer\Composer::preAutoloadDump
Hardening vendor directory with .htaccess and web.config files.
Cleaning vendor directory.
drupal/drupal: This package is meant for core development,
               and not intended to be used for production sites.
               See: https://www.drupal.org/node/3082474

So this looks great.

  • alexpott committed 32b0bac on 9.0.x
    Revert "Issue #3087482 by Mile23, greg.1.anderson, AaronMcHale,...

  • alexpott committed 866a1e6 on 8.9.x
    Revert "Issue #3087482 by Mile23, greg.1.anderson, AaronMcHale,...
alexpott’s picture

Version: 8.8.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Needs work

We ended up reverting this after further issues with Drupal packaging. There is a plan to move d.o packaging on to PHP 7 - @mixologic / @drumm will hopefully be abler to do this soon.

drumm’s picture

Status: Needs work » Needs review

Drupal.org packaging now uses PHP 7.3 for running composer: https://git.drupalcode.org/project/drupalorg/commit/d248718

Mixologic’s picture

Status: Needs review » Reviewed & tested by the community

I've tested the packaging of this now, and it works. The only issue remaining is the presence of the wikimedia plugin will cause the message to happen twice if there is no vendor.

We can remove the merge plugin entirely from drupal/drupal once we have the tarballs being generated by drupal.org, because they are only there for that layer of BC.

alexpott’s picture

The only issue remaining is the presence of the wikimedia plugin will cause the message to happen twice if there is no vendor.

Can you file a follow-up to fix this?

alexpott’s picture

Version: 8.9.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

I reverted the reverts. Setting back to 8.8.x to get an opinion from @catch for backport.

  • alexpott committed 661b7cc on 9.0.x
    Revert "Revert "Issue #3087482 by Mile23, greg.1.anderson, AaronMcHale,...

  • alexpott committed abb42fc on 8.9.x
    Revert "Revert "Issue #3087482 by Mile23, greg.1.anderson, AaronMcHale,...
xjm’s picture

Crediting myself, drumm, and Mixologic for work on tracking this down and (in their case) fixing d.o so this could be recommitted.

xjm’s picture

And actually checking the boxes.

xjm’s picture

Issue tags: +Amsterdam2019
greg.1.anderson’s picture

#45: Once the tarball downloads are build from drupal/legacy-project, we should be able to remove wikimedia/composer-merge-plugin from drupal/drupal, which will take care of the duplicate messages. I thought we already had a follow-up for that (post #2912387: Stop using wikimedia/composer-merge-plugin), but I couldn't find it, so I guess it still needs to be created.

Mile23’s picture

Status: Patch (to be ported) » Needs review
FileSize
39.97 KB

Here's the 8.8.x version.

alexpott’s picture

Status: Needs review » Reviewed & tested by the community

Diff between #31 and #53 - so the only changes are composer lock changes.

diff 3087482_31.patch.1 3087482_88x_53.patch 
52c52
< index 57ad28c5e8..e6baa6b15a 100644
---
> index 593779e58f..a6ed633123 100644
64c64
< @@ -877,6 +877,36 @@
---
> @@ -876,6 +876,36 @@
70c70
< +            "version": "8.9.x-dev",
---
> +            "version": "8.8.x-dev",
100,101c100,101
<              "version": "8.9.x-dev",
< @@ -6235,6 +6265,7 @@
---
>              "version": "8.8.x-dev",
> @@ -6234,6 +6264,7 @@
Mile23’s picture

The reason to have this in 8.8.x is so that we can display this message whenever anyone does composer install:

drupal/drupal: This package is meant for core development,
               and not intended to be used for production sites.
               See: https://www.drupal.org/node/3082474

That way they can know what's up, and have some documentation about the changes, and have this information during 8.8.0+ times.

alexpott’s picture

Status: Reviewed & tested by the community » Patch (to be ported)

Both @catch and I use the "patch to be ported" status to separate these from the other rtbc's that are yet to be committed to any branch. It makes it less likely to be missed.

alexpott’s picture

Status: Patch (to be ported) » Fixed
FileSize
279.72 KB

@catch and I agreed to backport.

Committed a1b5b5b and pushed to 8.8.x. Thanks!

Here's what it looks like when it works with composer create-project and the new metapackages.

  • alexpott committed a1b5b5b on 8.8.x
    Issue #3087482 by Mile23, greg.1.anderson, alexpott, xjm, AaronMcHale,...
jibran’s picture

In D9 commit composer/Plugin/ProjectMessage/composer.json has wrong PHP version.

jibran’s picture

Status: Fixed » Needs review
FileSize
1.06 KB
alexpott’s picture

Status: Needs review » Fixed

@jibran well as the code is composer/Plugin/ProjectMessage is the same on all branches we know that it's does actually support "php": ">=7.0.8", - but sure if we want to update it to be >=7.2.3 let's open a follow-up for that. For me it's not urgent or critical (or even particularly wrong) - but our test infra doesn't support testing components separately or allow them to test on their own PHP versions so we should make the change FWIW.

And @jibran please do not re-open issues for follow-ups.

alexpott’s picture

Fixing issue credit.

jibran’s picture

It was literally a one-line fix. Anyway created #3092209: Update PHP version for drupal/core-project-message.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.