Problem/Motivation
It is not clear how to override the temporary-files directory in settings.php
.
Proposed resolution
The first three of these lines are already in settings.php
(commented-out examples). Add the additional temporary-files directory in front so the keys are in alphabetical order.
# $config['system.file']['path']['temporary'] = '/tmp';
# $config['system.site']['name'] = 'My Drupal site';
# $config['system.theme']['default'] = 'stark';
# $config['user.settings']['anonymous'] = 'Visitor';
In addition to documenting a common use case, this provides an example of overriding a complex setting. The existing examples are simple string variables.
Remaining tasks
Put the new config override in alphabetical order with the other example config overrides. See Comment #46.
User interface changes
None
API changes
None
Data model changes
None
Original report by britter
There may be a logical explanation, however currently I could not find documentation regarding this change.
In Drupal 7 you were able to set the public, private, and temporary files directory in the main settings.php, which made it very easy to alter the path of each directory per environment. However, in Drupal 8 this does not work/seem to be possible to change the temporary files directory path (so I wrote my first D8 module to support this)(and you can change the public and private path, but not tmp), but is there a good explanation of why this was left out or should you not make the tmp directory changeable in this way? Should the temporary files directory not be separated out by itself in D8 or put inside the private files directory (usually not a good idea)? Suggestions are welcome.
Comment | File | Size | Author |
---|---|---|---|
#52 | add_the_temporary_files-2662646-52.patch | 541 bytes | philnguyen |
#49 | add_the_temporary_files-2662646-49.patch | 617 bytes | philnguyen |
#33 | path-tmp-d8.jpg | 708.52 KB | memtkmcc |
#29 | add_the_temporary_files-2662646-29.patch | 458 bytes | daften |
#12 | 2662646-configurable-temporary-file-path-settings.patch | 455 bytes | chera.jaswinder |
Comments
Comment #2
britter CreditAttribution: britter commentedComment #3
chera.jaswinder CreditAttribution: chera.jaswinder commentedLet me start with Drupal D7 in settings.php we write
$conf['file_temporary_path'] = '/tmp';
and changes are reflected.But D8 has a whole new configuration system to store configuration data in a consistent manner. All of your site configuration from enabled modules through fields, contact categories to views, are stored with this system. The system is designed to make it much easier than prior Drupal versions to make changes, export site configuration to files, and import those changes back into the site.
Earlier we made use of features and strong arm to export all the configurations but due to excellent configurations management (https://www.drupal.org/documentation/administer/config) we are able to export everything at once even settings.php configurations. So changes made in settings.php will not be reflected at administrative UI interface but will be reflected if exported and imported in some another environment (like stage) using Configuration Manager module to export settings required.
As mentioned by you that you was able to set settings.php according to environment, here you can also do the same. Let me provide you an example:
To give you a quick start, I want to share the settings.local.php that I use for my D8 development.
Copy the below code into a settings.local.php file and place it in the settings directory. Uncomment the code settings.php (below) to activate the include function.
This is how the code in settings.php should look now:
Do note that, different from Drupal 7, these overrides are not visible in the admin interface. For that purpose I have added an overridden site name to give you some visible proof that the configuration is overridden.
For me above one is cleaner way to manage settings per environment.
Other way round and simple, you can also override by using
in settings.php, just add the provided config code in settings.php.
You would also like know how I got $config['system.file']['path.temporary'] because this $config['system.site']['name'] is already mentioned.
Answer is earlier in D7 we store configurations in variable but after D8 configuration management system everything is now stored in config table. This table contains all the variables which stores its related variables information in serialized format. So after looking into system.file variable in unserialized format I was able to track the same and put configuration in same.
Please CLEAR THE CACHE at last and its all done.
Hope it makes your every doubt clear.
Comment #4
chera.jaswinder CreditAttribution: chera.jaswinder commentedComment #5
chera.jaswinder CreditAttribution: chera.jaswinder commentedComment #6
chera.jaswinder CreditAttribution: chera.jaswinder commentedDocumentation updates required in settings.php for configurable overrides.
Comment #7
chera.jaswinder CreditAttribution: chera.jaswinder commentedPatch added for latest version of Drupal 8.0.3
Comment #8
chera.jaswinder CreditAttribution: chera.jaswinder commentedComment #9
chera.jaswinder CreditAttribution: chera.jaswinder commentedComment #11
jhodgdonSorry, but I don't think we want to add this to the documentation. There is already a section in default.settings.php that explains how to do config overrides, and we are definitely not going to try to document every possible override there.
Comment #12
chera.jaswinder CreditAttribution: chera.jaswinder commentedHi Jhodgdon,
As users are not aware for the temporary path configuration. I think we should include at least attached new patch to make users aware and clear for temporary path.
It is most common thing which should be added for users ease.
Regards,
Jaswinder
Comment #13
chera.jaswinder CreditAttribution: chera.jaswinder commentedComment #14
cilefen CreditAttribution: cilefen commented@chera.jaswinder:
Thank you for your time and attention to this.
@jhodgdon is the documentation system maintainer. Unless a strong case can be made as to why this ought to be documented in the default settings now in Drupal 8 when it is not documented in the default settings in Drupal 7, the decision is clear. There are many important issues that need attention.
Consider enhancing the drupal.org documentation with this information if it will help people.
Comment #15
daften CreditAttribution: daften at District09 commentedI'd say there is a strong case for documenting this.
In Drupal 7 it wasn't documented, but the temporary path could be set in the settings.php file similar to how the public and private folder were set:
In Drupal 8 the examples in default.settings.php show that the public and private file folders can be set similarly, but with the $settings array. For the temporary path that is not the case however, and it only works by using
This can cause a lot of confusion for people. Saying it shouldn't be documented because "it wasn't documented in Drupal 7" is not a proper argument IMO, since the behaviour aligned with the other folder settings. Drupal 8 changes a LOT of things, this is one of them, and those changes should be documented properly. The patch in #12 does this very decently and has no inherent risk. While I agree, there should be a limit on what can be accepted in the code, it should at least be documented here I believe: https://www.drupal.org/node/1928898
Comment #16
colanI agree with #15; this is a rather important configuration setting, and should be explicitly stated in one of the settings files (either settings.php or settings.local.php).
In Aegir for example, we make these temporary directories unique for each site by sticking it in sites/www.example.com/private/temp. Besides being a good security practice (by not having multiple sites access the same temporary file space), it helps keep things organized. Sites' temporary data doesn't clutter the global space, and everything related to a particular site stays within its site directory.
Site maintainers should be forced to consider these factors when they review one of the shipped settings files. But this won't be possible if they don't find it anywhere. Having said that, it doesn't need to be in the main configuration settings block. I'd be fine with moving it to its own stanza, or another appropriate one, but it should be in there somewhere.
The decision to keep this out should be reversed.
RTBCing the last patch (#12), but this can also be set to NW if someone wants to stick it elsewhere. I'm also assuming that
['path.temporary']
is as acceptable as['path']['temporary']
. If not, then this definitely needs to be set to NW.Comment #17
cilefen CreditAttribution: cilefen commentedComment #18
memtkmcc CreditAttribution: memtkmcc at Omega8.cc commentedI agree that it should be documented somewhere, unless D8 core will fix this inconsistency, silently introduced. Like others explained above, in D7 all those files specific paths were bundled in the
$config[]
array, while in D8 two of them (along with other variables) have been moved/renamed to$settings[]
, so it is definitely not expected that this single path should still live in the$config[]
array, unless it will be properly documented.Comment #19
catchThis is configurable in the UI at admin/config/media/file-system, so I'm not sure why we'd also document it in settings.php
Comment #20
daften CreditAttribution: daften at District09 commentedThen why document setting the public and files directory path? That's also configurable through the UI ...
Documenting this is needed for the very simple reason: some people need it. E.g. you take a production database but on your local machine the temp path is different, but you don't want it changed in the DB. You have several environments (prod, QA, staging, ...) and you want to set these items differently per environment, but want to be able to sync your database downstream.
I honestly don't understand the question, because nearly everything is configurable in the UI, so why document anything in the settings.php file?
Comment #21
catchThey're not configurable in the UI. They're shown in the UI, which points you to settings.php to change them.
Config overrides are explaining generically in settings.php, we don't need to document every one.
Comment #22
daften CreditAttribution: daften at District09 commentedTrue, but something being settable in the UI, is not an argument to NOT document it in the settings.php. The would mean nothing for $config would need to be explained because "it's all configurable in the UI"
The reason why this warrants documentation is very simple:
1/ Public, private and temp folders had similar config names to be overridden in settings.php, so for most if not almost all developers coming from drupal 7, this is just plain confusing.
2/ Public and private folders are explained because they are deemed environment specific and now need to be set in settings.php. However the temp folder can also be seen as environment specific. Especially if you consider local dev as one of the environments.
The ONLY argument i read here is: we're not going to document every config override in settings.php. Which is in itself a valid point. But i wonder if anybody that makes this argument has thought for one second about all the new developers that come to Drupal 8. Because every dev that comes from "outside" or starts from Drupal 7 needs to be considered a new dev for drupal 8.
If it's not documented in settings.php, there should be a page on Drupal docs about this at least. I can't find any page about this ATM (could be because of the documentation migration).
Comment #23
cilefen CreditAttribution: cilefen commentedThe arguments on each side are:
I will add that for most site admins, the configuration system is a mystery if folks are even aware it exists. So to assume that every configuration is easily discoverable by anybody doesn't hold water. And, people seem to want this. And this issue is over one line...of documentation, which will have a maintenance cost of practically nothing.
So, I am feeling a +1 for this. However, the issue summary needs an update before this can be committed. It should outline the reasons for making this change—an effort to convince others of the worthiness of this addition. I am setting this to "Needs work" until that happens.
Comment #24
colanI'm quite sure that the patch needs work as well. It has
['path.temporary']
instead of['path']['temporary']
. I don't think that'll work.Comment #25
xjmI don't think the documentation of this belongs in
settings.php
-- as @catch points out, it's just a config override. There is already a separate section explaining config overrides and their risks and requirements.Maybe a compromise would be to simply add it to the config overrides examples? This bit:
Comment #26
colan@xjm: That's what's in the patch already.
Comment #27
cilefen CreditAttribution: cilefen commentedComment #28
xjm@colan I see an entire docblock?
Edit: Sorry, somehow missed over the change of direction in the more recent patch.
Comment #29
daften CreditAttribution: daften at District09 commentedI've added a new patch, because my settings.php file had what @colan said.
Comment #30
colanThat looks better. Now we just need to update the summary.
Comment #31
colanSorry, let's keep the status until that's done.
Comment #32
wesleydv CreditAttribution: wesleydv commentedJust tried this and $config['system.file']['path']['temporary'] did not work for me.
However $config['system.file']['path.temporary'] did, is this something that changed in dev?
Comment #33
memtkmcc CreditAttribution: memtkmcc at Omega8.cc commentedHmm... just tested this with Drupal-8.2.4 on Aegir, where we use
$config['system.file']['path']['temporary']
and it works perfectly.Comment #34
wesleydv CreditAttribution: wesleydv commentedPlease ignore my previous comment $config['system.file']['path']['temporary'] does work for me.
However it appears that a cache rebuild is necessary even after a fresh install, don't know if this is by design.
Comment #36
fagoSame for me $config['system.file']['path']['temporary'] works while $config['system.file']['path.temporary'] does not. Updated patch attached.
Comment #37
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commentedI have fixed it, please review.
Comment #38
colan#37: You patch looks identical to #36. Please provide an interdiff to illustrate any differences when providing updated patches.
Comment #39
Munavijayalakshmi CreditAttribution: Munavijayalakshmi at Valuebound commentedComment #40
osopolarOverwriting the path setting in database (for below example it is
/home/example.com/tmp
) with$config['system.file']['path']['temporary'] = '/tmp';
gives me on the page http://localhost/admin/config/media/file-system the following error. It does not hurt, but it does not look nice:Comment #41
benjifisherThe patches in #36 and #37 add a section to settings.php. The earlier discussion rejected this approach in favor of adding a single line as in the patch from #29.
Please let's stick with the patch from #29 and please re-read the comments #30 and #31. The reason this issue is marked NW is that the issue summary needs an update. The patch (#29) is fine.
Comment #42
benjifisher@osopolar, are you sure that you had
/tmp
and not justtmp
?Comment #43
osopolarYes I am sure, I double checked again and I changed above comment as it was misleading. The setting is
/home/example.com/tmp
on live. When I go to http://localhost/admin/config/media/file-system I get the error messages as drupal still thinks the tmp dir is in/home/example.com/tmp
although I already set$config['system.file']['path']['temporary'] = '/tmp';
. The complete backtrace error message is:I think it should be easy to reproduce (without copying db from somewhere):
1. Create a directory on the local machine, set temp in file system settings to that directory and save
2. Delete directory and make sure that webserver-user does not have rights to create it (mkdir)
3. Set
$config['system.file']['path']['temporary']
to a valid directory and call settings form again.Comment #44
benjifisher@osopolar, I tried your steps and I see the same behavior.
To clarify, in Step 1 there were no overrides in
settings.php
and I used the admin UI (/admin/config/media/file-system
) to set the temp directory to/opt/foo
. In Step 2, I removed the directory/opt/foo
.In Step 3, there was no error when I reloaded the page. At that point, the temp directory on the form was set to
/opt/foo
. When I submitted the form, I got the error message you describe. (I seem to have errors hidden, but I see something similar in the log.)It seems to me that this is expected behavior: when I submit the form with an invalid temporary directory, I get an error message. If I overwrite that with a valid directory, it works fine.
It is a little confusing because Drupal is keeping track of two values: one in the database and one from
settings.php
. You can see each usingdrush cget system.file
with and without the--include-overridden
flag. The default value when you load/admin/config/media/file-system
, and the one changed if you submit that form, is the value in the database. I am sure that has been discussed at length. After giving it a little thought, I think it makes sense.At any rate, that behavior is not related to this issue, which concerns documentation. If you think it is still a problem, then please open a separate issue. You can add this one as a related issue so that it will be easier to follow the discussion in the future.
Comment #45
benjifisherI have updated the issue summary, as requested in #30 and #31.
I have hidden the patches from #36 and #37 and the interdiff in #39. Please use the patch from #29. As I said in #41, most of the comments since #31 are not really relevant.
Comment #46
cilefen CreditAttribution: cilefen commentedI updated credit. Please, based on the #29 patch, put the new config override in alphabetical order with the other example config overrides.
Comment #47
benjifisherComment #48
philnguyen CreditAttribution: philnguyen commentedI am going to write a patch for number 46 at the baltimore sprint
Comment #49
philnguyen CreditAttribution: philnguyen as a volunteer commentedThis is the new patch
Comment #50
dagmarThanks! but
system.theme
should be before thanuser.settings
.Comment #51
xlin CreditAttribution: xlin as a volunteer commented@philnguyen Temporary directory should be
/tmp
instead oftmp
.Comment #52
philnguyen CreditAttribution: philnguyen as a volunteer commentedThis is the new patch
Comment #53
tomogden CreditAttribution: tomogden commented#52 Looks good to our sprint review group.
Comment #54
dagmarThanks everyone!
Comment #55
tomogden CreditAttribution: tomogden commentedComment #56
robertloo CreditAttribution: robertloo commentedReviewed. Patch looks good
Comment #57
tomogden CreditAttribution: tomogden commentedComment #58
robertloo CreditAttribution: robertloo commentedReviewed entire ticket. There are no outstanding issues. #52 resolves the reported problem.
Comment #59
dhopki12 CreditAttribution: dhopki12 commentedAs a feature request I switched the priority to 'normal' (documentation link: https://www.drupal.org/core/issue-priority#normal )
Comment #60
YesCT CreditAttribution: YesCT commentedWhat is our confidence that if someone uncommented this, that it would work as we hope?
Did someone try uncommenting it and test it?
And document on this issue what they had to do besides uncommenting it?
I agree a block of inline comments is not needed, but it would be helpful to say on this issue what steps had to be done.
Comment #61
dagmarI did it. It works as expected.
The only thing is you have to be sure permission are correctly set.
Comment #63
lauriiiCommitted on stage at DrupalCon Baltimore 2017 c4c8481 and pushed to 8.4.x. Thanks everyone!
Comment #64
benjifisher@YesCT, @dagmar, just to be clear: when you uncommented the line, you did that in
settings.php
, right? The patch applies todefault.settings.php
.Comment #65
dagmarYes. Sorry I forgot to mention my tests. So I modified the settings.php file, and then played with the folder permissions to check if that settings was being picked.
If you don't have enough permissions, the case of the site breaks as expected.