Skip to content
This repository has been archived by the owner on Feb 20, 2019. It is now read-only.

Move nginx update config block below static block #2651

Closed
wants to merge 1 commit into from
Closed

Move nginx update config block below static block #2651

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 2, 2016

Follow-up of the suggestion of @josh4trunks in #2551 (comment)

For me it seems i don't even need the fix from #2551 when moving the updater block below the static block .

@mention-bot
Copy link

@RealRancor, thanks for your PR! By analyzing the history of the files in this pull request, we identified @carlaschroder to be a potential reviewer.

@josh4trunks
Copy link
Contributor

@RealRancor
The only reason the block is even needed is to handle when someone browses to /updater or /updater/ or /ocs-provider or /ocs-provider/ to serve them the URL with an index.php
See https://github.com/owncloud/core/blob/master/lib/private/Setup.php#L462-L463

I'm trying to think if there is a better way to do this. What we have in this PR works.
But instead we could have something like the below makes the config longer but avoids regex. It also isn't location dependent so it could be just below the similar location / block.

location /updater {
        try_files $uri $uri/ =404;
        index index.php;
}
location /ocs-provider {
        try_files $uri $uri/ =404;
        index index.php;
}

@carlaschroder
Copy link
Contributor

I'm not touching this until you make up your minds :) Thanks for managing the nginx page, as I have no clue about nginx.

@josh4trunks
Copy link
Contributor

josh4trunks commented Oct 3, 2016

@RealRancor can decide! =P

@ghost
Copy link
Author

ghost commented Nov 10, 2016

Sorry, don't have time to test something (the updater app) which i don't use (and never will touch) thus i'm closing here. If some one want to take this over feel free.

@ghost ghost closed this Nov 10, 2016
@ghost ghost deleted the update_nginx_conf branch November 10, 2016 12:00
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants