When settings.php is a symbolic link, services.yml and settings.local.php are not included. The reason is the usage of __DIR__ to resolve the site path. In case of a symlink, __DIR__ returns the real path of settings.php.

CommentFileSizeAuthor
#13 interdiff-8-13.txt673 byteshchonov
#13 2680379-13.patch813 byteshchonov
#8 interdiff-2-8.patch1.15 KBhchonov
#8 2680379-8.patch1.15 KBhchonov
#2 2680379.patch1.16 KBmkalkbrenner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mkalkbrenner created an issue. See original summary.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
CreditAttribution: mkalkbrenner at bio.logis Genetic Information Management GmbH commented
Status: Active » Needs review
FileSize
2680379.patch1.16 KB

This patch for default.settings.php solves the issue (for new installations).

cilefen’s picture

cilefen CreditAttribution: cilefen commented

This is interesting. Can you look over the issue priorities and evaluate if this is really major? It could be, but its effect is limited to sites who made a symlink and generally know they did it.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
CreditAttribution: mkalkbrenner at bio.logis Genetic Information Management GmbH commented

@cilefen:
Yes, I think it's a least major. Personally I consider this bug as critical because I ran into this issue on an existing Drupal 8 production site!
The site was up and running with a services.yml it randomly picked up in the folder where the symlinked settings.php file physically resists. And this services.yml was different from the one that existed in sites/default. That's how I discovered the issue.
So, the assumption that "its effect is limited to sites who made a symlink and generally know they did it" has already be proven to be wrong.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr at Northern Commerce commented
Issue tags: +D8 major triage deferred
Related issues: +#2280383: FIx settings.local.php after new bootstrap

@mkalkbrenner first of all thanks for this report.

Discussed this issue with @alexpott @xjm @catch @effulgentsia.

DRUPAL_ROOT is effectively a global and we are trying to avoid using them, see #2328111: Replace most instances of the DRUPAL_ROOT constant with the app.root container parameter for some more background. Not sure what the fix here should be but potentially it could be adding documentation to default.settings.php. #2280383: FIx settings.local.php after new bootstrap is the issue that actually changed the settings.local.php include from DRUPAL_ROOT to __DIR__ and I think one of the goals there was to reduce fragility. It looks like #2381763: Adjust the order of container yamls to override settings per environment is where the services.yml reference in default.settings.php was added (I think that line of code has always been using __DIR__).

It would be nice to know more about your hosting setup or just more about how you ran into this situation. In #4 I can't tell if you set up the symlink or not or if it was a result of a hosting provider's infrastructure or similar. If you can provide any information about what steps you used to troubleshoot that may also be helpful in determine the priority of this issue. Thanks.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
CreditAttribution: mkalkbrenner at bio.logis Genetic Information Management GmbH commented

Yes, #2280383: FIx settings.local.php after new bootstrap introduced this issue. The real fix for that issue would have been to replace $conf_path by $site_path, what I did now. The people who discussed that issue two years ago were probably not aware of problem related to __DIR__ and symlinks.
From comment #3 in that issue:

I like this, __DIR__ is always preferable if we know that it has to be in the same directory.

The symlink is in the same directory! But __DIR__ resolves the symlink and therefor it leads to a jump to a different directory in our case. See http://php.net/manual/en/language.constants.predefined.php

Since Drupal 5 we use such symlinks. We have multiple settings files, one per environment (dev, testing, staging, production). Due to the fact that this file contains passwords, these files do not resist in the standard code repository a normal developer can access. They're located in a different repository and are therefor deployed in different directories on the servers. For Drupal 5, 6 and 7 symlinking settings.php was no problem.

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented
Issue tags: +DevDaysMilan
FileSize
2680379-8.patch1.15 KB
interdiff-2-8.patch1.15 KB

Exchanged DRUPAL_ROOT with $app_root as mentioned in #5.

Status: Needs review » Needs work

The last submitted patch, 8: interdiff-2-8.patch, failed testing.

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented
Status: Needs work » Needs review

Wrongly named interdiff-2-8.patch, it should've been interdiff-2-8.txt...

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented
cilefen’s picture

cilefen CreditAttribution: cilefen commented 
+++ b/sites/default/default.settings.php
@@ -654,9 +654,10 @@
+ * Load services definition file. Instead of __DIR__ we use $app_root and
+ * $site_path which works even if settings.php is symlinked.

This comment change is unnecessary.

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented
FileSize
2680379-13.patch813 bytes
interdiff-8-13.txt673 bytes

@cilefen like you said.

Status: Needs review » Needs work

The last submitted patch, 13: 2680379-13.patch, failed testing.

The last submitted patch, 13: 2680379-13.patch, failed testing.

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented
Status: Needs work » Needs review

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler at bio.logis Genetic Information Management GmbH commented
Status: Needs review » Reviewed & tested by the community

This makes a lot of sense.

settings.php itself is required with the following:

      require $app_root . '/' . $site_path . '/settings.php';

And that is also used in e.g. example.local.settings.php.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed 44adac7 and pushed to 8.3.x. Thanks!
Committed 8af9db2 and pushed to 8.2.x. Thanks!

I checked that $app_root is defined when settings.php is run and it is.

  • alexpott committed 44adac7 on 8.3.x 
    Issue #2680379 by hchonov, mkalkbrenner, cilefen: services.yml and...

  • alexpott committed 8af9db2 on 8.2.x 
    Issue #2680379 by hchonov, mkalkbrenner, cilefen: services.yml and...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

arsn’s picture

arsn CreditAttribution: arsn commented
Related issues: +#1008402: Allow the use of symlinks within the files directory.