fix: avoid `type: php` to detected project type but instead show a warning, fixes #4403, fixes #1919 #5011

gilbertsoft · 2023-06-22T18:36:32Z

The Issue

Any use of ddev config automatically changes a project type from 'php' to the discovered codebase type #4403
'ddev config' should default to existing config values unless overridden with flags #1919

How This PR Solves The Issue

If an app type was set already before running ddev config it won't longer be overwritten by the detected app type but a warning is shown to let the user know about. The only way to change the type config is to provide a new value using --projecttype flag.

Not implemented so far but could be an option to allow the type auto to enforce the value from the detection. But to me this currently looks superfluous.

Manual Testing Instructions

Use ddev config --projecttype=... with an other type than already specified in the project. This should set the new value and not using the auto detected type.

Automated Testing Overview

No new tests are introduced.

Release/Deployment Notes

github-actions · 2023-06-22T19:00:57Z

Download the artifacts for this pull request:

See Testing a PR

rfay

This seems to work as specified, but still emits the message:
You have specified a project type of 'php' but a project of type 'drupal9' was discovered in /home/rfay/workspace/d9/web - is that intended? (Edit: I see from the title that it is intended.)

However, if you use the simple ddev config command, it discovers the docroot and then switches to the discovered type. That's not what you want is it?

rfay

Thanks for solving this problem.

It looks to me like this mostly solves the problem, except that it doesn't respect existing with ddev config interactive configuration.

Could we use none for the no-project-type option? Have a better idea?

The ddev config --project-type would then have to take "none", but I imagine it already would with GetAppTypes() having the listed items.

Docs will need update to explain the situation of "none"; I'm pretty sure they say "php" is the default.

Don't forget that "none" in a python or django4 project would be controlled by webserver_type and may be a bit different.

pkg/ddevapp/config.go

rfay · 2023-06-26T22:06:09Z

pkg/nodeps/values.go

