Skip to content
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

Configurable temporary directory #18658

Merged
merged 4 commits into from
Sep 12, 2015
Merged

Configurable temporary directory #18658

merged 4 commits into from
Sep 12, 2015

Conversation

RobinMcCorkell
Copy link
Member

This allows the temporary directory used by ownCloud to be configured in config.php.

Replaces #16636

Fixes #14886, fixes #12910

cc @mmattel @MorrisJobke @PVince81 @DeepDiver1975

fix for unit test

some fixes

if (!function_exists('get_temp_dir')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? I guess this can cause problems or wasn't there a reason to inclide this check?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ... you added it as an \OC method :)

@karlitschek
Copy link
Contributor

👍 from me beside the mentioned issues.

tmpBaseDir can be overridden for unit testing purposes
@RobinMcCorkell
Copy link
Member Author

@owncloud-bot retest this please

@RobinMcCorkell RobinMcCorkell added this to the 8.2-current milestone Aug 31, 2015
@karlitschek
Copy link
Contributor

@Xenopathic where are we with that? Would be nice to have it in 8.2

@RobinMcCorkell
Copy link
Member Author

Waiting for reviewers, cc @MorrisJobke

* @return string
* @since 8.2.0
*/
public function getTempBaseDir();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@icewind1991 Isn't there already a public method to get a temp directory? What is the difference here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FFrom what I can tell this is the base folder we create the other folders in, I dont know if we need to expose this as public

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the public API since the original get_temp_dir() was accessible.

@rullzer
Copy link
Contributor

rullzer commented Sep 6, 2015

Looks good. And a very useful addition.
Please add it also to the config.sample.php.

@mmattel
Copy link
Contributor

mmattel commented Sep 6, 2015

I have written a documentation some months ago: owncloud-archive/documentation#779
But this one got accidatially merged and reverted: owncloud-archive/documentation#1254
Maybe the revert can be reverted to get it implemented...
In my original PR core #16636, I have written a config.php.sampe text which also could be reused if wanted.

@scrutinizer-notifier
Copy link

A new inspection was created.

@RobinMcCorkell
Copy link
Member Author

@rullzer @mmattel Please review

@rullzer
Copy link
Contributor

rullzer commented Sep 12, 2015

Yep thanks for the example config.
👍

@karlitschek
Copy link
Contributor

👍

karlitschek added a commit that referenced this pull request Sep 12, 2015
@karlitschek karlitschek merged commit 7562e49 into master Sep 12, 2015
@MorrisJobke MorrisJobke deleted the configurable-temp branch September 13, 2015 08:51
if ($temp = sys_get_temp_dir()) return $temp;

return null;
return \OC::$server->getTempManager()->t_get_temp_dir();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t_get_temp_dir ?????? not know

bildschirmfoto von 2015-09-14 18-07-42

LukasReschke added a commit that referenced this pull request Jan 9, 2016
This has never worked before. Introduced with #18658
@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
8 participants