Skip to content

build: add ddev typo3 multi version extension - #6

Merged
konradmichalik merged 2 commits into
mainfrom
ddev-typo3-multi-version-extension
Jul 31, 2025
Merged

build: add ddev typo3 multi version extension#6
konradmichalik merged 2 commits into
mainfrom
ddev-typo3-multi-version-extension

Conversation

@konradmichalik

@konradmichalik konradmichalik commented Jul 31, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features

    • Introduced automated multi-version TYPO3 setup and management within DDEV, supporting versions 11, 12, and 13.
    • Added scripts for easy installation, configuration, and command execution for each TYPO3 version.
    • Provided a web-based overview page displaying project details, admin credentials, and available commands.
    • Included demo site package and configuration for testing purposes.
  • Chores

    • Added configuration files for Apache, Docker Compose, and DDEV to streamline environment setup.
    • Updated .gitignore to exclude build artifacts.
    • Added placeholder files to ensure necessary directories are tracked.

@coderabbitai

coderabbitai Bot commented Jul 31, 2025

Copy link
Copy Markdown

Walkthrough

This change set introduces a comprehensive multi-version TYPO3 setup automation within a DDEV environment. It adds utility Bash scripts for managing TYPO3 installations, version handling, and configuration tasks, along with version-specific installation scripts for TYPO3 11, 12, and 13. A PHP template generates an overview page with project status and admin credentials. Apache configuration files are provided for both specific and wildcard subdomains, supporting HTTP and HTTPS. The Docker Compose and DDEV YAML configuration files define environment variables, hostnames, and post-start hooks for the TYPO3 setup. Additional files include a .gitignore update, addon manifest, a placeholder data file, and a demo sitepackage with its own composer.json and service configuration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~18–25 minutes

  • Complexity: The review involves multiple new scripts, configuration files, and a PHP template, each with their own logic, environment variable usage, and integration points. While most changes are additive and modular, the interplay between scripts, environment configuration, and DDEV/Docker setup requires careful inspection.
  • Scope: Approximately 18 new files spanning Bash, PHP, YAML, Apache config, and documentation.
  • Volume: The scripts and configurations are medium in length and complexity, with some requiring validation of workflows and correct handling of environment variables, error cases, and version-specific logic.
  • Estimated time: A reviewer familiar with DDEV, TYPO3, and Docker would need roughly 18–25 minutes to thoroughly review the changes, focusing on correctness, maintainability, and integration between the components.

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ddev-typo3-multi-version-extension

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 32

♻️ Duplicate comments (1)
.ddev/apache/20.conf (1)

9-12: Update deprecated Apache syntax for directory access control.

Same issue as in 10.conf - the Allow from All directive is deprecated Apache 2.2 syntax. Use Require all granted for Apache 2.4+ compatibility.

Apply this diff to use modern Apache 2.4 syntax:

     <Directory "/var/www/html/.Build">
-  		AllowOverride All
-  		Allow from All
+        AllowOverride All
+        Require all granted
 	</Directory>

Also applies to: 31-34

🧹 Nitpick comments (10)
Tests/.typo3-setup/packages/sitepackage/README.md (2)

1-1: Fix Markdown heading style – add a space after the hash

markdownlint flagged MD018. Change to make the heading valid Markdown:

-#ddev-generated
+# ddev-generated

2-6: Minor consistency nits

  1. Consider adding an empty line after the main heading for readability.
  2. The sentence “Adjust them to your needs …” – pronoun reference (“them”) might be unclear; maybe “Adjust the files to your needs …”.

Purely editorial – no functional impact.

Tests/.typo3-setup/packages/sitepackage/Configuration/Services.yaml (1)

1-6: Terminate file with newline to satisfy YAMLlint

Tool reports no new line character at the end of file (YAMLlint). Add a trailing newline:

 services:
   _defaults:
     autowire: true
     autoconfigure: true
     public: false
+
.ddev/config.typo3-setup.yaml (2)

7-9: Normalise YAML indentation for readability

Indentation jumps (0 → 3 → 4 spaces). While technically valid, 2-space steps are standard in DDEV samples and help avoid accidental mix-ups.

 hooks:
