Fix sub-command log level (#25537)

More fix for #24981

* #24981


Close #22361

* #22361

There were many patches for Gitea's sub-commands to satisfy the facts:

* Some sub-commands shouldn't output any log, otherwise the git protocol
would be broken
* Sometimes the users want to see "verbose" or "quiet" outputs

That's a longstanding problem, and very fragile. This PR is only a quick
patch for the problem.

In the future, the sub-command system should be refactored to a clear
solution.

----

Other changes:

* Use `ReplaceAllWriters` to replace
`RemoveAllWriters().AddWriters(writer)`, then it's an atomic operation.
* Remove unnecessary `syncLevelInternal` calls, because
`AddWriters/addWritersInternal` already calls it.

Co-authored-by: Giteabot <teabot@gitea.io>
This commit is contained in:
wxiaoguang 2023-06-28 14:02:06 +08:00 committed by GitHub
parent b943318617
commit 65d3e1161b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 39 additions and 18 deletions

View file

@ -106,5 +106,21 @@ func setupConsoleLogger(level log.Level, colorize bool, out io.Writer) {
WriterOption: log.WriterConsoleOption{Stderr: out == os.Stderr}, WriterOption: log.WriterConsoleOption{Stderr: out == os.Stderr},
} }
writer := log.NewEventWriterConsole("console-default", writeMode) writer := log.NewEventWriterConsole("console-default", writeMode)
log.GetManager().GetLogger(log.DEFAULT).RemoveAllWriters().AddWriters(writer) log.GetManager().GetLogger(log.DEFAULT).ReplaceAllWriters(writer)
}
// PrepareConsoleLoggerLevel by default, use INFO level for console logger, but some sub-commands (for git/ssh protocol) shouldn't output any log to stdout.
// Any log appears in git stdout pipe will break the git protocol, eg: client can't push and hangs forever.
func PrepareConsoleLoggerLevel(defaultLevel log.Level) func(*cli.Context) error {
return func(c *cli.Context) error {
level := defaultLevel
if c.Bool("quiet") || c.GlobalBoolT("quiet") {
level = log.FATAL
}
if c.Bool("debug") || c.GlobalBool("debug") || c.Bool("verbose") || c.GlobalBool("verbose") {
level = log.TRACE
}
log.SetConsoleLogger(log.DEFAULT, "console-default", level)
return nil
}
} }

View file

@ -151,7 +151,7 @@ func setupDoctorDefaultLogger(ctx *cli.Context, colorize bool) {
log.FallbackErrorf("unable to create file log writer: %v", err) log.FallbackErrorf("unable to create file log writer: %v", err)
return return
} }
log.GetManager().GetLogger(log.DEFAULT).RemoveAllWriters().AddWriters(writer) log.GetManager().GetLogger(log.DEFAULT).ReplaceAllWriters(writer)
} }
} }

View file

