Build workspace from scratch with no ready-made-envs #127
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
WalkthroughThis update focuses on enhancing the Drupal development environment setup. It includes changes to project configurations, scripts for setting up Drupal and installing essential packages, and updates to README instructions. The modifications aim to streamline the process, improve performance, and ensure compatibility with the latest versions of Drupal and MariaDB. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 6
Files selected for processing (6)
- .ddev/config.yaml (2 hunks)
- .gitpod.yml (1 hunks)
- .gitpod/README.md (1 hunks)
- .gitpod/drupal/drupalpod-setup.sh (10 hunks)
- .gitpod/drupal/install-essential-packages.sh (1 hunks)
- .gitpod/utils/set-base-environment.sh (1 hunks)
Files skipped from review due to trivial changes (4)
- .ddev/config.yaml
- .gitpod.yml
- .gitpod/README.md
- .gitpod/drupal/install-essential-packages.sh
Additional comments (Suppressed): 1
.gitpod/drupal/drupalpod-setup.sh (1)
- 36-45: The comment on line 38 indicates that
admin_toolbaris not compatible with Drupal 10, but the code to unsetDP_EXTRA_ADMIN_TOOLBARhas been commented out. If this is intentional, consider updating or removing the comment to avoid confusion.
| #!/usr/bin/env bash | ||
| if [ -n "$DEBUG_SCRIPT" ]; then | ||
| set -x | ||
| fi | ||
|
|
||
| # Load default envs | ||
| export "$(grep -v '^#' "$GITPOD_REPO_ROOT"/.env | xargs -d '\n')" | ||
|
|
||
| # Clone Drupal core repo | ||
| mkdir -p "${GITPOD_REPO_ROOT}"/repos | ||
| cd "${GITPOD_REPO_ROOT}"/repos && time git clone https://git.drupalcode.org/project/drupal.git |
There was a problem hiding this comment.
The script seems to be well-written and follows good practices. However, there is no error handling in case the environment variables file is not found or if the git clone operation fails. Adding some error handling would make the script more robust.
# Load default envs
if [ -f "$GITPOD_REPO_ROOT"/.env ]; then
export "$(grep -v '^#' "$GITPOD_REPO_ROOT"/.env | xargs -d '\n')"
else
echo "Environment file not found"
exit 1
fi
# Clone Drupal core repo
mkdir -p "${GITPOD_REPO_ROOT}"/repos
cd "${GITPOD_REPO_ROOT}"/repos && time git clone https://git.drupalcode.org/project/drupal.git || { echo "Git clone failed"; exit 1; }| # Run 'ddev start' in the background and capture its PID | ||
| ddev start & | ||
| ddev_start=$! |
There was a problem hiding this comment.
The script starts a ddev process in the background and captures its PID. However, there is no error handling if the ddev start command fails to execute properly. Consider adding error handling to ensure that the script doesn't continue executing if ddev start fails.
ddev start &
+ if [ $? -ne 0 ]; then
+ echo "Failed to start ddev"
+ exit 1
+ fi
ddev_start=$!| if [ -z "$GITPOD_HEADLESS" ]; then | ||
| mkdir -p ~/.ssh | ||
| host=git.drupal.org | ||
| SSHKey=$(ssh-keyscan $host 2> /dev/null) | ||
| echo "$SSHKey" >> ~/.ssh/known_hosts | ||
| SSHKey=$(ssh-keyscan $host 2>/dev/null) | ||
| echo "$SSHKey" >>~/.ssh/known_hosts | ||
| fi |
There was a problem hiding this comment.
This block of code adds the host git.drupal.org to the known hosts file for SSH. This could potentially be a security risk if git.drupal.org is compromised. It would be safer to manually verify the host key fingerprint for git.drupal.org and add it directly to the known hosts file.
| # Find if requested core version is dev or stable | ||
| d="$DP_CORE_VERSION" | ||
| case $d in | ||
| *.x) | ||
| *.x) | ||
| # If dev - use git checkout origin/* | ||
| checkout_type=origin | ||
| ;; | ||
| *) | ||
| *) | ||
| # stable - use git checkout tags/* | ||
| checkout_type=tags | ||
| ;; | ||
| esac | ||
|
|
||
| # Use origin or tags in git checkout command | ||
| cd "${GITPOD_REPO_ROOT}"/repos/drupal && \ | ||
| git fetch origin && \ | ||
| git fetch --all --tags && \ | ||
| git checkout "$checkout_type"/"$DP_CORE_VERSION" | ||
| cd "${GITPOD_REPO_ROOT}"/repos/drupal && | ||
| git fetch origin && | ||
| git fetch --all --tags && | ||
| git checkout "$checkout_type"/"$DP_CORE_VERSION" |
There was a problem hiding this comment.
The script checks whether the requested core version is dev or stable and sets the checkout_type accordingly. However, there's no error handling if an invalid value is provided for DP_CORE_VERSION. Consider adding error handling to ensure that the script doesn't continue executing if an invalid DP_CORE_VERSION is provided.
| # Remove site that was installed before (for debugging) | ||
| rm -rf "${GITPOD_REPO_ROOT}"/web | ||
| rm -rf "${GITPOD_REPO_ROOT}"/vendor | ||
| rm -f "${GITPOD_REPO_ROOT}"/composer.json | ||
| rm -f "${GITPOD_REPO_ROOT}"/composer.lock |
There was a problem hiding this comment.
These lines remove several directories and files without checking if they exist first. This could lead to errors if the files or directories don't exist. Consider checking if the files or directories exist before trying to remove them.
rm -rf "${GITPOD_REPO_ROOT}"/web
+ if [ -d "${GITPOD_REPO_ROOT}"/web ]; then
+ rm -rf "${GITPOD_REPO_ROOT}"/web
+ fi
rm -rf "${GITPOD_REPO_ROOT}"/vendor
+ if [ -d "${GITPOD_REPO_ROOT}"/vendor ]; then
+ rm -rf "${GITPOD_REPO_ROOT}"/vendor
+ fi
rm -f "${GITPOD_REPO_ROOT}"/composer.json
+ if [ -f "${GITPOD_REPO_ROOT}"/composer.json ]; then
+ rm -f "${GITPOD_REPO_ROOT}"/composer.json
+ fi
rm -f "${GITPOD_REPO_ROOT}"/composer.lock
+ if [ -f "${GITPOD_REPO_ROOT}"/composer.lock ]; then
+ rm -f "${GITPOD_REPO_ROOT}"/composer.lock
+ fi| # Symlink the simpletest folder into the Drupal core git repo. | ||
| # repos/drupal/sites/simpletest -> ../../../web/sites/simpletest | ||
| if [ ! -L "$GITPOD_REPO_ROOT"/repos/drupal/sites/simpletest ]; then | ||
| cd "$GITPOD_REPO_ROOT"/repos/drupal/sites && \ | ||
| ln -s ../../../web/sites/simpletest . | ||
| fi | ||
|
|
||
| # Update composer.lock to allow composer's symlink of repos/drupal/core | ||
| if [ "$ready_made_env_exist" ]; then | ||
| cd "${GITPOD_REPO_ROOT}" && time ddev composer update | ||
| else | ||
| "${GITPOD_REPO_ROOT}"/.gitpod/drupal/install-essential-packages.sh | ||
| cd "$GITPOD_REPO_ROOT"/repos/drupal/sites && | ||
| ln -s ../../../web/sites/simpletest . | ||
| fi |
There was a problem hiding this comment.
The script creates a symbolic link without checking if it already exists. This could lead to errors if the symbolic link already exists. Consider checking if the symbolic link exists before trying to create it.
if [ ! -L "$GITPOD_REPO_ROOT"/repos/drupal/sites/simpletest ]; then
+ if [ ! -e "$GITPOD_REPO_ROOT"/repos/drupal/sites/simpletest ]; then
cd "$GITPOD_REPO_ROOT"/repos/drupal/sites &&
ln -s ../../../web/sites/simpletest .
fi
Rate Limit ExceededYou have exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 15 minutes and 23 seconds before requesting another review. To re-trigger a review, convert this PR to a draft and then mark it as ready for review again. Alternatively, you can push a new commit to this PR. |
Rate Limit ExceededYou have exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 14 minutes and 8 seconds before requesting another review. To re-trigger a review, convert this PR to a draft and then mark it as ready for review again. Alternatively, you can push a new commit to this PR. |
Summary by CodeRabbit
Release Notes for the Pull Request:
.ddev/config.yaml.prepare-environments-gcs.shwithset-base-environment.shin.gitpod.yml..gitpod/README.md..gitpod/drupal/drupalpod-setup.shfor better performance and maintainability..gitpod/drupal/install-essential-packages.sh..gitpod/utils/set-base-environment.shthat clones the Drupal core repository and measures execution time.