-   post-start:
-    - exec: mkdir -p /var/www/html/.Build/ && cp /var/www/html/.ddev/.typo3-setup/templates/* /var/www/html/.Build/
+  post-start:
+    - exec: mkdir -p /var/www/html/.Build && cp /var/www/html/.ddev/.typo3-setup/templates/* /var/www/html/.Build/

(Also removed trailing slash after .Build in first mkdir – redundant.)


9-9: Quote the glob to prevent unexpected shell expansion

If the template directory is empty the unquoted * expands to itself and cp fails. Safer:

-    - exec: mkdir -p /var/www/html/.Build && cp /var/www/html/.ddev/.typo3-setup/templates/* /var/www/html/.Build/
+    - exec: mkdir -p /var/www/html/.Build && cp /var/www/html/.ddev/.typo3-setup/templates/"*" /var/www/html/.Build/ || true

|| true keeps the hook from aborting if nothing matches.

.ddev/commands/web/.install-11 (1)

1-23: Consider using a template for version-specific install scripts.

The .install-11, .install-12, and .install-13 scripts are nearly identical except for version numbers and package versions. This duplication could lead to maintenance issues.

Consider creating a single parameterized script or template to reduce code duplication and ensure consistency across versions.

.ddev/commands/web/all (1)

14-14: Consider making TYPO3 path configurable.

The hardcoded path /var/www/html/.test/ could be made configurable through environment variables for better flexibility.

-    TYPO3_PATH="/var/www/html/.test/${version}"
+    TYPO3_BASE_PATH="${TYPO3_BASE_PATH:-/var/www/html/.test}"
+    TYPO3_PATH="${TYPO3_BASE_PATH}/${version}"
.ddev/docker-compose.typo3-setup.yaml (3)

5-7: Consider making extension-specific values configurable.

The hardcoded extension key, name, and package name (xima_typo3_internal_news, xima-typo3-internal-news, xima/xima-typo3-internal-news) make this configuration specific to one project, reducing reusability.

Consider using placeholder values or making these configurable:

-            - EXTENSION_KEY=xima_typo3_internal_news
-            - EXTENSION_NAME=xima-typo3-internal-news
-            - PACKAGE_NAME=xima/xima-typo3-internal-news
+            - EXTENSION_KEY=${EXTENSION_KEY:-your_extension_key}
+            - EXTENSION_NAME=${EXTENSION_NAME:-your-extension-name}
+            - PACKAGE_NAME=${PACKAGE_NAME:-vendor/your-extension-name}

19-19: Document security considerations for default credentials.

The configuration uses default development credentials (admin/Password1!) which are appropriate for development but should be clearly documented as such.

Add a comment clarifying these are development-only credentials:

+            # Development-only credentials - change for production use
             - TYPO3_INSTALL_ADMIN_PASSWORD=Password1!

Also applies to: 31-31


20-20: Use consistent site/project naming.

The site name contains extension-specific references that should align with the configurable extension values suggested earlier.

-            - TYPO3_INSTALL_SITE_NAME=EXT:xima-typo3-internal-news Dev Environment
-            - TYPO3_PROJECT_NAME=EXT:xima-typo3-internal-news Dev Environment
+            - TYPO3_INSTALL_SITE_NAME=EXT:${EXTENSION_NAME:-your-extension} Dev Environment
+            - TYPO3_PROJECT_NAME=EXT:${EXTENSION_NAME:-your-extension} Dev Environment

Also applies to: 32-32

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dfc4d61 and 98674b9.

📒 Files selected for processing (20)
  • .ddev/.typo3-setup/scripts/utils.sh (1 hunks)
  • .ddev/.typo3-setup/templates/index.php (1 hunks)
  • .ddev/addon-metadata/typo3-multi-version-extension/manifest.yaml (1 hunks)
  • .ddev/apache/10.conf (1 hunks)
  • .ddev/apache/20.conf (1 hunks)
  • .ddev/commands/web/.install-11 (1 hunks)
  • .ddev/commands/web/.install-12 (1 hunks)
  • .ddev/commands/web/.install-13 (1 hunks)
  • .ddev/commands/web/11 (1 hunks)
  • .ddev/commands/web/12 (1 hunks)
  • .ddev/commands/web/13 (1 hunks)
  • .ddev/commands/web/all (1 hunks)
  • .ddev/commands/web/install (1 hunks)
  • .ddev/config.typo3-setup.yaml (1 hunks)
  • .ddev/docker-compose.typo3-setup.yaml (1 hunks)
  • .gitignore (1 hunks)
  • Tests/.typo3-setup/data/.gitkeep (1 hunks)
  • Tests/.typo3-setup/packages/sitepackage/Configuration/Services.yaml (1 hunks)
  • Tests/.typo3-setup/packages/sitepackage/README.md (1 hunks)
  • Tests/.typo3-setup/packages/sitepackage/composer.json (1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
Tests/.typo3-setup/packages/sitepackage/Configuration/Services.yaml

[error] 6-6: no new line character at the end of file

(new-line-at-end-of-file)

🪛 markdownlint-cli2 (0.17.2)
Tests/.typo3-setup/packages/sitepackage/README.md

1-1: No space after hash on atx style heading

(MD018, no-missing-space-atx)

🪛 Shellcheck (0.10.0)
.ddev/.typo3-setup/scripts/utils.sh

[warning] 105-105: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)


[warning] 166-166: Declare and assign separately to avoid masking return values.

(SC2155)


[warning] 202-202: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: SCA
🔇 Additional comments (11)
.gitignore (1)

14-14: Addition looks good – .Build/ should indeed be ignored

Keeping generated TYPO3 instances and artefacts out of VCS is the right call.

Tests/.typo3-setup/data/.gitkeep (1)

1-1: OK to keep directory under version control

Nothing to change – conventional use of a .gitkeep placeholder.

.ddev/addon-metadata/typo3-multi-version-extension/manifest.yaml (1)

8-8: Please verify the manifest path
I ran a directory scan and didn’t find any .typo3-setup folder under .ddev/addon-metadata/typo3-multi-version-extension. The entry in manifest.yaml:

- .typo3-setup/Tests/.typo3-setup/

may be pointing to a non-existent or mis-nested path.
• Confirm whether this nested .typo3-setup/Tests/.typo3-setup/ directory actually exists in your source (or was omitted by accident).
• If the second .typo3-setup/ is redundant, update the manifest to the correct relative path.

Tests/.typo3-setup/packages/sitepackage/composer.json (1)

1-26: LGTM! Well-structured TYPO3 extension composer.json.

The composer.json is properly configured for a TYPO3 extension with correct type, extension key, PSR-4 autoloading, and appropriate license. The placeholder author information is acceptable for a test package.

.ddev/commands/web/13 (1)

13-15: TYPO3 binary handling aligns with version differences
Version 11 uses vendor/bin/typo3cms${command:5}, whereas versions 12 and 13 use vendor/bin/${command} (i.e. typo3). This matches the upstream change from the legacy typo3cms binary to the unified typo3 console in newer TYPO3 releases—no changes required.

.ddev/apache/10.conf (1)

13-21: LGTM! Well-configured virtual hosts with proper HTTPS handling.

The rewrite rules correctly handle HTTPS protocol detection and trailing slash redirects. The SSL configuration and logging setup are appropriate for a DDEV environment.

Also applies to: 31-38

.ddev/commands/web/install (1)

8-26: LGTM! Clean argument handling and version validation.

The script properly handles different argument scenarios (none, specific version, "all") and leverages utility functions for version validation and retrieval.

.ddev/apache/20.conf (1)

15-16: LGTM! Effective subdomain routing for multi-version setup.

The rewrite rules correctly capture subdomains and route them to version-specific directories under .Build. The pattern ^([a-z0-9-]+)\.xima-typo3-internal-news\.ddev\.site$ appropriately matches TYPO3 version subdomains and maps to the expected directory structure.

Also applies to: 37-38

.ddev/.typo3-setup/templates/index.php (1)

48-56: LGTM! Effective version detection and URL generation.

The logic for checking installed versions and generating appropriate links is well-implemented. The conditional display based on directory existence provides good user feedback.

.ddev/.typo3-setup/scripts/utils.sh (2)

11-19: LGTM! Well-structured version management functions.

The functions for getting and validating TYPO3 versions are well-implemented with proper error handling and clear return values. The use of arrays and sorting for version management is appropriate.

Also applies to: 25-38


289-316: LGTM! Clean and reusable message function.

The colored message function is well-implemented with proper color handling and fallback for unsupported colors. Good use of ANSI escape codes.

# After that, it imports data and updates TYPO3.
function post_setup() {
message blue "Post Setup for TYPO3 $VERSION"
cd $BASE_PATH

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling for directory changes.

Static analysis correctly identifies that cd commands should include error handling to prevent silent failures that could lead to operations in wrong directories.

Apply this diff to add proper error handling:

-  cd $BASE_PATH
+  cd "$BASE_PATH" || { message red "Failed to change to directory $BASE_PATH"; exit 1; }
-    cd $BASE_PATH
+    cd "$BASE_PATH" || { message red "Failed to change to directory $BASE_PATH"; exit 1; }

Also applies to: 202-202

🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 105-105: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)

🤖 Prompt for AI Agents
In .ddev/.typo3-setup/scripts/utils.sh at line 105, the cd command lacks error
handling which can cause silent failures if the directory change fails. Modify
the cd command to include error checking by appending a conditional that exits
the script or handles the error if the cd fails. Also apply the same fix to
lines 202-202 where a similar cd command is used without error handling.

else
export TYPO3_BIN="$BASE_PATH/vendor/bin/typo3"
fi
mysql -uroot -proot -e "DROP DATABASE IF EXISTS $DATABASE"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Security concern: Hardcoded database credentials.

The script uses hardcoded database credentials (-uroot -proot) which poses security risks. Consider using environment variables or configuration files.

Apply this diff to use environment variables:

-    mysql -uroot -proot -e "DROP DATABASE IF EXISTS $DATABASE"
+    mysql -u"${DB_USER:-root}" -p"${DB_PASSWORD:-root}" -e "DROP DATABASE IF EXISTS $DATABASE"
-  mysql -h db -u root -p"root" -e "CREATE DATABASE $DATABASE;"
+  mysql -h db -u "${DB_USER:-root}" -p"${DB_PASSWORD:-root}" -e "CREATE DATABASE $DATABASE;"

Also applies to: 271-271

🤖 Prompt for AI Agents
In .ddev/.typo3-setup/scripts/utils.sh at lines 156 and 271, the script uses
hardcoded MySQL credentials (-uroot -proot), which is insecure. Replace these
hardcoded credentials with environment variables by referencing variables like
$DB_USER and $DB_PASSWORD instead. Ensure these environment variables are set
securely outside the script to avoid exposing sensitive information.

local EXCLUSIONS_ARRAY=()
IFS=' ' read -r -a EXCLUSIONS_ARRAY <<< "$EXCLUSIONS"
for item in ./*; do
local base_name=$(basename "$item")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix variable assignment to avoid masking return values.

Static analysis correctly identifies that declaring and assigning in the same line can mask command failures.

Apply this diff to separate declaration and assignment:

-        local base_name=$(basename "$item")
+        local base_name
+        base_name=$(basename "$item")
🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 166-166: Declare and assign separately to avoid masking return values.

(SC2155)

🤖 Prompt for AI Agents
In .ddev/.typo3-setup/scripts/utils.sh at line 166, the variable assignment
combines declaration and assignment, which can mask command failures. To fix
this, declare the variable first without assignment, then assign the output of
the basename command on the next line. This separation ensures any errors from
the command are not masked by the assignment.

Comment on lines +248 to +251
sed -i "/'deprecations'/,/^[[:space:]]*'disabled' => true,/s/'disabled' => true,/'disabled' => false,/" /var/www/html/.Build/$VERSION/public/typo3conf/LocalConfiguration.php

sed -i -e "s/base: ht\//base: \//g" /var/www/html/.Build/$VERSION/config/sites/main/config.yaml
sed -i -e 's/base: \/en\//base: \//g' /var/www/html/.Build/$VERSION/config/sites/main/config.yaml

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve robustness of sed commands with better error handling.

The sed commands for configuration file modifications should include validation that the target files exist and the operations succeed.

Add file existence checks and error handling:

+  if [[ -f "/var/www/html/.Build/$VERSION/public/typo3conf/LocalConfiguration.php" ]]; then
     sed -i "/'deprecations'/,/^[[:space:]]*'disabled' => true,/s/'disabled' => true,/'disabled' => false,/" /var/www/html/.Build/$VERSION/public/typo3conf/LocalConfiguration.php
+  fi

+  if [[ -f "/var/www/html/.Build/$VERSION/config/sites/main/config.yaml" ]]; then
     sed -i -e "s/base: ht\//base: \//g" /var/www/html/.Build/$VERSION/config/sites/main/config.yaml
     sed -i -e 's/base: \/en\//base: \//g' /var/www/html/.Build/$VERSION/config/sites/main/config.yaml
+  fi

Also applies to: 261-264, 275-275

🤖 Prompt for AI Agents
In .ddev/.typo3-setup/scripts/utils.sh around lines 248 to 251, the sed commands
modify configuration files without checking if the target files exist or if the
commands succeed. To fix this, add checks before each sed command to verify the
file exists, and after running sed, check the command's exit status. If the file
does not exist or the sed command fails, output an error message and handle the
failure appropriately. Apply the same pattern of file existence checks and error
handling to the sed commands in lines 261-264 and 275.

Comment on lines +7 to +10
$extensionKey = getenv('EXTENSION_NAME');
$typo3AdminUser = getenv('TYPO3_INSTALL_ADMIN_USER');
$typo3AdminPassword = getenv('TYPO3_INSTALL_ADMIN_PASSWORD');
$supportedVersions = explode(' ', getenv('TYPO3_VERSIONS'));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add input validation and XSS protection for environment variables.

Environment variables are directly output to HTML without sanitization, creating potential XSS vulnerabilities. Also, there's no validation that these variables are set.

Apply this diff to add proper sanitization and validation:

-$extensionKey = getenv('EXTENSION_NAME');
-$typo3AdminUser = getenv('TYPO3_INSTALL_ADMIN_USER');
-$typo3AdminPassword = getenv('TYPO3_INSTALL_ADMIN_PASSWORD');
-$supportedVersions = explode(' ', getenv('TYPO3_VERSIONS'));
+$extensionKey = htmlspecialchars(getenv('EXTENSION_NAME') ?: 'unknown', ENT_QUOTES, 'UTF-8');
+$typo3AdminUser = htmlspecialchars(getenv('TYPO3_INSTALL_ADMIN_USER') ?: 'admin', ENT_QUOTES, 'UTF-8');
+$typo3AdminPassword = htmlspecialchars(getenv('TYPO3_INSTALL_ADMIN_PASSWORD') ?: '', ENT_QUOTES, 'UTF-8');
+$supportedVersions = array_filter(explode(' ', getenv('TYPO3_VERSIONS') ?: ''));
🤖 Prompt for AI Agents
In .ddev/.typo3-setup/templates/index.php around lines 7 to 10, environment
variables are used directly without validation or sanitization, risking XSS
vulnerabilities and errors if variables are missing. Add checks to ensure each
environment variable is set and not empty, and sanitize their values before
outputting to HTML by applying appropriate escaping functions to prevent XSS
attacks.

Comment thread .ddev/commands/web/all
## Usage: all
## Example: "ddev all composer du -o" or "ddev all typo3 cache:flush"

. .ddev/.typo3-setup/scripts/utils.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling for sourced utility script.

Consistent with other scripts, this should verify the utility script exists before sourcing it.

-. .ddev/.typo3-setup/scripts/utils.sh
+if [ ! -f ".ddev/.typo3-setup/scripts/utils.sh" ]; then
+    echo "Error: Required utility script not found at .ddev/.typo3-setup/scripts/utils.sh"
+    exit 1
+fi
+. .ddev/.typo3-setup/scripts/utils.sh
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
. .ddev/.typo3-setup/scripts/utils.sh
if [ ! -f ".ddev/.typo3-setup/scripts/utils.sh" ]; then
echo "Error: Required utility script not found at .ddev/.typo3-setup/scripts/utils.sh"
exit 1
fi
. .ddev/.typo3-setup/scripts/utils.sh
🤖 Prompt for AI Agents
In .ddev/commands/web/all at line 8, the script sources the utility script
without checking if it exists, which can cause errors if the file is missing.
Add a conditional check to verify that the utility script file exists before
sourcing it, and handle the error appropriately if it does not, to maintain
consistency with other scripts.

Comment thread .ddev/commands/web/all
Comment on lines +16 to +22
if [[ $command == typo3* ]]; then
if [[ $version == 11 ]]; then
tempCommand="/usr/bin/php vendor/bin/typo3cms${command:5}"
else
tempCommand="/usr/bin/php vendor/bin/${command}"
fi
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix uninitialized variable for non-TYPO3 commands.

The $tempCommand variable is only set for commands starting with "typo3", but is used unconditionally on line 25, causing issues for other commands.

+        tempCommand="$command"
         if [[ $command == typo3* ]]; then
             if [[ $version == 11 ]]; then
                 tempCommand="/usr/bin/php vendor/bin/typo3cms${command:5}"
             else
                 tempCommand="/usr/bin/php vendor/bin/${command}"
             fi
         fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [[ $command == typo3* ]]; then
if [[ $version == 11 ]]; then
tempCommand="/usr/bin/php vendor/bin/typo3cms${command:5}"
else
tempCommand="/usr/bin/php vendor/bin/${command}"
fi
fi
tempCommand="$command"
if [[ $command == typo3* ]]; then
if [[ $version == 11 ]]; then
tempCommand="/usr/bin/php vendor/bin/typo3cms${command:5}"
else
tempCommand="/usr/bin/php vendor/bin/${command}"
fi
fi
🤖 Prompt for AI Agents
In .ddev/commands/web/all around lines 16 to 22, the variable $tempCommand is
only assigned when the command starts with "typo3", but it is used later
regardless of the command type. To fix this, initialize $tempCommand with a
default value before the conditional block or add an else clause to handle
non-TYPO3 commands, ensuring $tempCommand is always set before use.

Comment thread .ddev/commands/web/all
Comment on lines +23 to +25
cd $TYPO3_PATH
message magenta "[TYPO3 v${version}] ${tempCommand}"
$tempCommand

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for directory change and command execution.

The script changes directory and executes commands without error handling, which could mask failures.

-        cd $TYPO3_PATH
-        message magenta "[TYPO3 v${version}] ${tempCommand}"
-        $tempCommand
+        if ! cd "$TYPO3_PATH"; then
+            message red "Failed to change to directory: $TYPO3_PATH"
+            continue
+        fi
+        message magenta "[TYPO3 v${version}] ${tempCommand}"
+        if ! $tempCommand; then
+            message red "Command failed for TYPO3 v${version}: ${tempCommand}"
+        fi
🤖 Prompt for AI Agents
In .ddev/commands/web/all around lines 23 to 25, add error handling after
changing the directory and executing the command. Check if the cd command
succeeds before proceeding, and verify the execution of $tempCommand, capturing
any errors or non-zero exit codes. If an error occurs, output an appropriate
error message and exit the script to prevent masking failures.

if [ "$TYPO3" == "all" ]; then
mapfile -t versions < <(get_supported_typo3_versions)
for version in "${versions[@]}"; do
.ddev/commands/web/.install-$version

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for script execution.

The script executions don't handle potential failures. Consider adding error checking to ensure each installation script completes successfully.

Apply this diff to add error handling:

-        .ddev/commands/web/.install-$version
+        if ! .ddev/commands/web/.install-$version; then
+            echo "Failed to install TYPO3 version $version"
+            exit 1
+        fi
-    ".ddev/commands/web/.install-$TYPO3"
+    if ! ".ddev/commands/web/.install-$TYPO3"; then
+        echo "Failed to install TYPO3 version $TYPO3"
+        exit 1
+    fi

Also applies to: 25-25

🤖 Prompt for AI Agents
In .ddev/commands/web/install at lines 15 and 25, the script executions lack
error handling for potential failures. Modify the script to check the exit
status after each installation script runs, and if a script fails, output an
error message and stop further execution to prevent cascading errors.

Comment on lines +11 to +23
# TYPO3 v11 and v12 config
- TYPO3_INSTALL_DB_DRIVER=mysqli
- TYPO3_INSTALL_DB_USER=root
- TYPO3_INSTALL_DB_PASSWORD=root
- TYPO3_INSTALL_DB_HOST=db
- TYPO3_INSTALL_DB_UNIX_SOCKET=
- TYPO3_INSTALL_DB_USE_EXISTING=0
- TYPO3_INSTALL_ADMIN_USER=admin
- TYPO3_INSTALL_ADMIN_PASSWORD=Password1!
- TYPO3_INSTALL_SITE_NAME=EXT:xima-typo3-internal-news Dev Environment
- TYPO3_INSTALL_SITE_SETUP_TYPE=site
- TYPO3_INSTALL_WEB_SERVER_CONFIG=apache

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Address inconsistent environment variable naming between TYPO3 versions.

The configuration uses different variable naming conventions for TYPO3 v11/v12 (TYPO3_INSTALL_*) versus v13 (TYPO3_*), which could cause confusion and maintenance issues.

Consider standardizing the variable names or clearly documenting why different naming conventions are required for different TYPO3 versions.

Also applies to: 24-33

🤖 Prompt for AI Agents
In .ddev/docker-compose.typo3-setup.yaml around lines 11 to 33, the environment
variables use inconsistent naming conventions between TYPO3 v11/v12
(TYPO3_INSTALL_*) and v13 (TYPO3_*), which can cause confusion. To fix this,
standardize the environment variable names across all TYPO3 versions if
possible, or add clear comments explaining the reason for the different naming
conventions to improve maintainability and clarity.

@konradmichalik
konradmichalik merged commit 7d78ad6 into main Jul 31, 2025
1 of 2 checks passed
@konradmichalik
konradmichalik deleted the ddev-typo3-multi-version-extension branch August 1, 2025 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant