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

Move install pages out of main macaron routes #13195

Merged
merged 6 commits into from
Oct 19, 2020

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Oct 18, 2020

Currently Gitea will simply try to run normally after the install page has completed. This unfortunately doesn't quite work with duplicate log entries and other problems.

This PR moves the install pages out of the main routes page and if installation is required starts a specific router for that before the main router.

It also fixes initial login for administrator user.
And starts the PProf server before the global init so that can be examined too.

Signed-off-by: Andrew Thornton art27@cantab.net

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath changed the title Move install pages out of main macaron loop Move install pages out of main macaron routes Oct 18, 2020
@zeripath zeripath added the type/enhancement An improvement of existing functionality label Oct 18, 2020
@zeripath zeripath added this to the 1.14.0 milestone Oct 18, 2020
@codecov-io
Copy link

Codecov Report

Merging #13195 into master will decrease coverage by 0.04%.
The diff coverage is 11.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13195      +/-   ##
==========================================
- Coverage   42.01%   41.97%   -0.05%     
==========================================
  Files         683      683              
  Lines       75215    75279      +64     
==========================================
- Hits        31605    31599       -6     
- Misses      38443    38515      +72     
+ Partials     5167     5165       -2     
Impacted Files Coverage Δ
cmd/web.go 0.00% <0.00%> (ø)
cmd/web_graceful.go 0.00% <0.00%> (ø)
modules/context/auth.go 19.10% <ø> (+0.62%) ⬆️
modules/graceful/manager.go 29.03% <ø> (ø)
modules/graceful/server.go 47.00% <0.00%> (ø)
routers/install.go 0.00% <0.00%> (ø)
routers/routes/routes.go 84.28% <0.00%> (-0.59%) ⬇️
routers/init.go 52.06% <30.18%> (-14.61%) ⬇️
services/pull/temp_repo.go 26.59% <0.00%> (-3.20%) ⬇️
services/pull/check.go 48.90% <0.00%> (-2.92%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 25b7766...903e6bc. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 18, 2020
@zeripath
Copy link
Contributor Author

Replaces #12696

Co-authored-by: Lauris BH <lauris@nix.lv>
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 18, 2020
templates/install.tmpl Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 19, 2020
@techknowlogick
Copy link
Member

🚀

@techknowlogick techknowlogick merged commit 2f1353a into go-gitea:master Oct 19, 2020
@zeripath zeripath deleted the install-before-routes branch October 19, 2020 21:16
ivanvc added a commit to ivanvc/gitea that referenced this pull request Oct 21, 2020
…ments-in-pull-request-label-style

* origin/master: (27 commits)
  [skip ci] Updated translations via Crowdin
  add more clarification to the issue-template.md (go-gitea#13235)
  go-version constraints ignore pre-releases (go-gitea#13234)
  [skip ci] Updated translations via Crowdin
  Update some JS dependencies (go-gitea#13222)
  Return the full rejection message and errors in flash errors (go-gitea#13221)
  Update heatmap fixtures to restore tests (go-gitea#13224)
  [skip ci] Updated translations via Crowdin
  Add review request api (go-gitea#11355)
  [skip ci] Updated translations via Crowdin
  When the git ref is unable to be found return broken pr (go-gitea#13218)
  Various arc-green fixes (go-gitea#13214)
  Show stale label for stale code comment which is marked as resolved (go-gitea#13213)
  Move install pages out of main macaron routes (go-gitea#13195)
  Use CSS Variables for fonts, remove postcss-loader (go-gitea#13204)
  [skip ci] Updated translations via Crowdin
  Align `SSH_AUTHORIZED_KEYS_BACKUP` var with the value in `app.ini` (go-gitea#13212)
  Fix size and clickable area on file table back link (go-gitea#13205)
  [skip ci] Updated translations via Crowdin
  Fix error in diff html rendering (go-gitea#13191)
  ...
zeripath added a commit to zeripath/gitea that referenced this pull request Oct 24, 2020
Unfortunately there was an error in go-gitea#13195 which set the --port
option before the settings were read. This PR fixes this by
moving applying this option to after the the settings are read

However, on looking further into this code I believe that the setPort
code was slightly odd.

Firstly, it may make sense to run the install page on a different
temporary port to the full system and this should be possible with
a --install-port option.

Secondy, if the --port option is provided we should apply it to both
otherwise there will be unusual behaviour on graceful restart

Thirdly, the documentation for --port says that the setting is
temporary - it should therefore not save its result to the configuration

(This however, does mean that authorized_keys and internal links may
not be correct. - I think we need to discuss this option further.)

Fix go-gitea#13277

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Oct 24, 2020
techknowlogick added a commit that referenced this pull request Oct 30, 2020
* Fix --port setting

Unfortunately there was an error in #13195 which set the --port
option before the settings were read. This PR fixes this by
moving applying this option to after the the settings are read

However, on looking further into this code I believe that the setPort
code was slightly odd.

Firstly, it may make sense to run the install page on a different
temporary port to the full system and this should be possible with
a --install-port option.

Secondy, if the --port option is provided we should apply it to both
otherwise there will be unusual behaviour on graceful restart

Thirdly, the documentation for --port says that the setting is
temporary - it should therefore not save its result to the configuration

(This however, does mean that authorized_keys and internal links may
not be correct. - I think we need to discuss this option further.)

Fix #13277

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Update cmd/web.go

* Apply suggestions from code review

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants