configurable ownlcloud temp directory location #16636
Conversation
| * Get the temporary directory to store transfer data | ||
| * @return null|string Path to the temporary directory or null | ||
| */ | ||
| public function t_get_temp_dir() { |
There was a problem hiding this comment.
Function names need to be in camelCase as per contribution guidelines
mmattel
force-pushed
the
configurable_owncloud_temp_dir_II
branch
2 times, most recently
from
e1bd94a
to
9e11164
Compare
|
all changes requested done and squashed |
|
Note regarding the build failure: There were 2 failures:
1) Test\TempManager::testLogCantCreateFile
Failed asserting that '/tmp/oc_tmp_KxlMwF-.txt' is false.
/var/www/xxx/tests/lib/tempmanager.php:138
2) Test\TempManager::testLogCantCreateFolder
Failed asserting that '/tmp/oc_tmp_5TFKFA-folder/' is false.
/var/www/xxx/tests/lib/tempmanager.php:152These two failures also pop up when I have cloned a brand fresh master from git (tested yesterday and today), just finalizing the installation, without applying any PR or changing any code. |
|
The unit test failures are caused by the changes to the TempManager unit test, where the TempManager is not given the correct base directory. The baseDir used to get passed in during TempManager construction, but now the class is self-sufficient and so it does not get passed in. Perhaps the best solution would be to have an extra parameter to TempManager, to specify the baseDir. If specified as |
| // Get the temporary directory and log the path if loglevel is set to debug | ||
| // Info: based on the temp dir, further directories may be created unique to the instance | ||
| $temp = $this->gatherTempDir(); | ||
| \OCP\Util::writeLog('Core', 'Temporary directory set to: ' . ($temp ? $temp : 'NULL'), \OCP\Util::DEBUG); |
There was a problem hiding this comment.
Use $this->log to write log messages
There was a problem hiding this comment.
tried but:
Call to undefined method OC\\TempManager::log()
Will stay at the current code
|
@Xenopathic
I do not believe that, because as written, the test failed on blank and fresh git clone from master also, which was just finalized by the installing procedure having no single line of code changed. Just give it a try... |
changes as requested return false -> return null updates
mmattel
force-pushed
the
configurable_owncloud_temp_dir_II
branch
from
9e11164
to
652d9d5
Compare
|
A new inspection was created. |
|
Refer to this link for build results (access rights to CI server needed): Build result: FAILURE[...truncated 14 lines...] > git config remote.origin.url https://github.com/owncloud/core.git # timeout=10 > git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > git config remote.origin.url https://github.com/owncloud/core.git # timeout=10Fetching upstream changes from https://github.com/owncloud/core.gitusing GIT_SSH to set credentials using .gitcredentials to set credentials > git config --local credential.helper store --file=/tmp/git3763832391780320647.credentials # timeout=10 > git -c core.askpass=true fetch --tags --progress https://github.com/owncloud/core.git +refs/pull/:refs/remotes/origin/pr/ > git config --local --remove-section credential # timeout=10 > git rev-parse refs/remotes/origin/pr/16636/merge^{commit} # timeout=10 > git rev-parse refs/remotes/origin/origin/pr/16636/merge^{commit} # timeout=10Checking out Revision 74b7d141e32469189c0a4042c7960685f10b85f2 (refs/remotes/origin/pr/16636/merge) > git config core.sparsecheckout # timeout=10 > git checkout -f 74b7d141e32469189c0a4042c7960685f10b85f2 > git rev-list 6de87dbc5022e9b2dd1cc6bbdceb2b1a05e633c1 # timeout=10First time build. Skipping changelog. > git remote # timeout=10 > git submodule init # timeout=10 > git submodule sync # timeout=10 > git config --get remote.origin.url # timeout=10 > git submodule update --init --recursiveTriggering pull-request-analyser-ng-simple » SLAVEpull-request-analyser-ng-simple » SLAVE completed with result FAILURESetting status of 652d9d5 to FAILURE with url https://ci.owncloud.org/job/pull-request-analyser-ng-simple/12608/ and message: Build finished.💣 Test FAILed. 💣
|
|
@mmattel Other PRs are resulting in test success: https://ci.owncloud.org/job/pull-request-analyser-ng-simple/12605/ for example. The problem is, as I mentioned, the fact that before the unit tests passed a custom baseDir path into TempManager, but now the path is decided inside TempManager so the unit test does not have the chance to pass its custom path in, so there is a discrepancy. The unit tests that fail set what they think the baseDir to be to be unwritable ( |
|
|
||
| protected function setUp() { | ||
| parent::setUp(); | ||
|
|
||
| $this->baseDir = get_temp_dir() . $this->getUniqueID('/oc_tmp_test'); | ||
| $this->baseDir = $this->getManager()->getTempDir() . $this->getUniqueID('/oc_tmp_test'); |
There was a problem hiding this comment.
The line in question is here - this baseDir needs to get passed into TempManager somehow
|
@mmattel How can we help out here? @Xenopathic Any advise? |
|
The ability to override the temp directory for unit tests is absolutely critical. I'd suggests introducing a new method to TempManager that overrides the temp dir. This can be used in the unit tests. |
|
This is a good idea, but I currently do not have any free minute to do this. Just became a grandpa... 😄 |
|
@Xenopathic Can you help out and jump in? :) |
With this PR, the location of the temp directory used by owncloud can be configured.
Please see also PR #13031 (Configurabe owncloud temporary directory) which was closed due to rebasing problems.