From ff159e947b6337653cbd7a06ddf9dd682ea0b2ea Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 1 Nov 2025 02:12:03 +0800 Subject: [PATCH] Fix cli "Before" handling (#35797) Regression of #34973 Fix #35796 --- cmd/cmd.go | 6 ++++ cmd/keys.go | 2 +- cmd/main.go | 10 +++++-- cmd/main_test.go | 48 +++++++++++++++++++++++------- tests/integration/cmd_keys_test.go | 5 +++- 5 files changed, 55 insertions(+), 16 deletions(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index 5b96bcbf9a91a..25e90a169501a 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -121,6 +121,12 @@ func globalBool(c *cli.Command, name string) bool { // 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(context.Context, *cli.Command) (context.Context, error) { return func(ctx context.Context, c *cli.Command) (context.Context, error) { + if setting.InstallLock { + // During config loading, there might also be logs (for example: deprecation warnings). + // It must make sure that console logger is set up before config is loaded. + log.Error("Config is loaded before console logger is setup, it will cause bugs. Please fix it.") + return nil, errors.New("console logger must be setup before config is loaded") + } level := defaultLevel if globalBool(c, "quiet") { level = log.FATAL diff --git a/cmd/keys.go b/cmd/keys.go index 5ca3b91e15e73..035d39bfb8625 100644 --- a/cmd/keys.go +++ b/cmd/keys.go @@ -19,7 +19,7 @@ import ( var CmdKeys = &cli.Command{ Name: "keys", Usage: "(internal) Should only be called by SSH server", - Hidden: true, // internal commands shouldn't not be visible + Hidden: true, // internal commands shouldn't be visible Description: "Queries the Gitea database to get the authorized command for a given ssh key fingerprint", Before: PrepareConsoleLoggerLevel(log.FATAL), Action: runKeys, diff --git a/cmd/main.go b/cmd/main.go index 3fdaf48ed9665..da979ee5115a8 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -50,11 +50,15 @@ DEFAULT CONFIGURATION: func prepareSubcommandWithGlobalFlags(originCmd *cli.Command) { originBefore := originCmd.Before - originCmd.Before = func(ctx context.Context, cmd *cli.Command) (context.Context, error) { - prepareWorkPathAndCustomConf(cmd) + originCmd.Before = func(ctxOrig context.Context, cmd *cli.Command) (ctx context.Context, err error) { + ctx = ctxOrig if originBefore != nil { - return originBefore(ctx, cmd) + ctx, err = originBefore(ctx, cmd) + if err != nil { + return ctx, err + } } + prepareWorkPathAndCustomConf(cmd) return ctx, nil } } diff --git a/cmd/main_test.go b/cmd/main_test.go index d49ebfd4df41d..69ea1237c63f0 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" "github.com/urfave/cli/v3" @@ -28,11 +29,11 @@ func makePathOutput(workPath, customPath, customConf string) string { return fmt.Sprintf("WorkPath=%s\nCustomPath=%s\nCustomConf=%s", workPath, customPath, customConf) } -func newTestApp(testCmdAction cli.ActionFunc) *cli.Command { +func newTestApp(testCmd cli.Command) *cli.Command { app := NewMainApp(AppVersion{}) - testCmd := &cli.Command{Name: "test-cmd", Action: testCmdAction} - prepareSubcommandWithGlobalFlags(testCmd) - app.Commands = append(app.Commands, testCmd) + testCmd.Name = util.IfZero(testCmd.Name, "test-cmd") + prepareSubcommandWithGlobalFlags(&testCmd) + app.Commands = append(app.Commands, &testCmd) app.DefaultCommand = testCmd.Name return app } @@ -156,9 +157,11 @@ func TestCliCmd(t *testing.T) { for _, c := range cases { t.Run(c.cmd, func(t *testing.T) { - app := newTestApp(func(ctx context.Context, cmd *cli.Command) error { - _, _ = fmt.Fprint(cmd.Root().Writer, makePathOutput(setting.AppWorkPath, setting.CustomPath, setting.CustomConf)) - return nil + app := newTestApp(cli.Command{ + Action: func(ctx context.Context, cmd *cli.Command) error { + _, _ = fmt.Fprint(cmd.Root().Writer, makePathOutput(setting.AppWorkPath, setting.CustomPath, setting.CustomConf)) + return nil + }, }) for k, v := range c.env { t.Setenv(k, v) @@ -173,31 +176,54 @@ func TestCliCmd(t *testing.T) { } func TestCliCmdError(t *testing.T) { - app := newTestApp(func(ctx context.Context, cmd *cli.Command) error { return errors.New("normal error") }) + app := newTestApp(cli.Command{Action: func(ctx context.Context, cmd *cli.Command) error { return errors.New("normal error") }}) r, err := runTestApp(app, "./gitea", "test-cmd") assert.Error(t, err) assert.Equal(t, 1, r.ExitCode) assert.Empty(t, r.Stdout) assert.Equal(t, "Command error: normal error\n", r.Stderr) - app = newTestApp(func(ctx context.Context, cmd *cli.Command) error { return cli.Exit("exit error", 2) }) + app = newTestApp(cli.Command{Action: func(ctx context.Context, cmd *cli.Command) error { return cli.Exit("exit error", 2) }}) r, err = runTestApp(app, "./gitea", "test-cmd") assert.Error(t, err) assert.Equal(t, 2, r.ExitCode) assert.Empty(t, r.Stdout) assert.Equal(t, "exit error\n", r.Stderr) - app = newTestApp(func(ctx context.Context, cmd *cli.Command) error { return nil }) + app = newTestApp(cli.Command{Action: func(ctx context.Context, cmd *cli.Command) error { return nil }}) r, err = runTestApp(app, "./gitea", "test-cmd", "--no-such") assert.Error(t, err) assert.Equal(t, 1, r.ExitCode) assert.Empty(t, r.Stdout) assert.Equal(t, "Incorrect Usage: flag provided but not defined: -no-such\n\n", r.Stderr) - app = newTestApp(func(ctx context.Context, cmd *cli.Command) error { return nil }) + app = newTestApp(cli.Command{Action: func(ctx context.Context, cmd *cli.Command) error { return nil }}) r, err = runTestApp(app, "./gitea", "test-cmd") assert.NoError(t, err) assert.Equal(t, -1, r.ExitCode) // the cli.OsExiter is not called assert.Empty(t, r.Stdout) assert.Empty(t, r.Stderr) } + +func TestCliCmdBefore(t *testing.T) { + ctxNew := context.WithValue(context.Background(), any("key"), "value") + configValues := map[string]string{} + setting.CustomConf = "/tmp/any.ini" + var actionCtx context.Context + app := newTestApp(cli.Command{ + Before: func(context.Context, *cli.Command) (context.Context, error) { + configValues["before"] = setting.CustomConf + return ctxNew, nil + }, + Action: func(ctx context.Context, cmd *cli.Command) error { + configValues["action"] = setting.CustomConf + actionCtx = ctx + return nil + }, + }) + _, err := runTestApp(app, "./gitea", "--config", "/dev/null", "test-cmd") + assert.NoError(t, err) + assert.Equal(t, ctxNew, actionCtx) + assert.Equal(t, "/tmp/any.ini", configValues["before"], "BeforeFunc must be called before preparing config") + assert.Equal(t, "/dev/null", configValues["action"]) +} diff --git a/tests/integration/cmd_keys_test.go b/tests/integration/cmd_keys_test.go index be226a01ecc5f..d911bdf17d2d3 100644 --- a/tests/integration/cmd_keys_test.go +++ b/tests/integration/cmd_keys_test.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/cmd" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" @@ -36,13 +37,15 @@ func Test_CmdKeys(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // FIXME: this test is not quite right. Each "command run" always re-initializes settings + defer test.MockVariableValue(&cmd.CmdKeys.Before, nil)() // don't re-initialize logger during the test + var stdout, stderr bytes.Buffer app := &cli.Command{ Writer: &stdout, ErrWriter: &stderr, Commands: []*cli.Command{cmd.CmdKeys}, } - cmd.CmdKeys.HideHelp = true err := app.Run(t.Context(), append([]string{"prog"}, tt.args...)) if tt.wantErr { assert.Error(t, err)