@@ -82,6 +82,7 @@ var ValidNodeJSVersions = []string{"14", "16", "18", "20"}

 // App types
 const (
+	AppTypeNone      = ""


I think this would be better as "none", not sure. Hate empty values like this.

Just an internal thing which never gets saved to the config, so value is not important here but we can change it to none if you prefer.

gilbertsoft · 2023-06-27T06:32:04Z

However, if you use the simple ddev config command, it discovers the docroot and then switches to the discovered type. That's not what you want is it?

Have to check interactive config again...

gilbertsoft · 2023-06-27T06:35:06Z

It looks to me like this mostly solves the problem, except that it doesn't respect existing with ddev config interactive configuration.

Will check again...

Could we use none for the no-project-type option? Have a better idea?

The ddev config --project-type would then have to take "none", but I imagine it already would with GetAppTypes() having the listed items.

No, like explained above none is an internal value only during the init of the ddevapp and is no valid option for the user. If it's still none on saving it gets replaced by php instead.

Docs will need update to explain the situation of "none"; I'm pretty sure they say "php" is the default.

php is still the default, has not changed see above.

gilbertsoft · 2023-06-27T10:08:47Z

Interactive config works for me so far. Can you please explain your exact case a bit more @rfay ?

rfay · 2023-06-27T13:13:14Z

Here's interactive config with it changing the project type from php back to drupal9. Maybe this is intended and OK? But it's changing it back when not expected probably.

# Existing drupal9
$ ddev config --project-type=php
You are reconfiguring the project at /Users/rfay/workspace/d9.
The existing configuration will be updated and replaced.
You have specified a project type of 'php' but a project of type 'drupal9' was discovered in /Users/rfay/workspace/d9/web
2023-06-27T07:10:37.163 Flushed mutagen sync session 'd9'
Configuration complete. You may now run 'ddev start'.
rfay@rfay-tag1-m1:~/workspace/d9$ ddev describe
2023-06-27T07:10:48.242 detected terminal width=98 urlPortWidth=39.2 infoWidth=24
┌────────────────────────────────────────────────────────────────────────────────────┐
│ Project: d9 ~/workspace/d9 https://d9.ddev.site                                    │
│ Docker provider: Colima                                                            │
│ Router: traefik                                                                    │
├────────────┬──────┬─────────────────────────────────────────┬──────────────────────┤
│ SERVICE    │ STAT │ URL/PORT                                │ INFO                 │
├────────────┼──────┼─────────────────────────────────────────┼──────────────────────┤
│ web        │ OK   │ https://d9.ddev.site                    │ php PHP8.1           │
│            │      │ InDocker: web:443,80,8025               │ nginx-fpm            │
│            │      │ Host: 127.0.0.1:49565,49564             │ docroot:'web'        │
│            │      │                                         │ Mutagen enabled (ok) │
│            │      │                                         │ NodeJS:14            │
├────────────┼──────┼─────────────────────────────────────────┼──────────────────────┤
│ db         │ OK   │ InDocker: db:3306                       │ mariadb:10.4         │
│            │      │ Host: 127.0.0.1:9102                    │ User/Pass: 'db/db'   │
│            │      │                                         │ or 'root/root'       │
├────────────┼──────┼─────────────────────────────────────────┼──────────────────────┤
│ PHPMyAdmin │ OK   │ https://d9.ddev.site:8037               │                      │
│            │      │ InDocker: dba:80,80                     │                      │
│            │      │ `ddev launch -p`                        │                      │
├────────────┼──────┼─────────────────────────────────────────┼──────────────────────┤
│ Mailhog    │      │ MailHog: https://d9.ddev.site:8026      │                      │
│            │      │ `ddev launch -m`                        │                      │
├────────────┼──────┼─────────────────────────────────────────┼──────────────────────┤
│ All URLs   │      │ https://d9.ddev.site,                   │                      │
│            │      │ https://127.0.0.1:49564,                │                      │
│            │      │ http://d9.ddev.site,                    │                      │
│            │      │ http://127.0.0.1:49565                  │                      │
└────────────┴──────┴─────────────────────────────────────────┴──────────────────────┘

rfay@rfay-tag1-m1:~/workspace/d9$ ddev config
You are reconfiguring the project at /Users/rfay/workspace/d9.
The existing configuration will be updated and replaced.
Project name (d9):

The docroot is the directory from which your site is served.
This is a relative path from your project root at /Users/rfay/workspace/d9
You may leave this value blank if your site files are in the project root
Docroot Location (web):
Found a drupal9 codebase at /Users/rfay/workspace/d9/web.
Project Type [backdrop, craftcms, django4, drupal10, drupal6, drupal7, drupal8, drupal9, laravel, magento, magento2, php, python, shopware6, typo3, wordpress] (drupal9):
2023-06-27T07:11:09.019 Ensuring write permissions for d9
2023-06-27T07:11:09.02 Existing settings.php file includes settings.ddev.php
Configuration complete. You may now run 'ddev start'.
rfay@rfay-tag1-m1:~/workspace/d9$ ddev describe
2023-06-27T07:11:16.734 detected terminal width=98 urlPortWidth=39.2 infoWidth=24
┌────────────────────────────────────────────────────────────────────────────────────┐
│ Project: d9 ~/workspace/d9 https://d9.ddev.site                                    │
│ Docker provider: Colima                                                            │
│ Router: traefik                                                                    │
├────────────┬──────┬─────────────────────────────────────────┬──────────────────────┤
│ SERVICE    │ STAT │ URL/PORT                                │ INFO                 │
├────────────┼──────┼─────────────────────────────────────────┼──────────────────────┤
│ web        │ OK   │ https://d9.ddev.site                    │ drupal9 PHP8.1       │
│            │      │ InDocker: web:8025,443,80               │ nginx-fpm            │
│            │      │ Host: 127.0.0.1:49564,49565             │ docroot:'web'        │
│            │      │                                         │ Mutagen enabled (ok) │
│            │      │                                         │ NodeJS:14            │
├────────────┼──────┼─────────────────────────────────────────┼──────────────────────┤
│ db         │ OK   │ InDocker: db:3306                       │ mariadb:10.4         │
│            │      │ Host: 127.0.0.1:9102                    │ User/Pass: 'db/db'   │
│            │      │                                         │ or 'root/root'       │
├────────────┼──────┼─────────────────────────────────────────┼──────────────────────┤
│ PHPMyAdmin │ OK   │ https://d9.ddev.site:8037               │                      │
│            │      │ InDocker: dba:80,80                     │                      │
│            │      │ `ddev launch -p`                        │                      │
├────────────┼──────┼─────────────────────────────────────────┼──────────────────────┤
│ Mailhog    │      │ MailHog: https://d9.ddev.site:8026      │                      │
│            │      │ `ddev launch -m`                        │                      │
├────────────┼──────┼─────────────────────────────────────────┼──────────────────────┤
│ All URLs   │      │ https://d9.ddev.site,                   │                      │
│            │      │ https://127.0.0.1:49564,                │                      │
│            │      │ http://d9.ddev.site,                    │                      │
│            │      │ http://127.0.0.1:49565                  │                      │
└────────────┴──────┴─────────────────────────────────────────┴──────────────────────┘

gilbertsoft · 2023-06-27T13:45:55Z

Here's interactive config with it changing the project type from php back to drupal9. Maybe this is intended and OK? But it's changing it back when not expected probably.

Got it now. Okay, we have to discuss the desired behavior then. At the moment the discovered project type is suggested during interactive setup but the user can change it again to another type. To me this looks fine but well, one could have an other opinion. I'm open for both ways...

rfay · 2023-07-03T14:46:38Z

Rebased

rfay

Seems good to me, did manual testing again. Thanks!

rfay changed the title ~~Avoid changing an already set type but show a warning~~ Avoid changing an already set type but show a warning, fixes #4403 Jun 22, 2023

gilbertsoft force-pushed the bugfix/config-command branch 10 times, most recently from 2c81c2a to 121f225 Compare June 25, 2023 18:25

gilbertsoft marked this pull request as ready for review June 26, 2023 07:35

gilbertsoft requested a review from a team as a code owner June 26, 2023 07:35

rfay changed the title ~~Avoid changing an already set type but show a warning, fixes #4403~~ Avoid changing an already set type but show a warning, fixes #4403, fixes #1919 Jun 26, 2023

rfay reviewed Jun 26, 2023

View reviewed changes

rfay requested changes Jun 26, 2023

View reviewed changes

rfay changed the title ~~Avoid changing an already set type but show a warning, fixes #4403, fixes #1919~~ Avoid type: php to detected project type but instead show a warning, fixes #4403, fixes #1919 Jun 27, 2023

gilbertsoft force-pushed the bugfix/config-command branch from 121f225 to c7621a4 Compare June 27, 2023 17:04

gilbertsoft changed the title ~~Avoid type: php to detected project type but instead show a warning, fixes #4403, fixes #1919~~ fix: avoid type: php to detected project type but instead show a warning, fixes #4403, fixes #1919 Jun 28, 2023

gilbertsoft force-pushed the bugfix/config-command branch from c7621a4 to 62cce53 Compare June 30, 2023 08:19

gilbertsoft and others added 3 commits July 3, 2023 08:44

Avoid changing an already set type but show a warning

5ab0a84

Respect configured AppType on interactive config

9a55c61

Fix up imports in apptypes.go

f1bc8f0

rfay force-pushed the bugfix/config-command branch from 62cce53 to f1bc8f0 Compare July 3, 2023 14:46

github-actions bot added the bugfix label Jul 3, 2023

rfay approved these changes Jul 3, 2023

View reviewed changes

rfay merged commit a22c463 into ddev:master Jul 4, 2023
19 checks passed

rfay deleted the bugfix/config-command branch July 4, 2023 18:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: avoid `type: php` to detected project type but instead show a warning, fixes #4403, fixes #1919 #5011

fix: avoid `type: php` to detected project type but instead show a warning, fixes #4403, fixes #1919 #5011

gilbertsoft commented Jun 22, 2023 •

edited by rfay

github-actions bot commented Jun 22, 2023 •

edited

rfay left a comment •

edited

rfay left a comment

rfay Jun 26, 2023

gilbertsoft Jun 27, 2023

gilbertsoft commented Jun 27, 2023

gilbertsoft commented Jun 27, 2023

gilbertsoft commented Jun 27, 2023

rfay commented Jun 27, 2023

gilbertsoft commented Jun 27, 2023 •

edited

rfay commented Jul 3, 2023

rfay left a comment

fix: avoid type: php to detected project type but instead show a warning, fixes #4403, fixes #1919 #5011

fix: avoid type: php to detected project type but instead show a warning, fixes #4403, fixes #1919 #5011

Conversation

gilbertsoft commented Jun 22, 2023 • edited by rfay

The Issue

How This PR Solves The Issue

Manual Testing Instructions

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

github-actions bot commented Jun 22, 2023 • edited

rfay left a comment • edited

Choose a reason for hiding this comment

rfay left a comment

Choose a reason for hiding this comment

rfay Jun 26, 2023

Choose a reason for hiding this comment

gilbertsoft Jun 27, 2023

Choose a reason for hiding this comment

gilbertsoft commented Jun 27, 2023

gilbertsoft commented Jun 27, 2023

gilbertsoft commented Jun 27, 2023

rfay commented Jun 27, 2023

gilbertsoft commented Jun 27, 2023 • edited

rfay commented Jul 3, 2023

rfay left a comment

Choose a reason for hiding this comment

fix: avoid `type: php` to detected project type but instead show a warning, fixes #4403, fixes #1919 #5011

fix: avoid `type: php` to detected project type but instead show a warning, fixes #4403, fixes #1919 #5011

gilbertsoft commented Jun 22, 2023 •

edited by rfay

github-actions bot commented Jun 22, 2023 •

edited

rfay left a comment •

edited

gilbertsoft commented Jun 27, 2023 •

edited