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>

diff --git a/cmd/cmd.go b/cmd/cmd.go
index 8076acecaa257a7e2801c26b1669c42999b8a68f..4ed636a9b4cc5c3c87b0aef7a47a0c56f75f48de 100644 (file)
--- a/cmd/cmd.go
+++ b/cmd/cmd.go
@@ -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
+       }
 }

diff --git a/cmd/doctor.go b/cmd/doctor.go
index b79436fc0a9fae78d843e20bf543f7411b2e60f0..cd5f125e20e4e197a72e53044134b10e22323ed3 100644 (file)
--- a/cmd/doctor.go
+++ b/cmd/doctor.go
@@ -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)
        }
 }
 

diff --git a/cmd/hook.go b/cmd/hook.go
index 64532678328357649d65e6d8a08d052c481359ce..ed6efc19ea2e147650b1b5a4243182a50c966cd0 100644 (file)
--- a/cmd/hook.go
+++ b/cmd/hook.go
@@ -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,

diff --git a/cmd/keys.go b/cmd/keys.go
index deb94fca5d6abdf7b8f5b4d69af779d10db1861b..8aeee37527da45fb749b8da12874e373615c8c28 100644 (file)
--- a/cmd/keys.go
+++ b/cmd/keys.go
@@ -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{

diff --git a/cmd/serv.go b/cmd/serv.go
index 01102d3800c0031196d0a4b9b9c3dc590dc6097f..79052e58a8f56eac3f5133550c69b57d298f458d 100644 (file)
--- a/cmd/serv.go
+++ b/cmd/serv.go
@@ -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{

diff --git a/cmd/web.go b/cmd/web.go
index 7a257a62a277d94cab86a4b62f47d325877dba03..05f3b2ddb2eaa324aebd1d8ba2b19e9d37e59789 100644 (file)
--- a/cmd/web.go
+++ b/cmd/web.go
@@ -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))

diff --git a/main.go b/main.go
index 7b447e7533f0dbc2fd31ec4531993be7dc319f1a..f604311f54f227ecaba79ae7587b2c0a30ac35ce 100644 (file)
--- a/main.go
+++ b/main.go
@@ -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)

diff --git a/modules/log/logger_global.go b/modules/log/logger_global.go
index 5ccef34b5b7566b3914e743060abaa87b957e0bd..994acfedbb3a16da90b17f512f85c2d69574415c 100644 (file)
--- a/modules/log/logger_global.go
+++ b/modules/log/logger_global.go
@@ -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)
 }

diff --git a/modules/log/logger_impl.go b/modules/log/logger_impl.go
index 903d8cefc2a5a600d8940026a8a7e0289731a25b..d38c6516ed5cefe8d143078539e26320c1cb9264 100644 (file)
--- a/modules/log/logger_impl.go
+++ b/modules/log/logger_impl.go
@@ -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
 }

diff --git a/modules/log/manager_test.go b/modules/log/manager_test.go
index aa01f79980c596061645dc7100cafa92c5c9d5ba..b8fbf846133c547a0b0c059c7f1a8c898c6e6592 100644 (file)
--- a/modules/log/manager_test.go
+++ b/modules/log/manager_test.go
@@ -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

diff --git a/modules/setting/log.go b/modules/setting/log.go
index af64ea8d85373d02ce27c00fe0856bca41054ee6..66206f8f4b22a0f1dcb9be4c11ff095125dc33e4 100644 (file)
--- a/modules/setting/log.go
+++ b/modules/setting/log.go
@@ -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) {