Skip to content

fix: stop all old PHP-FPM services before upgrade and remove unreachable branch in php-update.sh - #228

Merged
PDowney merged 3 commits into
masterfrom
copilot/stop-all-old-php-versions
May 2, 2026
Merged

fix: stop all old PHP-FPM services before upgrade and remove unreachable branch in php-update.sh#228
PDowney merged 3 commits into
masterfrom
copilot/stop-all-old-php-versions

Conversation

Copilot AI commented May 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Two bugs in scripts/update/php-update.sh: only the first detected old PHP-FPM service was stopped before the upgrade, and the final summary contained a dead else branch that could never be reached.

Software Version Updates

Changed Versions

  • scripts/update/php-update.sh — bug fixes only, no version changes

Version Diff

-# Stop old PHP service
-echo "Stopping PHP ${OLD_PHP_VER} service..."
-systemctl stop "php${OLD_PHP_VER}-fpm" 2>/dev/null || true
+# Stop old PHP service(s)
+for old_ver in "${MIGRATION_SOURCE_PHP_VERS[@]}"; do
+    echo "Stopping PHP ${old_ver} service..."
+    systemctl stop "php${old_ver}-fpm" 2>/dev/null || true
+done

-if [[ ${#MIGRATION_SOURCE_PHP_VERS[@]} -gt 0 ]]; then
-    echo "  - Removed PHP version(s): ${MIGRATION_SOURCE_PHP_VERS[*]}"
-else
-    echo "  - Removed PHP ${OLD_PHP_VER} installation"
-fi
+echo "  - Removed PHP version(s): ${MIGRATION_SOURCE_PHP_VERS[*]}"

Verification Checklist

  • Version numbers are correctly formatted
  • Version numbers are stable releases (not beta/RC)
  • No syntax errors in variables file

Notes

Fix 1 — Multi-version service stop: The pre-upgrade stop only called systemctl stop on OLD_PHP_VER (the first element). When multiple old PHP versions are installed, the remaining services stayed running during the upgrade. Now iterates MIGRATION_SOURCE_PHP_VERS[@], consistent with the existing cleanup loop at lines 194–216.

Fix 2 — Dead code removal: The if [[ ${#MIGRATION_SOURCE_PHP_VERS[@]} -gt 0 ]] guard in the final summary is always true — the script exits at lines 49–58 when no old versions are detected, making the else branch permanently unreachable. Replaced with the unconditional echo.

Original prompt 
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"This only stops the first detected old PHP version (OLD_PHP_VER). When multiple old PHP versions are detected, all should be stopped before proceeding with the upgrade. Consider iterating over MIGRATION_SOURCE_PHP_VERS array similar to the cleanup logic at lines 194-216.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -73,9 +73,11 @@\n echo \"Detected PHP installation(s): ${MIGRATION_SOURCE_PHP_VERS[*]}\"\n echo \"Proceeding with upgrade to PHP ${NEW_PHP_VER}...\"\n \n-# Stop old PHP service\n-echo \"Stopping PHP ${OLD_PHP_VER} service...\"\n-systemctl stop \"php${OLD_PHP_VER}-fpm\" 2>/dev/null || true\n+# Stop old PHP service(s)\n+for old_ver in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n+    echo \"Stopping PHP ${old_ver} service...\"\n+    systemctl stop \"php${old_ver}-fpm\" 2>/dev/null || true\n+done\n \n # Install new PHP version\n echo \"Installing PHP ${NEW_PHP_VER}...\"\n"}]},{"message":"The condition on line 241 will always be true because the script exits early (line 49-58) if OLD_PHP_VERS is empty, making the else branch unreachable. The else branch referencing OLD_PHP_VER is therefore dead code and should be removed.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -238,11 +238,7 @@\n echo \"  - Installed PHP ${NEW_PHP_VER} and extensions\"\n echo \"  - Applied EngineScript configuration for PHP ${NEW_PHP_VER}\"\n echo \"  - Updated Nginx configuration\"\n-if [[ ${#MIGRATION_SOURCE_PHP_VERS[@]} -gt 0 ]]; then\n-    echo \"  - Removed PHP version(s): ${MIGRATION_SOURCE_PHP_VERS[*]}\"\n-else\n-    echo \"  - Removed PHP ${OLD_PHP_VER} installation\"\n-fi\n+echo \"  - Removed PHP version(s): ${MIGRATION_SOURCE_PHP_VERS[*]}\"\n echo \"\"\n echo \"=============================================================\"\n echo \"\"\n"}]}]

Copilot AI changed the title [WIP] Fix stopping only first detected old PHP version during upgrades fix: stop all old PHP-FPM services before upgrade and remove unreachable branch in php-update.sh May 2, 2026
Copilot finished work on behalf of PDowney May 2, 2026 04:15
Copilot AI requested a review from PDowney May 2, 2026 04:15
@PDowney
Remove outdated PHP update entry from CHANGELOG
4b31643 
Removed outdated entry for PHP update on 2026-05-02, including fixes for service stop and dead code removal.
@github-actions

github-actions Bot commented May 2, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@sonarqubecloud

sonarqubecloud Bot commented May 2, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@PDowney
PDowney marked this pull request as ready for review May 2, 2026 04:54
Copilot AI review requested due to automatic review settings May 2, 2026 04:54
@PDowney
PDowney merged commit e10ddaa into master May 2, 2026
10 checks passed
@github-actions
github-actions Bot deleted the copilot/stop-all-old-php-versions branch May 2, 2026 04:54
Copilot AI reviewed May 2, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes scripts/update/php-update.sh to better handle multi-version PHP upgrades and removes dead summary logic, improving reliability of PHP-FPM upgrades in EngineScript.

Changes:

  • Stop all detected old phpX.Y-fpm services (not just the first) before installing the new PHP version.
  • Remove an unreachable else branch in the final “Changes made” summary output.

Comment thread scripts/update/php-update.sh
Comment on lines +77 to +80
for old_ver in "${MIGRATION_SOURCE_PHP_VERS[@]}"; do
echo "Stopping PHP ${old_ver} service..."
systemctl stop "php${old_ver}-fpm" 2>/dev/null || true
done
Comment thread scripts/update/php-update.sh
Comment on lines +76 to +80
# Stop old PHP service(s)
for old_ver in "${MIGRATION_SOURCE_PHP_VERS[@]}"; do
echo "Stopping PHP ${old_ver} service..."
systemctl stop "php${old_ver}-fpm" 2>/dev/null || true
done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
+1 more reviewer
@PDowney PDowney PDowney approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@PDowney PDowney

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@PDowney