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

In gitea serv switch off console logger to fix #5866 #5887

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Jan 28, 2019

By default, if setting.NewContext() prints out any warning logs, these are printed to the stdout breaking git receive-pack etc. meaning that even if there is a warning because of a minor problem in your app.ini but gitea starts despite this - you CANNOT push or pull over SSH.

This PR disables the console logger whilst in serv.go

Fix #5866

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

By default warning logs are printed to the stdout which breaks a git receive-pack etc.
This PR disables the console logger whilst in serv.go

Signed-off-by: Andrew Thornton <art27@cantab.net>
@codecov-io
Copy link

codecov-io commented Jan 28, 2019

Codecov Report

Merging #5887 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5887      +/-   ##
==========================================
+ Coverage   38.27%   38.29%   +0.01%     
==========================================
  Files         330      330              
  Lines       48518    48518              
==========================================
+ Hits        18572    18578       +6     
+ Misses      27273    27266       -7     
- Partials     2673     2674       +1
Impacted Files Coverage Δ
routers/repo/view.go 47.3% <0%> (+1.19%) ⬆️
models/unit.go 14.28% <0%> (+14.28%) ⬆️

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 f81c6cc...1b1bc05. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 28, 2019
@techknowlogick techknowlogick added this to the 1.8.0 milestone Jan 30, 2019
@lunny
Copy link
Member

lunny commented Feb 1, 2019

Maybe del the logger after setting.NewContext()?

@zeripath
Copy link
Contributor Author

zeripath commented Feb 1, 2019

setting.NewContext() is emitting the warnings. Hence the console logger must be deleted before this.

I think it's fine for Gitea serv to be silent for these warnings - it's not meant to be used by humans. Emitting anything on the console will break git causing it to hang on the client.

@zeripath
Copy link
Contributor Author

zeripath commented Feb 2, 2019

Basically without this PR - If warnings are emitted because of a minor problem in app.ini that doesn't stop gitea from running - you cannot push or pull over ssh.

@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 Feb 3, 2019
@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 Feb 3, 2019
@zeripath zeripath merged commit 2902b3a into go-gitea:master Feb 3, 2019
@zeripath zeripath deleted the issue-5866-switch-off-console-logging-in-serv branch February 3, 2019 11:19
zeripath added a commit to zeripath/gitea that referenced this pull request Feb 3, 2019
By default, if `setting.NewContext()` prints out any warning logs, these are printed to the stdout breaking `git receive-pack` etc. meaning that even if there is a warning because of a minor problem in your app.ini but gitea starts despite this - you **CANNOT** push or pull over SSH.

This PR disables the console logger whilst in `serv.go`

Signed-off-by: Andrew Thornton <art27@cantab.net>
@techknowlogick techknowlogick added the backport/done All backports for this PR have been created label Feb 3, 2019
techknowlogick pushed a commit that referenced this pull request Feb 3, 2019
By default, if `setting.NewContext()` prints out any warning logs, these are printed to the stdout breaking `git receive-pack` etc. meaning that even if there is a warning because of a minor problem in your app.ini but gitea starts despite this - you **CANNOT** push or pull over SSH.

This PR disables the console logger whilst in `serv.go`

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request May 26, 2019
7 tasks
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
@GiteaBot GiteaBot added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label May 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git clone via SSH hangs when markup.asciidoc is partially defined in config
7 participants