Incopmatibility from BLT: The default value of $drs_override_config_directories is different from BLT's

[nsfisis]

Describe the bug

The default value of $drs_override_config_directories is different from $blt_override_config_directories.

BLT:

https://github.com/acquia/blt/pull/4709/files#diff-f9a40a07d9a773b708777ca2e41c24519280d8ffc19f7379b56e2587d8e389ef

if (!isset($blt_override_config_directories)) {
  $blt_override_config_directories = TRUE;
}

If $blt_override_config_directories is not set, it is set to TRUE.

Drupal Recommended Settings:

https://github.com/acquia/drupal-recommended-settings/blob/60a053a057e5b80fcb83c559ce8c5479f665eaaf/settings/config.settings.php

$drs_override_config_directories = $drs_override_config_directories ?? FALSE;

If $drs_override_config_directories is not set, it is set to FALSE.

To Reproduce

N/A

Expected behavior

According to the source code comment,

/**
 * Override config directories.
 *
 * Acquia Drupal Recommended Settings makes the assumption that,
 * if using multisite, the default configuration
 * directory should be shared between all multi-sites, and each multisite will
 * override this selectively using configuration splits. However, some
 * applications may prefer to manage the configuration for each multisite
 * completely separately. If this is the case,
 * set FALSE to $drs_override_config_directories
 * and $drs_override_site_studio_sync_directories
 * $settings['config_sync_directory'] = $dir . "/config/$site_dir"
 * $settings['site_studio_sync'] =  $dir . "/sitestudio/$site_dir" in
 * settings.php, and we will not overwrite it.
 */

setting FALSE to $drs_override_config_directories seems an exceptional case. Thus, the default value of it is expected to be TRUE as it is in BLT.

If this change is intended, it should be mentioned in the migration guide: https://dev.acquia.com/tutorial/you-dont-need-blt-acquia-cloud.

Screenshots

N/A

Environment

DRS v1.0.1 (the latest version).

Additional context

No.

[secretsayan]

@vishalkhode1 : Any ideas when we can land this, its blocking BLT removal for us?

[laccd-wst-services-admin]

This bug was causing the config_sync_directory setting to come back different for our multisite environment.

With BLT:
"config_sync_directory" => "/var/www/html/drupal/config/default"

With DRS:
"config_sync_directory" => "../config/site.com"

• BLT assumed $blt_override_config_directories was TRUE if not set.
• DRS assumes $drs_override_config_directories is FALSE if not set.

Quick fix is to add $drs_override_config_directories = TRUE; in settings.php above the DRS require but the fix should be how BLT behaved if you want a nice migration path.

[vishalkhode1]

I think if we make it like blt now, then this change will impact others as well (who are not using blt or not used blt till now). I think the quick solution would be, changing code
From

$drs_override_config_directories = $drs_override_config_directories ?? FALSE;

To

$drs_override_config_directories = $drs_override_config_directories ?? (bool) getenv('DRS_OVERRIDE_CONFIG_DIR') ?? FALSE;

So, you just need to set one environment variable at your end i.e DRS_OVERRIDE_CONFIG_DIR to true and this would behave same like you are expecting.
Let me know your thoughts on it.
cc: @secretsayan / @nsfisis / @laccd-wst-services-admin

[laccd-wst-services-admin]

@vishalkhode1 I think a cleaner approach would be to add drs_override_config_directories to the $settings array. That way you can also move the comment about these settings to the settings.php template rather than the config.settings.php below:

`
/**

• Override config directories.
• Acquia Drupal Recommended Settings makes the assumption that,
• if using multisite, the default configuration
• directory should be shared between all multi-sites, and each multisite will
• override this selectively using configuration splits. However, some
• applications may prefer to manage the configuration for each multisite
• completely separately. If this is the case,
• set FALSE to $drs_override_config_directories
• and $drs_override_site_studio_sync_directories
• $settings['config_sync_directory'] = $dir . "/config/$site_dir"
• $settings['site_studio_sync'] = $dir . "/sitestudio/$site_dir" in
• settings.php, and we will not overwrite it.
  */
  `

At the moment, the $drs_override_config_directories is a variable in the config.settings.php file unmodifiable by the end-user. In other words, how would the user set this flag besides setting the variable in the settings.php like we are doing right now? I think to make it consistent with how settings are currently set in the $settings variable, adding it as $settings['override_config_directories'] would be cleaner.

Another thing the user can do to address this bug at the moment is to just set $settings['config_sync_directory'] = "../config/default"; in settings.php.

[secretsayan]

@vishalkhode1 I am fine with any approach which replicates the same behaviour as BLT. But my only concern is that we have 150 plus sites and the solution should not make us change settings on a per site basis.

To summarise the solution should have the following capabilities:

1. The config sync folder should by default be "../config/default" for all the sites.

2. We should have the ability to change the config sync folder for a particular site.

So, I will be happy with your proposed solution of setting one environment variable for all the sites. But if I were in your position, I would have created DRS 2.0 and changed the setting to TRUE in DRS and mentioned the same as breaking changes in the release notes.

[secretsayan]

Update: In ACSF, one can workaround this bug by setting config_sync_directory to ../config/default in factory-hooks/post-settings-php