-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow creating of missing tmp directories in debug (development) mode #1194
Conversation
… for cache and log to avoid unnecessary errors thrown
Every build failed in travis. At least some should pass before it can be merged in. |
Any idea what could be causing it? Or what we can do here? |
Looks like it is related to file permissions. Perhaps the new directories aren't getting the correct permissions set? |
A remaining problem is: "CacheTest::testInvalidConfig In the tests it creates this invalid path "webroot/tmpdata" even though the chmod rights should not be sufficient enough here. |
if (Configure::read('debug')) { | ||
$path = $dir->getPathname(); | ||
if (!is_dir($path)) { | ||
mkdir($path, 0775, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it worth checking the return value, and log an error if it can't be created?
Maybe also considering to use Folder::create()
- it does the "logging" for you, and more sanity checks
This pull request has a few open comments & failing tests. The current plan is to start the RC process for 2.4 next week. If you'd still like this to be part of 2.4 could you resolve the remaining issues. |
… for cache and log to avoid unnecessary errors thrown - using 0775 for dirs and 0664 for files
Tests pass - squashed. |
Allow creating of missing tmp directories in debug (development) mode
@dereuromark Can you upgrade the migration guide + docs? |
Sure. PR: cakephp/docs#677 |
Thx for @tigrang who pointed me in the right direction. |
This ticket can be closed now: |
Sooo.. my comments was disregarded ? :| |
It is just for debug mode - and only to avoid errors thrown. Not sure if we should create the overhead with Folder class here then. Also: why logging this if you will get a notice/warning anyway when it fails in debug with a verbose description of what failed - on the screen? |
Folder's 'logging' is hardly useful in this context, which is why I didn't wait before merging it. |
I don't read any insisting @dereuromark only "why was there no reply to my comment". If it was read, weighed and decided against - there's no way to tell that from "". |
Yeah, I had the same reasoning as @markstory mentioned. Sry for not expressing that more verbosely. |
Allow creating of missing tmp directories in debug (development) mode for cache and log to avoid unnecessary errors thrown.