Support Drupal 11 SQLite version #13
Conversation
WalkthroughThe changes in this pull request focus on updating the Dockerfile and the build.sh script to handle different versions of the PHP CLI. The primary change is the addition of a conditional installation of SQLite3 for Drupal 11 testing, which is triggered when the CLI version is 8.3 or higher. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Dockerfile
participant Docker
CLI->>Dockerfile: Provide CLI version
Dockerfile->>Dockerfile: Check CLI version
Dockerfile->>Dockerfile: Set INSTALL_SQLITE variable
Dockerfile->>Docker: Build image with INSTALL_SQLITE
Docker->>Dockerfile: Install SQLite3 if INSTALL_SQLITE is true
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
build.sh (3)
6-7: Add version format validationWhile the default version is correctly set, the script should validate the input version format to prevent invalid versions from being passed to Docker.
CLI_VERSION=${1:-8.3} +# Validate version format (e.g., 8.3, 7.4, etc.) +if ! echo "$CLI_VERSION" | grep -qE '^[0-9]+\.[0-9]+$'; then + echo "Error: Invalid version format. Expected format: X.Y (e.g., 8.3)" + exit 1 +fi
8-12: Enhance comment clarity for SQLite installationThe current comment could be more specific about the purpose and requirements.
-# Install SQLite 3.x for Drupal 11 testing if CLI version >= 8.3 +# Enable SQLite 3.x installation for Drupal 11 Kernel and Functional testing +# This is required for PHP CLI versions >= 8.3 as per https://www.drupal.org/project/drupal/issues/3346338
9-12: Consider making SQLite version requirement configurableCurrently, the SQLite installation is hardcoded to PHP 8.3+. Consider making this more flexible for future requirements.
You could introduce an environment variable or configuration file to specify PHP versions requiring SQLite, making it easier to update without changing the script:
-if [ "$(echo "${CLI_VERSION} >= 8.3" | bc -l)" -eq 1 ]; then +# Default versions that need SQLite +SQLITE_REQUIRED_VERSIONS=${SQLITE_REQUIRED_VERSIONS:-"8.3,8.4"} + +if echo "$SQLITE_REQUIRED_VERSIONS" | tr ',' '\n' | grep -q "^${CLI_VERSION}$"; then
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
build.sh(1 hunks)src/Dockerfile(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Dockerfile
🔇 Additional comments (1)
build.sh (1)
14-14: Verify build arguments in Dockerfile
The build arguments look correct, but let's verify their usage in the Dockerfile.
✅ Verification successful
Build arguments are properly declared and used in Dockerfile
The verification confirms that:
CLI_VERSIONandINSTALL_SQLITEare properly declared as build argumentsINSTALL_SQLITEis effectively used in a conditional SQLite installation block- While
CLI_VERSIONappears to use a template value ({{cli_version}}), let's verify its actual usage
Build arguments are properly configured and utilized
The build arguments in the Dockerfile are correctly implemented:
CLI_VERSIONis used to specify the base image version (FROM docksal/cli:${CLI_VERSION})INSTALL_SQLITEcontrols the conditional installation of SQLite in a dedicated RUN block
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the Dockerfile properly uses the build arguments
# Check if build arguments are properly declared and used
echo "Checking build arguments in Dockerfile..."
rg -A 2 'ARG (CLI_VERSION|INSTALL_SQLITE)' src/Dockerfile
# Check SQLite installation logic
echo "Checking SQLite installation logic..."
rg -A 5 'INSTALL_SQLITE' src/Dockerfile
Length of output: 1165
Script:
#!/bin/bash
# Check how CLI_VERSION is used in the Dockerfile
rg -A 3 'CLI_VERSION' src/Dockerfile
# Also check if there are any FROM statements that might use it
rg '^FROM' src/Dockerfile
Length of output: 335
This is a downstream fix of docksal/service-cli#327
There's some talk of removing this version requirement in https://www.drupal.org/project/drupal/issues/3346338 but until then, this is necessary for running pre-build Drupal tests (Kernel, and Functional).
Summary by CodeRabbit
New Features
Chores