Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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
Comment | File | Size | Author |
---|---|---|---|
#60 | 3087482-9.0.x-follow-up.patch | 1.06 KB | jibran |
#57 | Screenshot 2019-11-04 at 12.11.02.png | 279.72 KB | alexpott |
#53 | 3087482_88x_53.patch | 39.97 KB | Mile23 |
#38 | 3087482-37.patch | 1.69 KB | greg.1.anderson |
#31 | interdiff.txt | 481 bytes | Mile23 |
Comments
Comment #2
AaronMcHaleOh I like this idea!
Off the top of my head, some things which might be nice to include:
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".
Comment #3
greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commentedIf 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.
Comment #4
AaronMcHaleYes, 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.
Comment #5
Mile23Here'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:
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.
Comment #6
greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commentedGood start. I like the configuration options. Seems to meet all of the basic needs.
Comment #7
johnwebdev CreditAttribution: johnwebdev commentedYeah 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?
To me that reads like after installing Composer, as in, Composer itself.
Comment #8
Mile23Under 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.Comment #9
mbayntonCapturing 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 runcomposer 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 ofcomposer install
ing from the git clone as well, but it should say "hey, you're doing core dev. this is for development purposes etc"Comment #10
Mile23Adds a message to drupal/drupal.
Comment #11
AaronMcHale+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.
Comment #12
Mile23Trimmed 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.
Comment #13
greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commentedCode 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.
Comment #14
Mile23This adds messages to the templates, and also adds a feature. You can specify
include-keys
forhomepage
and/orsupport
, and these elements from the composer.json file will be included at the end of your message.We might add
name
anddescription
as well, so that other users could bypass actually writing a message and just show the linksComment #15
greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commentedThis 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.Ready for RTBC after that.
Comment #16
Mile23Comment #17
Mile23Reverts drupalci.yml, rerolled for 8.9.x, updates README.
Comment #18
greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commentedIt's working! (The composer lock hash test)
Comment #19
AaronMcHaleSome 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.
Comment #20
greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commentedSetting 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.
Comment #21
Mile23Updated 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.
Comment #22
greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commentedI 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.
Comment #23
AaronMcHaleHappy to refine wording in a followup, as like you said the key requirements are met here.
Comment #24
greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commentedI added a draft change record. It should do in a pinch, but feel free to enhance.
Comment #25
alexpottI'm not sure about this object name. Does it not represent the message - should we call it Message and the method getMessageText() -> getText()?
Needs a leading slash
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.
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?
This is really smart way of allowing people to remove the message. I like it.
Comment #26
Mile23#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.
Comment #27
Mile23Re: #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.
Comment #28
Mile23Comment #29
Mile23Added docs page: https://www.drupal.org/docs/develop/using-composer/using-drupals-project...
Comment #31
Mile23Update lock file.
Comment #32
afeijoLooks good, I see the changes that Alex requested and the new doc page looks nice.
Comment #33
alexpottCommitted b547ffe and pushed to 9.0.x. Thanks!
Committed 70addd7 and pushed to 8.9.x. Thanks!
I'll ask @catch about backporting this.
Comment #36
xjmThis 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.
Comment #37
xjm@greg.1.anderson is working on a patch to remove the PHP 7-specific syntax from the plugin to hotfix this (hopefully).
Comment #38
greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commentedHere's a PHP 5.x version of the Message Plugin.
Comment #39
alexpottTested manually, without the patch on 5.6...
with the patch:
So this looks great.
Comment #42
alexpottWe 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.
Comment #43
drummDrupal.org packaging now uses PHP 7.3 for running composer: https://git.drupalcode.org/project/drupalorg/commit/d248718
Comment #44
MixologicI'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.
Comment #45
alexpottCan you file a follow-up to fix this?
Comment #46
alexpottI reverted the reverts. Setting back to 8.8.x to get an opinion from @catch for backport.
Comment #49
xjmCrediting myself, drumm, and Mixologic for work on tracking this down and (in their case) fixing d.o so this could be recommitted.
Comment #50
xjmAnd actually checking the boxes.
Comment #51
xjmComment #52
greg.1.anderson CreditAttribution: greg.1.anderson commented#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.
Comment #53
Mile23Here's the 8.8.x version.
Comment #54
alexpottDiff between #31 and #53 - so the only changes are composer lock changes.
Comment #55
Mile23The reason to have this in 8.8.x is so that we can display this message whenever anyone does composer install:
That way they can know what's up, and have some documentation about the changes, and have this information during 8.8.0+ times.
Comment #56
alexpottBoth @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.
Comment #57
alexpott@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.
Comment #59
jibranIn D9 commit
composer/Plugin/ProjectMessage/composer.json
has wrong PHP version.Comment #60
jibranComment #61
alexpott@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.
Comment #62
alexpottFixing issue credit.
Comment #63
jibranIt was literally a one-line fix. Anyway created #3092209: Update PHP version for drupal/core-project-message.