Skip to content

fix: redirect early CLI console logger to stderr#37507

Merged
bircni merged 8 commits into
go-gitea:mainfrom
tht:fix/dump-stdout-log-pollution
May 2, 2026
Merged

fix: redirect early CLI console logger to stderr#37507
bircni merged 8 commits into
go-gitea:mainfrom
tht:fix/dump-stdout-log-pollution

Conversation

@tht
Copy link
Copy Markdown
Contributor

@tht tht commented May 2, 2026 •
edited by bircni
Loading

When running gitea dump with output routed to stdout (--file -), deprecation warnings from loadAvatarsFrom were written to stdout, corrupting the archive stream.

Root cause: PrepareConsoleLoggerLevel (called in app.Before) sets up a console logger via SetConsoleLogger, which used WriterConsoleOption{} defaulting Stderr to false (i.e. stdout). This logger is installed before the dump subcommand can redirect logging to stderr in runDump.

Fix: use WriterConsoleOption{Stderr: true} in SetConsoleLogger so all early CLI diagnostic output goes to stderr from the start. This is correct for all subcommands — diagnostic/log output should never pollute stdout.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 2, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented May 2, 2026

I think it needs some comments, otherwise some days later maybe some people would change it back to stdout again.

@tht
Copy link
Copy Markdown
Contributor Author

tht commented May 2, 2026

IMHO it's just about being consistent. There are already log-messages on STDERR when doing a dump. Only these deprecation warnings go to stdout. But I get it, some comments would not hurt.

@tht @claude
fix: redirect early CLI console logger to stderr
85cd94a 
When running `gitea dump` with output routed to stdout (--file -),
deprecation warnings from loadAvatarsFrom were written to stdout,
corrupting the archive stream.

Root cause: PrepareConsoleLoggerLevel (called in app.Before) sets up
a console logger via SetConsoleLogger, which used WriterConsoleOption{}
defaulting Stderr to false (i.e. stdout). This logger is installed before
the dump subcommand can redirect logging to stderr in runDump.

Fix: use WriterConsoleOption{Stderr: true} in SetConsoleLogger so all
early CLI diagnostic output goes to stderr from the start. This is correct
for all subcommands — diagnostic/log output should never pollute stdout.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@tht tht force-pushed the fix/dump-stdout-log-pollution branch from 7b3c2e1 to 85cd94a Compare May 2, 2026 13:57
@tht
Copy link
Copy Markdown
Contributor Author

tht commented May 2, 2026

It may be a bit too verbose but it clearly describes why it needs to be STDERR.

wxiaoguang
wxiaoguang reviewed May 2, 2026
View reviewed changes
Comment thread modules/log/logger_global.go
@wxiaoguang
Update modules/log/logger_global.go
fdc7db2 
Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
wxiaoguang
wxiaoguang reviewed May 2, 2026
View reviewed changes
Comment thread modules/log/logger_global.go Outdated
@wxiaoguang
Update modules/log/logger_global.go
c7ca88c 
Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
@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 May 2, 2026
@tht
Copy link
Copy Markdown
Contributor Author

tht commented May 2, 2026

Thanks @wxiaoguang for looking into this. It actually broke my backup because of the output to STDOUT. I fixed it by removing the deprecated fields from the config.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented May 2, 2026

Added more comments.

More complicated than it looks

@wxiaoguang wxiaoguang force-pushed the fix/dump-stdout-log-pollution branch from f499450 to dbb3540 Compare May 2, 2026 14:52
@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 May 2, 2026
@bircni bircni enabled auto-merge (squash) May 2, 2026 15:23
@bircni bircni merged commit f049668 into go-gitea:main May 2, 2026
21 of 22 checks passed
@GiteaBot GiteaBot added this to the 1.27.0 milestone May 2, 2026
silverwind added a commit to McMichalK/gitea that referenced this pull request May 5, 2026
@silverwind
Merge remote-tracking branch 'origin/main' into filter-by-file-extension
9c8cf2a 
* origin/main: (49 commits)
  ci: lint PR titles with commitlint (go-gitea#37498)
  Make ServeSetHeaders default to download attachment if filename exists (go-gitea#37552)
  fix(actions): validate workflow param to prevent 500 error (go-gitea#37546)
  Fix various problems (go-gitea#37547)
  docs: fix 4 typos in CHANGELOG.md (go-gitea#37549)
  [skip ci] Updated translations via Crowdin
  chore(deps): update action dependencies (go-gitea#37540)
  fix: Fix `nolyfill` for renovate (go-gitea#37537)
  Refactor pull request view (7) (go-gitea#37524)
  Update go js py dependencies (go-gitea#37525)
  Don't unblock run-level-concurrency-blocked runs in the resolver (go-gitea#37461)
  Refactor pull request view (6) (go-gitea#37522)
  Refactor pull request view (5) (go-gitea#37517)
  fix: persist mirror repository metadata (go-gitea#37519)
  fix(packages): use file names for generic web downloads (go-gitea#37514)
  fix: merge autodetect can't close other PRs but only the last one when multiple PRs are pushed at once (go-gitea#37512)
  Fix update branch protection order (go-gitea#37508)
  Refactor "flex-list" to "flex-divided-list" (go-gitea#37505)
  fix: redirect early CLI console logger to stderr (go-gitea#37507)
  Fix mCaptcha broken after Vite migration (go-gitea#37492)
  ...

# Conflicts:
#	templates/repo/diff/box.tmpl
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 6, 2026
@zjjhot
Merge branch 'main' into mainzjj
7a4c3ea 
* main: (36 commits)
  refactor(deps): migrate from `nektos/act` fork to `gitea/runner` (go-gitea#37557)
  ci: lint PR titles with commitlint (go-gitea#37498)
  Make ServeSetHeaders default to download attachment if filename exists (go-gitea#37552)
  fix(actions): validate workflow param to prevent 500 error (go-gitea#37546)
  Fix various problems (go-gitea#37547)
  docs: fix 4 typos in CHANGELOG.md (go-gitea#37549)
  [skip ci] Updated translations via Crowdin
  chore(deps): update action dependencies (go-gitea#37540)
  fix: Fix `nolyfill` for renovate (go-gitea#37537)
  Refactor pull request view (7) (go-gitea#37524)
  Update go js py dependencies (go-gitea#37525)
  Don't unblock run-level-concurrency-blocked runs in the resolver (go-gitea#37461)
  Refactor pull request view (6) (go-gitea#37522)
  Refactor pull request view (5) (go-gitea#37517)
  fix: persist mirror repository metadata (go-gitea#37519)
  fix(packages): use file names for generic web downloads (go-gitea#37514)
  fix: merge autodetect can't close other PRs but only the last one when multiple PRs are pushed at once (go-gitea#37512)
  Fix update branch protection order (go-gitea#37508)
  Refactor "flex-list" to "flex-divided-list" (go-gitea#37505)
  fix: redirect early CLI console logger to stderr (go-gitea#37507)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@bircni bircni bircni approved these changes

Assignees

No one assigned

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore.

Projects

None yet

Milestone

1.27.0

Development

Successfully merging this pull request may close these issues.

4 participants

@tht @wxiaoguang @bircni @GiteaBot