fix: do not explicitly set the backdrop_drupal_compatibility and update_free_access settings, fixes #6090

[klonos]

The Issue

As described in #6090, if these settings are explicitly set in settings.ddev.php, trying to set them in settings.php or settings.local.php never takes.

How This PR Solves The Issue

This PR removes the lines in settings.ddev.php that explicitly set these two settings, which allows the values to be set in settings.php or settings.local.php (which unlike Drupal is provided by default OOTB in Backdrop).

Manual Testing Instructions

1. Install a vanilla Backdrop CMS site using the DDEV recipe.
2. Edit your settings.php file or the settings.local.php file and set $settings['backdrop_drupal_compatibility'] = FALSE;.
3. Try reading that setting via settings_get('backdrop_drupal_compatibility') (for instance using the devel module and dpm() or something) -> regardless of how that setting is configured, it is always returned as TRUE 👎🏼
4. Apply this PR/patch -> try reading the setting again -> this time it should return the value as set in the settings.php file or the settings.local.php file 👍🏼

Automated Testing Overview

N/A

Related Issue Link(s)

Fixes #6090

Release/Deployment Notes

N/A

[github-actions]

Download the artifacts for this pull request:

• all-ddev-executables.zip
• ddev-macos-amd64.zip
• ddev-macos-arm64.zip
• ddev-linux-arm64.zip
• ddev-linux-amd64.zip
• ddev-windows-amd64-installer.zip

See Testing a PR.

[rfay]

This is fine with me. But it seems like it would be better to

1. Include settings.local.php after settings.ddev.php (fixing that)
2. In settings.ddev.php say "if not set then set them" and keep the settings.

But we really just want to do what works best for Backdrop folks. Glad to have you here.

[rfay]

Hoping to hear a response from you about this @klonos - I do think there's an easier way.

If I don't hear back by May 15 or so I'll close this.

[klonos]

Hey @rfay 👋🏼 ...apologies for the late reply here; we have been working on things in Backdrop core since today is the feature freeze for the upcoming 1.28.0 minor release, which is slated for May 15. There has been too much activity to keep up with, and there will continue to be a lot of effort/focus in the next couple of weeks on fixing bugs and improving all the features that got in before the feature freeze.

I promise to loop back to this after all that "release frenzy" has subsided (if not sooner than that).

[rfay]

I'll close this about June 15, 2024 if you don't get back to it. Thanks for your work on it, hope to see you successful.

[stasadev]

What if we just read if the value has been set before and use it?

I intentionally did not use the shorthand ??= to make it clearer how the value is set.

I'm not sure why settings.local.php doesn't override the value here:

ddev/pkg/ddevapp/drupal/backdrop/settings.php

Lines 438 to 445 in 28a5afc

	// Automatically generated include for settings managed by ddev.
	if (getenv('IS_DDEV_PROJECT') == 'true' && file_exists(__DIR__ . '/settings.ddev.php')) {
	include __DIR__ . '/settings.ddev.php';
	}
	if (file_exists(__DIR__ . '/settings.local.php')) {
	include __DIR__ . '/settings.local.php';
	}

[stasadev]

I tried the quickstart and saw some difference between the existing settings.php and the generated one.

After research, I agree that these values should be removed as they already have the same values ​​in settings.php.

The only thing I added was copying settings.php from a fresh Backdrop installation (with settings.ddev.php at the end): this changes the order for settings.ddev.php and settings.local.php, but makes it the same as for a fresh quickstart, which removes the confusion I had initially.

[rfay]

@klonos if you're good with this it can go in.