Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
If you have an existing settings.php with database settings and a hash salt it is possible to install drupal without the install_profile setting being written to settings.php
Discovered in #2090115: Don't install a module when its default configuration has unmet dependencies
Proposed resolution
- If the settings.php exists and has install_profile - it should be used and the form to select the profile should be skipped
- If settings.php exists and is writable and does not have the install_profile - it should be selected using the form and written to settings.php
Remaining tasks
User interface changes
API changes
Comment | File | Size | Author |
---|---|---|---|
#19 | 11-16-interdiff.txt | 2.24 KB | alexpott |
#19 | 2451363.19.patch | 5.72 KB | alexpott |
#16 | 2451363.16.patch | 5.68 KB | alexpott |
#16 | 11-16-interdiff.txt | 2.24 KB | alexpott |
#11 | 2451363.11.patch | 5.46 KB | alexpott |
Comments
Comment #1
YesCT CreditAttribution: YesCT commentedmight be a duplicate of #2451369: It is possible to install a site without install_profile in settings.php
Comment #2
alexpott[Edit: was meant to be issue summary :)]
Comment #3
alexpottComment #4
alexpottSettings.php has to be writable during the installer (see
install_check_requirements()
) so removing the requirement to deal with non-writable settings.php from issue summary.Comment #5
alexpottComment #7
pjcdawkins CreditAttribution: pjcdawkins commentedThis works with the debug script I made.
I can't really review it from a code perspective, though I suspect the mkdir() calls are now unnecessary.
I think the install_profile setting should be documented in
default.settings.php
. Particularly since it is now required when settings.php is not writable (it wasn't required before). Can that be part of this issue?Comment #8
alexpott@pjcdawkins it was required if you were going to using a profile that provided modules. But yes I agree we should document it in default.settings.php
Comment #9
BerdirHm.
So far, I've only noticed this problem when installing with drush, which *does* allow to install when your settings.php is not writable, as long as all the required information is already there.
The fix makes sense for the UI think, although it might be a bit confusing, not sure. Also not exactly sure how that will work in combination with a distribution, which skips that step too?
For drush, it's a bit confusing/unexpected too. Because you are required to provide the install profile upfront, but it will then actually not be used. To test, drush si -y standard, and then drush si -y minimal will install again in standard. Without ever telling you so. Maybe that's something that drush needs to take care of, not sure.
Comment #10
alexpott@Berdir all this patch does is say that if install_profile is set in settings.php then that is what is used which is consistent with what we for databases and config_directories. HEAD is currently very broken because if you had an exclusive distribution with a non writable settings.php which did not have the install_profile set and install using drush then modules from the profile are used during installation but once Drupal is installed they won't be.
Comment #11
alexpottDiscussed this with @Berdir in IRC we decided to not force the profile according to what is in settings.php but to instead ensure that we rewrite if necessary. This means that drush si will crash if working with a write protected settings.php and a mismatched install profile.
Comment #14
BerdirManually tested with drush si and a distribution install profile. Works well, I think this is the better approach. Will do another review and RTBC later.
Comment #15
Berdir"array" missing.
Comment is a bit strange.. why Actually, should "skipped to" be "skipped too", but too doesn't make sense to me either...
Not correct anymore I think?
Comment #16
alexpottAddressed everything in @Berdir's review. Thanks!
Comment #17
BerdirLooks great now.
Comment #18
pjcdawkins CreditAttribution: pjcdawkins commentedThe patch works but the documentation is slightly wrong.
This should be
$settings['install_profile']
, and it should be under the "Settings" section just below.Comment #19
alexpott@pjcdawkins nice catch :)
The interdiff is more confusing than just reading the patch :)
Comment #20
pjcdawkins CreditAttribution: pjcdawkins commentedThanks. I tested with the same script I was using before, under a bunch of different cases, including writable and non-writable settings.php.
Comment #22
webchickOops, my comment got eaten.
Committed and pushed to 8.0.x. Thanks!