Problem/Motivation
#3498907: The launcher script can cause an error if a DDEV project with the same name already exists added a new if statement to prevent errors if a project type name is already taken that adds that -1 for every project that is created. This results in -1 being added on every created project even if it is unique.
Steps to reproduce
- Download the zip file and execute the launcher script
- See that the project name is appended with
-1even if it is unique
Proposed resolution
To summarize a few ideas how to improve that @stasadev already comment on https://github.com/ddev/ddev/pull/6893#issuecomment-2593998198 a similar approach to the @rfay provided in the discussion on slack. instead of going with
if [ $n > 0 ]; then
NAME=$NAME-$(expr $n + 1)
fiuse the following instead:
if [ "${n}" -gt 0 ]; then
NAME=${NAME}-$((n + 1))
fi
and to work around the declare statement avoiding the ddev list and grep @stasadev proposed the following approach in a followup conversation over discord:
set -e
if ! command -v ddev >/dev/null 2>&1; then
echo "DDEV needs to be installed. Visit https://ddev.com/get-started for instructions."
exit 1
fi
# If there is no existing project yet.
if ! ddev describe >/dev/null 2>&1; then
# If there is any other DDEV project in this system with this name, add a numeric suffix.
NAME="$(basename $PWD)"
SUFFIX=1
# Loop to increment the suffix until the name is unique.
while ddev describe "${NAME}" >/dev/null 2>&1; do
NAME="$(basename $PWD)-${SUFFIX}"
SUFFIX=$((SUFFIX + 1))
done
# Configure a new project.
ddev config --project-type=drupal11 --docroot=web --php-version=8.3 --ddev-version-constraint=">=1.24.0" --project-name="$NAME"
fi
PROJECT_ROOT="$(ddev describe -j | docker run -i --rm ddev/ddev-utilities jq -r .raw.approot)"
CURRENT_DIRECTORY="$(pwd)"
cd "${PROJECT_ROOT}"
if [ "$(pwd)" != "${CURRENT_DIRECTORY}" ]; then
echo "You're trying to set up a Drupal CMS inside an existing project in $(pwd), refusing to do it."
exit 1
fi
echo "Running script in $(pwd)"
# Start your engines.
ddev start
# Install dependencies if not already done.
test -f composer.lock || ddev composer install
# All set, let's get Drupalin'.
ddev launch
But as a disclaimer, I did a brief test and ran into some problems with the approach from Stas. But i gotta retry tomorrow when i have a little more peace and quiet and test more thoroughly..
User interface changes
Data model changes
Release notes snippet
Issue fork drupal_cms-3500131
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- adjust-launch-script
changes, plain diff MR !456
Comments
Comment #2
rkollerComment #3
zeshan.ziya commentedThis looks correct, but I see one issue: if I run the script again by mistake, it will create a new project each time instead of starting the current project. It keeps appending a numeric suffix to the name each time it runs.
With @stasadev's suggestion, I see another issue. If we try to install it inside inner directories of any DDEV project, it will not work as expected. This is because `ddev describe` will return the project from the parent directory instead of the current directory. So, a more reliable approach could be:
Comment #4
stasadev commented> With @stasadev's suggestion, I see another issue. If we try to install it inside inner directories of any DDEV project, it will not work as expected. This is because `ddev describe` will return the project from the parent directory instead of the current directory.
I am not sure if this is the right approach to create a DDEV project inside another DDEV project.
By the way, in the next release of DDEV, it will not be possible to configure a DDEV project inside another DDEV project using the command line (`.ddev/config.yaml` still can be created manually) https://github.com/ddev/ddev/pull/6852
Comment #5
zeshan.ziya commentedThanks, @stasadev, for highlighting the upcoming change in DDEV!
You're absolutely right—it’s not an ideal approach to create a DDEV project inside another DDEV project. However, since it was allowed previously, I considered that scenario.
Comment #6
stasadev commentedI updated my code in the issue summary, which should better handle the edge cases described.
Comment #8
rkollerI've discussed the topic with @stasadev via Discord and tested the proposed resolution. with previous proposed resolutions i ran into issues. the initial proposal i sent over had the problem with three folders of drupal-cms the first launched launcher gets drupal-cms as the project name , the second drupal-cms-1 and for the third it refused to set up a project. and the problem with trying to setup a new instances of drupal cms inside the subfolder of an already installed instance of drupal cms ran on DDEV HEAD wasit behaved correctly, by running ddev config (updating the config the project in the parent folder) and then run ddev start (ddev restart in this case strictly speaking), but as i said not apparent to the user.
with the proposed resolution @stasadev posted i was able to create several instances of drupal-cms across my file system, and it properly counts up, i'Ve stopped after drupal-cms-4... the first project was drupal-cms, the second drupal-cms-1.. and when you try to execute the launcher in a directory where its folder is a subfolder of another already registered drupal project the script now just exits and we agreed on providing some context with
You're trying to set up a Drupal CMS inside an existing project in $(pwd), refusing to do it.... that way the user knows what goes wrong and where to look for the parent project by the path that was provided.Comment #9
stasadev commentedComment #10
rkollerthank you! looks good to me. tested another round with the latest changes. created three instances in three different directories which results in drupal-cms, drupal-cms-1, and drupal-cms-2 projects. and when i copied another install inside a subdirectory of one of those three instances i get:
i leave the issue at needs reviews. but from a manual testing perspective this looks good to go and is an improvement.
Comment #11
rkollerunassigned @stasadev since the issue is ready for review (but asked him and made sure he isnt still working on anything)
Comment #12
kristen polI just tested as follows:
composer create-project drupal/cmswith the old scriptcms-3which makes sense based on the old logiccmswhich makes sense because it's using an exact name matchcms-1cms-2cms-4cms-2and installed again and it createdcms-2as expectedSo, the logic works, but I'm not sure if the code looks okay.
Comment #13
kristen polActually... this needs work because, when I use a different directory name, then it doesn't work:
We should be able to use a different name.
Comment #14
stasadev commented> when I use a different directory name, then it doesn't work
Thanks, should be fine now.
Changes:
- check for underscores and spaces in the directory name
- early check for DDEV v1.24.0 before doing any action
- remove fragile code that depended on `jq` and `docker`
- replace infinite "while" loop with "for 1 to 1000"
Comment #15
pameeela commentedComment #16
rfayI do have a question here that is a bit meta. Why are we using a complex launcher script that requires all this maintenance, when we can do the exact same thing with `ddev composer create` as with the suggested `composer create-project`? The DDEV-recommended technique in https://ddev.readthedocs.io/en/stable/users/quickstart/#drupal-drupal-cms is to do the `ddev composer create` and it's very easy. I'm baffled why we need a launcher script and a zipball. Or perhaps if we want a launcher script, why doesn't it just do the `ddev composer create` ?
I'm happy to open another issue, but this one could be put aside if we just went with simpler instructions.
Comment #17
phenaproximaThe changes look okay but there are some out-of-scope things that should be done in other issues.
@rfay's point is also a good one - we have to find the right balance between making this easy for people and driving ourselves nuts. Given the way things look on new.drupal.org -- where the DDEV instructions are linked to on a separate page -- the value of the launcher script is looking very dubious indeed. This is a product decision, though, and therefore needs to be run by @pameeela.
So, assigning to her and postponing any further action until we've decided if this launcher script is worth it to us.
Comment #18
phenaproximaComment #19
pameeela commentedI'm inclined to agree now that there is dedicated Drupal CMS documentation for DDEV. The existence of this launcher script is (I think) also causing some folks to think that DDEV is the only way to install from the zip.
So probably we need to create a new issue to remove the launcher script in the next release, and coordinate all necessary updates to documentation. As part of this, we should also make it clear that DDEV is a suggestion and not a requirement.
And then we can close this as won't fix.
Comment #20
phenaproximaThe launcher was removed in #3501605: Replace the launcher script with better documentation, so this is not gonna get fixed. :)