Fixed provision failing on fallback install from standard profile + added cache rebuild after updb.#2406
Fixed provision failing on fallback install from standard profile + added cache rebuild after updb.#2406AlexSkrypnyk wants to merge 13 commits intomainfrom
Conversation
…ild skip variable.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a skippable cache-rebuild step after database updates, extends profile-based provisioning fallback to accept context flags and perform cleanup when used as a fallback, and updates docs and tests to reflect the new flow and environment flag. Changes
Sequence Diagram(s)sequenceDiagram
participant Ahoy as Ahoy CLI
participant Script as provision.sh
participant Drush as Drush (Drupal)
participant DB as Database
participant Config as Config import
Ahoy->>Script: run provision
Script->>DB: check/import DB dump
alt DB updated (drush updatedb)
Script->>Drush: drush updatedb --no-cache-clear
alt cache rebuild not skipped
Script->>Drush: drush -y cache:rebuild
Drush-->>Script: cache rebuilt
else cache rebuild skipped
Script-->>Ahoy: "Skipped cache rebuild..."
end
end
Script->>Config: verify config unchanged?
alt DB import failed and fallback enabled
Script->>Drush: drush site:install (profile, is_fallback=1)
Script->>DB: delete profile-created shortcut tables (SQL)
Script->>Drush: drush config:delete <specific items>
Script->>Config: drush config:import
else normal path
Script->>Config: drush config:import
end
Config-->>Ahoy: provisioning complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/tests/bats/unit/provision.bats:
- Around line 116-119: Add two Bats tests to cover the new alternate branches:
one that exports VORTEX_PROVISION_CACHE_REBUILD_AFTER_DB_UPDATE_SKIP=1 before
invoking the provision scenario and asserts that "@drush -y cache:rebuild" is
not called and the "Cache was cleared." message is absent; and a second test
that simulates the fallback-to-profile path (ensure the test setup triggers the
profile fallback condition used by the provision logic) and asserts the sequence
continues through "config:import" rather than stopping, verifying the
appropriate log/messages for "config:import". Make sure the new tests reference
the environment variable VORTEX_PROVISION_CACHE_REBUILD_AFTER_DB_UPDATE_SKIP and
the commands "cache:rebuild" and "config:import" so the two new branches
introduced by the PR are exercised.
In `@scripts/vortex/provision.sh`:
- Around line 207-223: The direct profile install path currently adds
--existing-config when has_config is true but does not perform the same
post-install cleanup as the fallback path, so installs fail for profiles
implementing hook_install(); after calling drush site:install (the line invoking
drush site:install "${opts[@]}") ensure the cleanup commands (the drush
entity:delete shortcut_set and the drush config:delete calls for
field.field.node.article.body, field.field.node.article.field_image,
field.storage.node.field_image, taxonomy.vocabulary.tags,
user.role.content_editor) run for direct profile installs as well when
has_config is true (i.e., either move that cleanup block out of the if [
"${is_fallback}" = "1" ] condition or add an equivalent branch that runs when [
"${is_fallback}" != "1" ] && [ "${has_config}" = "1" ]) so profile-created
config is removed before configuration import.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b4772418-0318-4a46-be54-010243544e28
📒 Files selected for processing (4)
.vortex/docs/content/development/variables.mdx.vortex/docs/content/drupal/provision.mdx.vortex/tests/bats/unit/provision.batsscripts/vortex/provision.sh
| # Only use --existing-config for direct profile provisions. | ||
| [ "${is_fallback}" != "1" ] && [ "${has_config}" = "1" ] && opts+=(--existing-config) | ||
|
|
||
| # Database may exist in non-bootstrappable state - truncate it. | ||
| drush sql:drop || true | ||
|
|
||
| drush site:install "${opts[@]}" | ||
|
|
||
| if [ "${is_fallback}" = "1" ] && [ "${has_config}" = "1" ]; then | ||
| note "Removing entities and config created by the profile to prevent conflicts during configuration import." | ||
| drush entity:delete shortcut_set || true | ||
| drush config:delete field.field.node.article.body || true | ||
| drush config:delete field.field.node.article.field_image || true | ||
| drush config:delete field.storage.node.field_image || true | ||
| drush config:delete taxonomy.vocabulary.tags || true | ||
| drush config:delete user.role.content_editor || true | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Drupal 10/11 with Drush, does drush site:install --existing-configsucceed for install profiles that implementhook_install()such asstandard, or is a post-install config:import workflow required instead?
💡 Result:
drush site:install --existing-config does not succeed when the chosen install profile implements hook_install() (including core’s standard profile). Drupal’s installer explicitly blocks “install from existing configuration” in that case with an error like: “The selected profile has a hook_install() implementation and therefore cannot be installed from configuration.” [1], [2].
So for Drupal 10/11, you must either:
- Use a profile without
hook_install()(commonlyminimal, or a custom profile) when doing--existing-config[1], [2], or - Use a post-install workflow (e.g.,
drush site:install …thendrush config:import) if you must keep using an install profile likestandard[1].
Core is tracking this limitation as an unresolved issue (“Needs work”) [2].
Sources:
[1] Drupal Stack Exchange: restriction/error when using --existing-config with profiles that implement install hooks
[2] Drupal core issue #2982052 (“Allow an install hook in profiles to install from configuration”)
Direct profile installs with config fail when the profile implements hook_install().
The --existing-config flag (line 209) is passed for direct profile installs whenever config files are present. However, Drupal's installer explicitly rejects --existing-config for profiles implementing hook_install() (including the core standard profile) with the error: "The selected profile has a hook_install() implementation and therefore cannot be installed from configuration."
Direct VORTEX_PROVISION_TYPE=profile runs with synced config will fail, while fallback installs are protected by the cleanup block (lines 215–220) that removes profile-created config before import. Apply the same post-install cleanup pattern to direct profile installs, or key --existing-config usage on profile compatibility rather than is_fallback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/vortex/provision.sh` around lines 207 - 223, The direct profile
install path currently adds --existing-config when has_config is true but does
not perform the same post-install cleanup as the fallback path, so installs fail
for profiles implementing hook_install(); after calling drush site:install (the
line invoking drush site:install "${opts[@]}") ensure the cleanup commands (the
drush entity:delete shortcut_set and the drush config:delete calls for
field.field.node.article.body, field.field.node.article.field_image,
field.storage.node.field_image, taxonomy.vocabulary.tags,
user.role.content_editor) run for direct profile installs as well when
has_config is true (i.e., either move that cleanup block out of the if [
"${is_fallback}" = "1" ] condition or add an equivalent branch that runs when [
"${is_fallback}" != "1" ] && [ "${has_config}" = "1" ]) so profile-created
config is removed before configuration import.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/tests/phpunit/Functional/ProvisionFallbackTest.php:
- Around line 59-61: The test appends a second
VORTEX_PROVISION_FALLBACK_TO_PROFILE env var via fileAddVar which results in
duplicate keys; change the test to replace the existing variable instead of
appending (or restore the original .env before the second provision). Locate the
calls to fileAddVar('.env', 'VORTEX_PROVISION_FALLBACK_TO_PROFILE', ...) in
ProvisionFallbackTest and either swap them for a helper that performs an in-file
replace (update the existing key's value) or add a restore/reset step before the
second call and then call syncToContainer(['.env']) so the container receives
the single, correct env var value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 96ce24fc-ae10-4e04-b7c6-03726c843fe5
📒 Files selected for processing (1)
.vortex/tests/phpunit/Functional/ProvisionFallbackTest.php
| $this->logSubstep('Provision with fallback should succeed'); | ||
| $this->fileAddVar('.env', 'VORTEX_PROVISION_FALLBACK_TO_PROFILE', 1); | ||
| $this->syncToContainer(['.env']); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using a replace operation instead of appending duplicate env vars.
The fileAddVar() method appends to the file, so after line 47 and line 60, the .env file contains both VORTEX_PROVISION_FALLBACK_TO_PROFILE=0 and VORTEX_PROVISION_FALLBACK_TO_PROFILE=1. While this works because shell scripts typically use the last occurrence, it relies on implicit behavior and could cause confusion during debugging.
Consider either:
- Restoring the backup between the two provision attempts, or
- Using a helper that replaces the value instead of appending
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/tests/phpunit/Functional/ProvisionFallbackTest.php around lines 59 -
61, The test appends a second VORTEX_PROVISION_FALLBACK_TO_PROFILE env var via
fileAddVar which results in duplicate keys; change the test to replace the
existing variable instead of appending (or restore the original .env before the
second provision). Locate the calls to fileAddVar('.env',
'VORTEX_PROVISION_FALLBACK_TO_PROFILE', ...) in ProvisionFallbackTest and either
swap them for a helper that performs an in-file replace (update the existing
key's value) or add a restore/reset step before the second call and then call
syncToContainer(['.env']) so the container receives the single, correct env var
value.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2406 +/- ##
==========================================
- Coverage 79.37% 78.85% -0.53%
==========================================
Files 126 119 -7
Lines 6711 6574 -137
Branches 44 0 -44
==========================================
- Hits 5327 5184 -143
- Misses 1384 1390 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
AlexSkrypnyk
changed the title
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/vortex/provision.sh (1)
207-208:⚠️ Potential issue | 🟠 MajorDirect profile installs still fail for the default
standardprofile.
has_config=1still adds--existing-configon the non-fallback path, andDRUPAL_PROFILEdefaults tostandard. That leavesVORTEX_PROVISION_TYPE=profilebroken whenever sync config exists, because the fallback-only cleanup at Lines 215-229 never runs here. Gate--existing-configon profile compatibility, or use the same post-install/import flow for direct profile installs too.For Drupal 10/11 with Drush, can `drush site:install --existing-config` be used with the `standard` install profile, or do profiles that implement `hook_install()` require a post-install `config:import` workflow instead?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/vortex/provision.sh` around lines 207 - 208, The current logic unconditionally appends --existing-config when has_config=1 and not is_fallback, which breaks direct profile installs because DRUPAL_PROFILE defaults to "standard" and the fallback-only post-install/import cleanup (the steps around the existing fallback cleanup block referenced at lines 215-229) never runs; update the condition that adds opts+=(--existing-config) to only do so when the chosen DRUPAL_PROFILE is compatible with --existing-config (i.e., not "standard" or when the profile declares support), or alternatively remove that append for VORTEX_PROVISION_TYPE=profile and instead invoke the same post-install/config:import flow used by the fallback path after site:install; adjust the condition around is_fallback/has_config to check DRUPAL_PROFILE and VORTEX_PROVISION_TYPE or re-use the fallback post-install/import routine for direct profile installs so --existing-config is not wrongly applied to profiles that require post-install config import.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/vortex/provision.sh`:
- Around line 370-379: Move the existing cache-rebuild block (conditioned on
VORTEX_PROVISION_CACHE_REBUILD_AFTER_DB_UPDATE_SKIP and invoking drush
cache:rebuild) so it executes immediately after the drush updatedb
--no-cache-clear call and before the subsequent drush config:export used for
verification (the export guarded by
VORTEX_PROVISION_VERIFY_CONFIG_UNCHANGED_AFTER_UPDATE), so the verification runs
against a fresh cache state; keep the same skip condition
(VORTEX_PROVISION_CACHE_REBUILD_AFTER_DB_UPDATE_SKIP) and preserve the pass/echo
messages and behavior when skipped.
---
Duplicate comments:
In `@scripts/vortex/provision.sh`:
- Around line 207-208: The current logic unconditionally appends
--existing-config when has_config=1 and not is_fallback, which breaks direct
profile installs because DRUPAL_PROFILE defaults to "standard" and the
fallback-only post-install/import cleanup (the steps around the existing
fallback cleanup block referenced at lines 215-229) never runs; update the
condition that adds opts+=(--existing-config) to only do so when the chosen
DRUPAL_PROFILE is compatible with --existing-config (i.e., not "standard" or
when the profile declares support), or alternatively remove that append for
VORTEX_PROVISION_TYPE=profile and instead invoke the same
post-install/config:import flow used by the fallback path after site:install;
adjust the condition around is_fallback/has_config to check DRUPAL_PROFILE and
VORTEX_PROVISION_TYPE or re-use the fallback post-install/import routine for
direct profile installs so --existing-config is not wrongly applied to profiles
that require post-install config import.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ec1eec28-3f27-4f91-a4c7-85db365460dc
📒 Files selected for processing (1)
scripts/vortex/provision.sh
| if [ "${VORTEX_PROVISION_CACHE_REBUILD_AFTER_DB_UPDATE_SKIP}" != "1" ]; then | ||
| task "Clearing cache after database updates." | ||
| drush cache:rebuild | ||
| pass "Cache was cleared." | ||
| echo | ||
| else | ||
| pass "Skipped cache rebuild after database updates." | ||
| echo | ||
| fi | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and get its line count
wc -l scripts/vortex/provision.shRepository: drevops/vortex
Length of output: 89
🏁 Script executed:
# Read the relevant section around lines 343-346 and 370-379
sed -n '330,390p' scripts/vortex/provision.sh | cat -nRepository: drevops/vortex
Length of output: 2468
🏁 Script executed:
# Get a broader view of the execution flow
sed -n '330,385p' scripts/vortex/provision.sh | cat -nRepository: drevops/vortex
Length of output: 2170
Move cache rebuild to immediately after drush updatedb --no-cache-clear.
When VORTEX_PROVISION_VERIFY_CONFIG_UNCHANGED_AFTER_UPDATE=1, the second drush config:export at line 344-345 runs before the cache rebuild at line 370-379, leaving the verification running against stale caches. This can cause the verification to produce unreliable results if cache state affects config export output. Move the cache rebuild block to immediately after the updatedb --no-cache-clear call, before the verification export.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/vortex/provision.sh` around lines 370 - 379, Move the existing
cache-rebuild block (conditioned on
VORTEX_PROVISION_CACHE_REBUILD_AFTER_DB_UPDATE_SKIP and invoking drush
cache:rebuild) so it executes immediately after the drush updatedb
--no-cache-clear call and before the subsequent drush config:export used for
verification (the export guarded by
VORTEX_PROVISION_VERIFY_CONFIG_UNCHANGED_AFTER_UPDATE), so the verification runs
against a fresh cache state; keep the same skip condition
(VORTEX_PROVISION_CACHE_REBUILD_AFTER_DB_UPDATE_SKIP) and preserve the pass/echo
messages and behavior when skipped.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php:
- Around line 467-481: Replace the append-style call to fileAddVar() with an
in-place replacement using File::replaceContentInFile() to avoid duplicate .env
keys: locate the test lines that set VORTEX_PROVISION_FALLBACK_TO_PROFILE
(currently using fileAddVar('.env', 'VORTEX_PROVISION_FALLBACK_TO_PROFILE',
...)), and change them to call File::replaceContentInFile('.env',
'/^VORTEX_PROVISION_FALLBACK_TO_PROFILE=.*$/m',
'VORTEX_PROVISION_FALLBACK_TO_PROFILE=0' or '=1' as appropriate), then keep the
existing syncToContainer(['.env']) and subsequent cmdFail/assert logic unchanged
so the .env contains a single updated key value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bac60fdb-77c3-4ced-8141-564500782b6a
📒 Files selected for processing (1)
.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
AlexSkrypnyk
force-pushed
the
feature/fix-profile-fallback
branch
from
8989e93 to
10cd4a3
Compare
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
|
Code coverage (threshold: 90%) Per-class coverage |
1 participant
Summary
Fixed provisioning failures when falling back to profile installation from the
standardprofile. Thestandardprofile'shook_install()creates entities and config that conflict with subsequent configuration imports. This fix avoids using--existing-configduring fallback installs and cleans up profile-created entities before importing configuration. A cache rebuild step was also added betweendrush updatedbanddrush config:importto ensure a clean state.Changes
scripts/vortex/provision.sh— Core logic fixprovision_from_profile()to acceptis_fallbackandhas_configarguments, replacing use of the outer scope variablesite_has_config_files.--existing-configis no longer passed todrush site:install— profiles withhook_install()(likestandard) cannot be installed from existing config.hook_install()are deleted to prevent conflicts during config import.VORTEX_PROVISION_CACHE_REBUILD_AFTER_DB_UPDATE_SKIPvariable and a cache rebuild step betweendrush updatedbanddrush config:import.provision_from_profilecall sites to pass the new arguments..vortex/tests/bats/unit/provision.bats— Unit test coverage.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php— Functional testtestAhoyWorkflowProvisionFallbackToProfile()covering the full fallback flow: build with DB dump, export config, remove dump, provision fails without fallback, provision succeeds with fallback, assert modules and homepage..vortex/docs/— Documentation updatesVORTEX_PROVISION_CACHE_REBUILD_AFTER_DB_UPDATE_SKIPto the variables reference.VORTEX_PROVISION_FALLBACK_TO_PROFILEand added entry forVORTEX_PROVISION_CACHE_REBUILD_AFTER_DB_UPDATE_SKIP.Before / After
Fallback profile install flow
Variable control
Summary by CodeRabbit
New Features
Documentation
Tests