Skip to content

Conversation

@gdamjan
Copy link
Contributor

@gdamjan gdamjan commented Jul 29, 2016

nextcloud by default uses the /config/ directory in the source/application tree for its config file(s). with this commit that directory can be overridden by the NEXTCLOUD_CONFIG_DIR environment variable.

in uwsgi, you would use the option --env "NEXTCLOUD_CONFIG_DIR=/tmp/nx-config/"
in apache use SetENV …
and the cli command can be run with: NEXTCLOUD_CONFIG_DIR=/tmp/nx-config ./occ (or just use export once in the shell).

NEXTCLOUD_CONFIG_DIR can be supplied with or without the trailing slash (/), but in all cases $configDir will have it automatically added if needed.

The other changes are several occurrences of OC::$SERVERROOT . '/config' to OC::$configDir.

@gdamjan
Copy link
Contributor Author

gdamjan commented Jul 29, 2016

ps, the only thing I was not sure about is autoconfig.php in SetupController.php. Do I have access to configDir here? or I should repeat the logic about NEXTCLOUD_CONFIG_DIR?

@gdamjan
Copy link
Contributor Author

gdamjan commented Jul 29, 2016

ps. I'm struglling a bit with getConfig in Server.php - brb

nextcloud by default uses the `/config/` directory in the source/application tree for its config file(s).
with this commit that directory can be overridden by the `NEXTCLOUD_CONFIG_DIR` environment variable.

in uwsgi, you would use the option `--env "NEXTCLOUD_CONFIG_DIR=/tmp/nx-config/"`
in apache `SetENV …`
and the cli command can be run with: `NEXTCLOUD_CONFIG_DIR=/tmp/nx-config ./occ` (or just use `export` once in the
shell).

NEXTCLOUD_CONFIG_DIR can be supplied with or without the trailing slash (`/`), but in all cases `$configDir` will have
it automatically added if needed.

The other changes are several occurrences of `OC::$SERVERROOT . '/config'` to `OC::$configDir`.
@schiessle
Copy link
Member

@gdamjan Thanks a lot for your pull request. Can you explain the use case of it? Thanks!

@gdamjan
Copy link
Contributor Author

gdamjan commented Jul 29, 2016

well, I've talked about it here #300 but the main issue is, I want to avoid the config file to be part of the nextcloud directory, since that directory is controlled by the package manager (and can be on a read-only filesystem etc).

@schiessle
Copy link
Member

@gdamjan thanks for providing the reference

@MorrisJobke MorrisJobke added this to the Nextcloud 11.0 milestone Jul 30, 2016
@MorrisJobke MorrisJobke added 3. to review Waiting for reviews enhancement labels Jul 30, 2016
@MorrisJobke
Copy link
Member

I think this makes sense. @LukasReschke are there any implications security wise? I can't see some, but just want to double check (especially regarding the ENV variable injection stuff that HTTPoxy did).

@gdamjan
Copy link
Contributor Author

gdamjan commented Jul 30, 2016

The environment variable injection would only inject variables starting with HTTP_. It was unfortunate that libraries/programs also used http_proxy :/

But in this case it can't be done.

@icewind1991
Copy link
Member

Looks good 👍

@MorrisJobke
Copy link
Member

@gdamjan There seems to be another occurrence in the setup controller. Could you also fix that? See https://github.com/nextcloud/server/pull/697/files

@gdamjan
Copy link
Contributor Author

gdamjan commented Aug 2, 2016

@MorrisJobke yes, I asked about that one several comments before.

should I modify it to use \OC::$configDir too?

@MorrisJobke
Copy link
Member

should I modify it to use \OC::$configDir too?

Makes sense. Thanks 😃

@kyrofa
Copy link
Member

kyrofa commented Aug 3, 2016

Great PR @gdamjan, thank you 👍

@oparoz
Copy link
Member

oparoz commented Aug 8, 2016

Just one question about the rumoured performance hit of doing it this way. Just a rumour?

@gdamjan
Copy link
Contributor Author

gdamjan commented Aug 8, 2016

the environment is stored in the process memory, getenv is not even a sys-call. just a search through a small table of variables. compared to everything nextcloud does, even what PHP does, I think it's less than negligible.

@MorrisJobke
Copy link
Member

Just one question about the rumoured performance hit of doing it this way. Just a rumour?

Yes. Performance wise this isn't really an issue.

@MorrisJobke
Copy link
Member

I tested this and it works 👍

@MorrisJobke MorrisJobke merged commit 4277051 into nextcloud:master Aug 9, 2016
@MorrisJobke
Copy link
Member

@gdamjan Thanks a lot for your contribution :) Feel free to join our IRC channel #nextcloud-dev on freenode :)

@gdamjan gdamjan deleted the custom-config-dir branch August 9, 2016 08:59
@gdamjan
Copy link
Contributor Author

gdamjan commented Aug 9, 2016

thanks all

gdamjan added a commit to gdamjan/uwsgi-deployments that referenced this pull request Aug 11, 2016
gdamjan added a commit to gdamjan/uwsgi-deployments that referenced this pull request Aug 11, 2016
GitHubUser4234 pushed a commit to GitHubUser4234/server that referenced this pull request Aug 30, 2016
introduce NEXTCLOUD_CONFIG_DIR env variable (see nextcloud#300)
R0Wi pushed a commit to R0Wi/server that referenced this pull request Nov 22, 2025
…ions/actions/setup-python-6.0.0

chore(deps): Bump actions/setup-python from 5.6.0 to 6.0.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants