ddev / ddev

Docker-based local PHP+Node.js web development environments
https://ddev.com
Apache License 2.0
2.75k stars 602 forks source link

Sync default DDEV PHP version with current Drupal release #6306

Closed gitressa closed 4 months ago

gitressa commented 4 months ago

Preliminary checklist

Problem

Drupal 11 will be released the week of July 29, 2024.

https://www.drupal.org/about/core/policies/core-release-cycles/schedule

It would be nice if this command gave you PHP 8.3 before the week of July 29, 2024:

ddev config --project-type=drupal --docroot=web

Workaround

ddev config --project-type=drupal --docroot=web --php-version=8.3

Solution

PROPOSAL: Sync the default PHP version of DDEV with PHP requirements of the current release of Drupal, so that these simple steps get you the latest Drupal, with the correct PHP version:

mkdir drupal11 && cd drupal11
ddev config --project-type=drupal --docroot=web
ddev start
ddev composer create drupal/recommended-project

I found https://github.com/rfay/ddev/blob/master/pkg/nodeps/php_values.go where it looks like the default PHP version for all systems is PHP 8.2. Perhaps other systems are not ready for PHP 8.3 ... so should the default drupal config be changed from using PHPDefault to PHP 8.3? (I am not sure where)

stasadev commented 4 months ago

Hi @gitressa,

With a new drupal project type (that handles Drupal 8,9,10,11), it's impossible to detect the Drupal version, that will be used.

That's why our quickstart for Drupal sets --php-version explicitly https://ddev.readthedocs.io/en/stable/users/quickstart/#drupal

And even when you try Drupal 11 quickstart without --php-version, it will be updated to 8.3 when you run ddev config --update.


I understand that you simply want to change the default PHP version for Drupal. I think it's reasonable, because we use --php-version=8.3 for Drupal 10 and 11 quickstarts.

This is the place to do it:

https://github.com/ddev/ddev/blob/ef57efe0bf601a2836c4242a514f487af9c36303/pkg/ddevapp/drupal.go#L359-L364

if err != nil || v == "" {
+   app.PHPVersion = nodeps.PHP83
    util.Warning("Unable to detect Drupal version, continuing")
    return nil
}

Thanks for bringing up this topic.

gitressa commented 4 months ago

Thanks for a fast reply @stasadev and link to the /pkg/ddevapp/drupal.go file. I tried adding a line via a PR, maybe that's all it takes?

stasadev commented 4 months ago

Yeah, but it also needs a change in tests here:

https://github.com/ddev/ddev/blob/ef57efe0bf601a2836c4242a514f487af9c36303/pkg/ddevapp/apptypes_test.go#L68

gitressa commented 4 months ago

Ah yes, thanks -- I updated that file as well.

rfay commented 4 months ago

If people do the recommended ddev config --update then Drupal 11 sets things to PHP 8.3 already. Could you please see if that does the job for you?

rfay commented 4 months ago

As discussed in the PR, DDEV already handles this correctly and has for some time. Closing for now, happy to continue the conversation.

gitressa commented 4 months ago

Thanks for fast replies as always @stasadev and @rfay, I really appreciate it. You and the rest of the DDEV team do a stellar job maintaining and expanding the DDEV tool with new features, it makes working with Drupal so much easier. I do disagree with not setting the default PHP version to 8.3 for Drupal, and have added a comment in the PR.

gitressa commented 2 weeks ago

I just updated Drupal's Local Development Guide, adding --php-version=8.3. It doesn't seem right, that you need to increase the PHP version, for the current release of Drupal.

In my opinion, it's high time to increase the default PHP version in DDEV to 8.3, so that these commands set up Drupal 11 -- compact and short :-)

mkdir drupal11 && cd drupal11
ddev config --project-type=drupal --docroot=web
ddev start
ddev composer create drupal/recommended-project
rfay commented 2 weeks ago

Thanks for keeping those docs maintained! I hope you try to sync them with https://ddev.readthedocs.io/en/stable/users/quickstart/#drupal

If you use the community-agreed approach with ddev config --update, you won't need the --php-version=8.3, because it's the job of ddev config --update to detect that. See the Drupal 10 and Drupal 11 instructions in https://ddev.readthedocs.io/en/stable/users/quickstart/#drupal-drupal-11.

Didn't you promote that approach?

Note that DDEV v1.24.0 in late November or December will have PHP 8.3 default for everything that doesn't have other specifications.

gitressa commented 2 weeks ago

You're welcome!

Sorry, but I don't agree that config --update is a great solution. It seems kludgy to me. In my opinion, setting PHP 8.3 as the default version in July to coincide with the release of Drupal 11 would have been the correct solution. PHP projects still on 8.1 or 8.2 should bear the burden of jumping through hoops, to get a correct configuration.

It feels like Drupal is being punished for doing best practices, and using the current stable PHP 8.3. https://www.php.net/downloads.php#v8.3.12

Then, the four lines I posted above your comment would be enough.

Anyway, I am looking forward to having the PHP 8.3 as the default version, I only wish it had coincided with the release of Drupal 11 in July 2024.

Perhaps moving forward, DDEV could try to keep default PHP version in sync with Drupal PHP requirements, so that the default PHP version is identical to the current release of Drupal? https://www.drupal.org/docs/getting-started/system-requirements/php-requirements#versions

gitressa commented 2 weeks ago

Adding my comment from PR #6307 (June 14th 2024) as well, for completeness:

I am a big proponent of removing extra steps and parameters, as can be seen in Make Composer create-project As Lean As Possible (ALAP). This issue was a success, since extra parameters are no longer needed, and the most basic version of a command such as this works well:

composer create-project drupal/recommended-project

Extra steps and parameters can become like magic dust for the user to sprinkle, sometimes without really knowing what it does, or why it's there (--update). And simply put, extra steps and parameters are just annoying.

Also, I do believe that the correct thing to do is make the default PHP version for Drupal the currently recommended version, which is PHP 8.3 for both Drupal 10 and Drupal 11.

I am very grateful for the simplified drupal type, but actually, it says right there in the Issue Summary of the issue:

Solution

Unify Drupal 8+ under a drupal project type, following the recommended PHP version for the current release [my emphasis] (Drupal 10 and PHP 8.1)

  • 4957

The default for Cake is already set to PHP 8.3, so why not also for Drupal?

https://github.com/ddev/ddev/blob/375963b4625d8e709773002e839ef3e2bf9a67b5/pkg/ddevapp/cakephp.go#L79

I hope at least this issue can be reopened, possibly as postponed, and that the default PHP version can be set to PHP 8.3 for Drupal by the end of July 2024, at the latest, since Drupal 11 will be released there, and it requires PHP 8.3.

rfay commented 2 weeks ago

Related conversation in

We obviously need to keep talking.

Note that DDEV did in fact change the default version for D10 and D11 to PHP8.3 before release of D11. It just didn't change the default for drupal. And of course, there's the dependency on ddev config --update figuring it all out.

gitressa commented 2 weeks ago

By all means, let's keep talking ... but I think I have made my point of view clear by now :-)

What do you think about my proposal?

Perhaps moving forward, DDEV could try to keep default PHP version in sync with Drupal PHP requirements, so that the default PHP version is identical to the current release of Drupal? https://www.drupal.org/docs/getting-started/system-requirements/php-requirements#versions

rfay commented 2 weeks ago

The reason we didn't do that earlier is that we try not to change defaults except in major version releases.

rfay commented 2 weeks ago

Followup,