Backport #25537 More fix for #24981 * #24981 Close #22361, #25552 * #22361 * #25552 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.tags/v1.20.0
@@ -106,5 +106,21 @@ func setupConsoleLogger(level log.Level, colorize bool, out io.Writer) { | |||
WriterOption: log.WriterConsoleOption{Stderr: out == os.Stderr}, | |||
} | |||
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 | |||
} | |||
} |
@@ -151,7 +151,7 @@ func setupDoctorDefaultLogger(ctx *cli.Context, colorize bool) { | |||
log.FallbackErrorf("unable to create file log writer: %v", err) | |||
return | |||
} | |||
log.GetManager().GetLogger(log.DEFAULT).RemoveAllWriters().AddWriters(writer) | |||
log.GetManager().GetLogger(log.DEFAULT).ReplaceAllWriters(writer) | |||
} | |||
} | |||
@@ -15,6 +15,7 @@ import ( | |||
"time" | |||
"code.gitea.io/gitea/modules/git" | |||
"code.gitea.io/gitea/modules/log" | |||
"code.gitea.io/gitea/modules/private" | |||
repo_module "code.gitea.io/gitea/modules/repository" | |||
"code.gitea.io/gitea/modules/setting" | |||
@@ -32,6 +33,7 @@ var ( | |||
Name: "hook", | |||
Usage: "Delegate commands to corresponding Git hooks", | |||
Description: "This should only be called by Git", | |||
Before: PrepareConsoleLoggerLevel(log.FATAL), | |||
Subcommands: []cli.Command{ | |||
subcmdHookPreReceive, | |||
subcmdHookUpdate, |
@@ -8,6 +8,7 @@ import ( | |||
"fmt" | |||
"strings" | |||
"code.gitea.io/gitea/modules/log" | |||
"code.gitea.io/gitea/modules/private" | |||
"github.com/urfave/cli" | |||
@@ -17,6 +18,7 @@ import ( | |||
var CmdKeys = cli.Command{ | |||
Name: "keys", | |||
Usage: "This command queries the Gitea database to get the authorized command for a given ssh key fingerprint", | |||
Before: PrepareConsoleLoggerLevel(log.FATAL), | |||
Action: runKeys, | |||
Flags: []cli.Flag{ | |||
cli.StringFlag{ |
@@ -44,6 +44,7 @@ var CmdServ = cli.Command{ | |||
Name: "serv", | |||
Usage: "This command should only be called by SSH shell", | |||
Description: "Serv provides access auth for repositories", | |||
Before: PrepareConsoleLoggerLevel(log.FATAL), | |||
Action: runServ, | |||
Flags: []cli.Flag{ | |||
cli.BoolFlag{ |
@@ -35,6 +35,7 @@ var CmdWeb = cli.Command{ | |||
Usage: "Start Gitea web server", | |||
Description: `Gitea web server is the only thing you need to run, | |||
and it takes care of all the other things for you`, | |||
Before: PrepareConsoleLoggerLevel(log.INFO), | |||
Action: runWeb, | |||
Flags: []cli.Flag{ | |||
cli.StringFlag{ | |||
@@ -206,11 +207,6 @@ func servePprof() { | |||
} | |||
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() { | |||
if panicked := recover(); panicked != nil { | |||
log.Fatal("PANIC: %v\n%s", panicked, log.Stack(2)) |
@@ -108,7 +108,9 @@ func main() { | |||
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{ | |||
cli.HelpFlag, | |||
cli.StringFlag{ | |||
@@ -128,9 +130,10 @@ func main() { | |||
// Set the default to be equivalent to cmdWeb and add the default flags | |||
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.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) | |||
for i := range app.Commands { | |||
prepareSubcommands(&app.Commands[i], globalFlags) |
@@ -79,5 +79,5 @@ func SetConsoleLogger(loggerName, writerName string, level Level) { | |||
Colorize: CanColorStdout, | |||
WriterOption: WriterConsoleOption{}, | |||
}) | |||
GetManager().GetLogger(loggerName).RemoveAllWriters().AddWriters(writer) | |||
GetManager().GetLogger(loggerName).ReplaceAllWriters(writer) | |||
} |
@@ -96,7 +96,10 @@ func (l *LoggerImpl) removeWriterInternal(w EventWriter) { | |||
func (l *LoggerImpl) AddWriters(writer ...EventWriter) { | |||
l.eventWriterMu.Lock() | |||
defer l.eventWriterMu.Unlock() | |||
l.addWritersInternal(writer...) | |||
} | |||
func (l *LoggerImpl) addWritersInternal(writer ...EventWriter) { | |||
for _, w := range writer { | |||
if old, ok := l.eventWriters[w.GetWriterName()]; ok { | |||
l.removeWriterInternal(old) | |||
@@ -126,8 +129,8 @@ func (l *LoggerImpl) RemoveWriter(modeName string) error { | |||
return nil | |||
} | |||
// RemoveAllWriters removes all writers from the logger, non-shared writers are closed and flushed | |||
func (l *LoggerImpl) RemoveAllWriters() *LoggerImpl { | |||
// ReplaceAllWriters replaces all writers from the logger, non-shared writers are closed and flushed | |||
func (l *LoggerImpl) ReplaceAllWriters(writer ...EventWriter) { | |||
l.eventWriterMu.Lock() | |||
defer l.eventWriterMu.Unlock() | |||
@@ -135,8 +138,7 @@ func (l *LoggerImpl) RemoveAllWriters() *LoggerImpl { | |||
l.removeWriterInternal(w) | |||
} | |||
l.eventWriters = map[string]EventWriter{} | |||
l.syncLevelInternal() | |||
return l | |||
l.addWritersInternal(writer...) | |||
} | |||
// 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 | |||
func (l *LoggerImpl) Close() { | |||
l.RemoveAllWriters() | |||
l.ReplaceAllWriters() | |||
l.ctxCancel() | |||
} | |||
@@ -233,7 +235,6 @@ func NewLoggerWithWriters(ctx context.Context, name string, writer ...EventWrite | |||
l.ctx, l.ctxCancel = newProcessTypedContext(ctx, "Logger: "+name) | |||
l.LevelLogger = BaseLoggerToGeneralLogger(l) | |||
l.eventWriters = map[string]EventWriter{} | |||
l.syncLevelInternal() | |||
l.AddWriters(writer...) | |||
return l | |||
} |
@@ -23,7 +23,7 @@ func TestSharedWorker(t *testing.T) { | |||
loggerTest := m.GetLogger("test") | |||
loggerTest.AddWriters(w) | |||
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") | |||
// the shared writer can still be used later |
@@ -244,7 +244,7 @@ func initLoggerByName(manager *log.LoggerManager, rootCfg ConfigProvider, logger | |||
eventWriters = append(eventWriters, eventWriter) | |||
} | |||
manager.GetLogger(loggerName).RemoveAllWriters().AddWriters(eventWriters...) | |||
manager.GetLogger(loggerName).ReplaceAllWriters(eventWriters...) | |||
} | |||
func InitSQLLoggersForCli(level log.Level) { |