@ -15,6 +15,7 @@ import (
"time" "time"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/private" "code.gitea.io/gitea/modules/private"
repo_module "code.gitea.io/gitea/modules/repository" repo_module "code.gitea.io/gitea/modules/repository"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
@ -32,6 +33,7 @@ var (
Name: "hook", Name: "hook",
Usage: "Delegate commands to corresponding Git hooks", Usage: "Delegate commands to corresponding Git hooks",
Description: "This should only be called by Git", Description: "This should only be called by Git",
Before: PrepareConsoleLoggerLevel(log.FATAL),
Subcommands: []cli.Command{ Subcommands: []cli.Command{
subcmdHookPreReceive, subcmdHookPreReceive,
subcmdHookUpdate, subcmdHookUpdate,

View file

@ -8,6 +8,7 @@ import (
"fmt" "fmt"
"strings" "strings"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/private" "code.gitea.io/gitea/modules/private"
"github.com/urfave/cli" "github.com/urfave/cli"
@ -17,6 +18,7 @@ import (
var CmdKeys = cli.Command{ var CmdKeys = cli.Command{
Name: "keys", Name: "keys",
Usage: "This command queries the Gitea database to get the authorized command for a given ssh key fingerprint", Usage: "This command queries the Gitea database to get the authorized command for a given ssh key fingerprint",
Before: PrepareConsoleLoggerLevel(log.FATAL),
Action: runKeys, Action: runKeys,
Flags: []cli.Flag{ Flags: []cli.Flag{
cli.StringFlag{ cli.StringFlag{

View file

@ -44,6 +44,7 @@ var CmdServ = cli.Command{
Name: "serv", Name: "serv",
Usage: "This command should only be called by SSH shell", Usage: "This command should only be called by SSH shell",
Description: "Serv provides access auth for repositories", Description: "Serv provides access auth for repositories",
Before: PrepareConsoleLoggerLevel(log.FATAL),
Action: runServ, Action: runServ,
Flags: []cli.Flag{ Flags: []cli.Flag{
cli.BoolFlag{ cli.BoolFlag{

View file

@ -35,6 +35,7 @@ var CmdWeb = cli.Command{
Usage: "Start Gitea web server", Usage: "Start Gitea web server",
Description: `Gitea web server is the only thing you need to run, Description: `Gitea web server is the only thing you need to run,
and it takes care of all the other things for you`, and it takes care of all the other things for you`,
Before: PrepareConsoleLoggerLevel(log.INFO),
Action: runWeb, Action: runWeb,
Flags: []cli.Flag{ Flags: []cli.Flag{
cli.StringFlag{ cli.StringFlag{
@ -206,11 +207,6 @@ func servePprof() {
} }
func runWeb(ctx *cli.Context) error { func runWeb(ctx *cli.Context) error {
if ctx.Bool("verbose") {
setupConsoleLogger(log.TRACE, log.CanColorStdout, os.Stdout)
} else if ctx.Bool("quiet") {
setupConsoleLogger(log.FATAL, log.CanColorStdout, os.Stdout)
}
defer func() { defer func() {
if panicked := recover(); panicked != nil { if panicked := recover(); panicked != nil {
log.Fatal("PANIC: %v\n%s", panicked, log.Stack(2)) log.Fatal("PANIC: %v\n%s", panicked, log.Stack(2))

View file

@ -108,7 +108,9 @@ func main() {
cmd.CmdActions, cmd.CmdActions,
} }
// default configuration flags // shared configuration flags, they are for global and for each sub-command at the same time
// eg: such command is valid: "./gitea --config /tmp/app.ini web --config /tmp/app.ini", while it's discouraged indeed
// keep in mind that the short flags like "-C", "-c" and "-w" are globally polluted, they can't be used for sub-commands anymore.
globalFlags := []cli.Flag{ globalFlags := []cli.Flag{
cli.HelpFlag, cli.HelpFlag,
cli.StringFlag{ cli.StringFlag{
@ -128,9 +130,10 @@ func main() {
// Set the default to be equivalent to cmdWeb and add the default flags // Set the default to be equivalent to cmdWeb and add the default flags
app.Flags = append(app.Flags, globalFlags...) app.Flags = append(app.Flags, globalFlags...)
app.Flags = append(app.Flags, cmd.CmdWeb.Flags...) app.Flags = append(app.Flags, cmd.CmdWeb.Flags...) // TODO: the web flags polluted the global flags, they are not really global flags
app.Action = prepareWorkPathAndCustomConf(cmd.CmdWeb.Action) app.Action = prepareWorkPathAndCustomConf(cmd.CmdWeb.Action)
app.HideHelp = true // use our own help action to show helps (with more information like default config) app.HideHelp = true // use our own help action to show helps (with more information like default config)
app.Before = cmd.PrepareConsoleLoggerLevel(log.INFO)
app.Commands = append(app.Commands, cmdHelp) app.Commands = append(app.Commands, cmdHelp)
for i := range app.Commands { for i := range app.Commands {
prepareSubcommands(&app.Commands[i], globalFlags) prepareSubcommands(&app.Commands[i], globalFlags)

View file

@ -79,5 +79,5 @@ func SetConsoleLogger(loggerName, writerName string, level Level) {
Colorize: CanColorStdout, Colorize: CanColorStdout,
WriterOption: WriterConsoleOption{}, WriterOption: WriterConsoleOption{},
}) })
GetManager().GetLogger(loggerName).RemoveAllWriters().AddWriters(writer) GetManager().GetLogger(loggerName).ReplaceAllWriters(writer)
} }

View file

@ -96,7 +96,10 @@ func (l *LoggerImpl) removeWriterInternal(w EventWriter) {
func (l *LoggerImpl) AddWriters(writer ...EventWriter) { func (l *LoggerImpl) AddWriters(writer ...EventWriter) {
l.eventWriterMu.Lock() l.eventWriterMu.Lock()
defer l.eventWriterMu.Unlock() defer l.eventWriterMu.Unlock()
l.addWritersInternal(writer...)
}
func (l *LoggerImpl) addWritersInternal(writer ...EventWriter) {
for _, w := range writer { for _, w := range writer {
if old, ok := l.eventWriters[w.GetWriterName()]; ok { if old, ok := l.eventWriters[w.GetWriterName()]; ok {
l.removeWriterInternal(old) l.removeWriterInternal(old)
@ -126,8 +129,8 @@ func (l *LoggerImpl) RemoveWriter(modeName string) error {
return nil return nil
} }
// RemoveAllWriters removes all writers from the logger, non-shared writers are closed and flushed // ReplaceAllWriters replaces all writers from the logger, non-shared writers are closed and flushed
func (l *LoggerImpl) RemoveAllWriters() *LoggerImpl { func (l *LoggerImpl) ReplaceAllWriters(writer ...EventWriter) {
l.eventWriterMu.Lock() l.eventWriterMu.Lock()
defer l.eventWriterMu.Unlock() defer l.eventWriterMu.Unlock()
@ -135,8 +138,7 @@ func (l *LoggerImpl) RemoveAllWriters() *LoggerImpl {
l.removeWriterInternal(w) l.removeWriterInternal(w)
} }
l.eventWriters = map[string]EventWriter{} l.eventWriters = map[string]EventWriter{}
l.syncLevelInternal() l.addWritersInternal(writer...)
return l
} }
// DumpWriters dumps the writers as a JSON map, it's used for debugging and display purposes. // DumpWriters dumps the writers as a JSON map, it's used for debugging and display purposes.
@ -161,7 +163,7 @@ func (l *LoggerImpl) DumpWriters() map[string]any {
// Close closes the logger, non-shared writers are closed and flushed // Close closes the logger, non-shared writers are closed and flushed
func (l *LoggerImpl) Close() { func (l *LoggerImpl) Close() {
l.RemoveAllWriters() l.ReplaceAllWriters()
l.ctxCancel() l.ctxCancel()
} }
@ -233,7 +235,6 @@ func NewLoggerWithWriters(ctx context.Context, name string, writer ...EventWrite
l.ctx, l.ctxCancel = newProcessTypedContext(ctx, "Logger: "+name) l.ctx, l.ctxCancel = newProcessTypedContext(ctx, "Logger: "+name)
l.LevelLogger = BaseLoggerToGeneralLogger(l) l.LevelLogger = BaseLoggerToGeneralLogger(l)
l.eventWriters = map[string]EventWriter{} l.eventWriters = map[string]EventWriter{}
l.syncLevelInternal()
l.AddWriters(writer...) l.AddWriters(writer...)
return l return l
} }

View file

@ -23,7 +23,7 @@ func TestSharedWorker(t *testing.T) {
loggerTest := m.GetLogger("test") loggerTest := m.GetLogger("test")
loggerTest.AddWriters(w) loggerTest.AddWriters(w)
loggerTest.Info("msg-1") loggerTest.Info("msg-1")
loggerTest.RemoveAllWriters() // the shared writer is not closed here loggerTest.ReplaceAllWriters() // the shared writer is not closed here
loggerTest.Info("never seen") loggerTest.Info("never seen")
// the shared writer can still be used later // the shared writer can still be used later

View file

@ -244,7 +244,7 @@ func initLoggerByName(manager *log.LoggerManager, rootCfg ConfigProvider, logger
eventWriters = append(eventWriters, eventWriter) eventWriters = append(eventWriters, eventWriter)
} }
manager.GetLogger(loggerName).RemoveAllWriters().AddWriters(eventWriters...) manager.GetLogger(loggerName).ReplaceAllWriters(eventWriters...)
} }
func InitSQLLoggersForCli(level log.Level) { func InitSQLLoggersForCli(level log.Level) {