Renames tasks directory to Tasks during upgrade #7939
Conversation
| if (is_dir(Config::$tasksdir) && is_writable(Config::$tasksdir) && is_writable(dirname(Config::$tasksdir))) { | ||
| rename(Config::$tasksdir, dirname(Config::$tasksdir) . DIRECTORY_SEPARATOR . 'Tasks_temp'); | ||
| rename(dirname(Config::$tasksdir) . DIRECTORY_SEPARATOR . 'Tasks_temp', dirname(Config::$tasksdir) . DIRECTORY_SEPARATOR . 'Tasks'); | ||
| $changes['tasksdir'] = dirname(Config::$tasksdir) . '/Tasks'; |
There was a problem hiding this comment.
I assume this should be $sourcedir?
There was a problem hiding this comment.
In 99.999% of cases, dirname(Config::$tasksdir) will be the same as Config::$sourcedir, yes. But in theory someone might have moved it, so it's best to use dirname(Config::$tasksdir) to be sure.
There was a problem hiding this comment.
I missed the dirname() call.
There was a problem hiding this comment.
Ah. Got it. Easy enough to overlook. 🙂
There was a problem hiding this comment.
Only thing I would suggest is maybe we check if we need to rename it or not. I wonder if realpath check would suffice. realpath($taskdir) != realpath(dirname($taskdir) . '/Tasks')?
There was a problem hiding this comment.
Ah. Yes, now I see what you mean. Hm... that does pose a bit of a problem, doesn't it? Got any ideas?
There was a problem hiding this comment.
Not sure yet. Windows doesn't care, it sees it as the same. Linux will see this as if they where named 2 different things. PHP we can use strcasecmp to check them. Maybe strcasecmp($taskdir) != strcasecmp(dirname($taskdir) . '/Tasks');?
There was a problem hiding this comment.
I think I figured out a way to consistently handle all cases.
There was a problem hiding this comment.
Oh, wait, nope. I thought realpath() would report the true case of the dir's name on case insensitive file systems. Apparently that was wrong, which means I need to think about this further. Ugh.
There was a problem hiding this comment.
Got it now. This is exactly what inodes are for.
Signed-off-by: Jon Stovell <[email protected]>
Sesquipedalian
force-pushed
the
tasks_uprader
branch
2 times, most recently
from
f72abdf to
a5c49db
Compare
Signed-off-by: Jon Stovell <[email protected]>
Sesquipedalian
force-pushed
the
tasks_uprader
branch
from
a5c49db to
83d7f4d
Compare
|
Honestly, just thinking on this. Do we really need to specify |
|
That's not a bad argument. But in that case, we need to add logic to the upgrader to move a custom tasks dir back to the standard location. |
|
At any rate, such a change would be Alpha 2, not Alpha 1. |
|
Ugh, I see this got closed. Apparently that happens automatically if you rename the branch that you want to merge. I'll open a new PR. |
Fixes #7932