From 52b319bc00712da095ee4121b616be232b1e455b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 20 Dec 2024 12:17:14 +0800 Subject: [PATCH 01/18] Refactor pprof labels and process desc (#32909) * Deprecate "gopid" in log, it is not useful and requires very hacky approach * Remove "git.Command.SetDescription" because it is not useful and only makes the logs too flexible --- modules/git/batch_reader.go | 18 ------- modules/git/blame.go | 5 +- modules/git/command.go | 68 +++++++++++++------------- modules/git/command_test.go | 6 +-- modules/git/repo.go | 13 ----- modules/graceful/manager.go | 7 +-- modules/graceful/manager_common.go | 16 +++--- modules/gtprof/gtprof.go | 25 ++++++++++ modules/log/event_format.go | 26 +++++----- modules/log/event_format_test.go | 30 ++++++------ modules/log/flags.go | 2 +- modules/log/groutinelabel.go | 19 ------- modules/log/groutinelabel_test.go | 33 ------------- modules/log/logger_impl.go | 5 -- modules/process/manager.go | 21 +++----- modules/process/manager_stacktraces.go | 14 +++--- modules/queue/workerqueue.go | 4 ++ modules/util/runtime.go | 13 +++++ modules/util/runtime_test.go | 32 ++++++++++++ routers/web/repo/githttp.go | 1 - services/gitdiff/gitdiff.go | 1 - services/mirror/mirror_pull.go | 43 +++++----------- services/mirror/mirror_push.go | 6 --- services/release/release.go | 1 - services/repository/adopt.go | 1 - services/repository/check.go | 3 +- services/repository/create.go | 6 +-- services/repository/fork.go | 2 - services/repository/generate.go | 2 - services/repository/init.go | 5 +- services/repository/migrate.go | 1 - 31 files changed, 182 insertions(+), 247 deletions(-) create mode 100644 modules/gtprof/gtprof.go delete mode 100644 modules/log/groutinelabel.go delete mode 100644 modules/log/groutinelabel_test.go create mode 100644 modules/util/runtime.go create mode 100644 modules/util/runtime_test.go diff --git a/modules/git/batch_reader.go b/modules/git/batch_reader.go index 7dfda72155..532dbad989 100644 --- a/modules/git/batch_reader.go +++ b/modules/git/batch_reader.go @@ -7,10 +7,8 @@ import ( "bufio" "bytes" "context" - "fmt" "io" "math" - "runtime" "strconv" "strings" @@ -32,7 +30,6 @@ type WriteCloserError interface { func ensureValidGitRepository(ctx context.Context, repoPath string) error { stderr := strings.Builder{} err := NewCommand(ctx, "rev-parse"). - SetDescription(fmt.Sprintf("%s rev-parse [repo_path: %s]", GitExecutable, repoPath)). Run(&RunOpts{ Dir: repoPath, Stderr: &stderr, @@ -62,13 +59,9 @@ func catFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, cancel() }() - _, filename, line, _ := runtime.Caller(2) - filename = strings.TrimPrefix(filename, callerPrefix) - go func() { stderr := strings.Builder{} err := NewCommand(ctx, "cat-file", "--batch-check"). - SetDescription(fmt.Sprintf("%s cat-file --batch-check [repo_path: %s] (%s:%d)", GitExecutable, repoPath, filename, line)). Run(&RunOpts{ Dir: repoPath, Stdin: batchStdinReader, @@ -114,13 +107,9 @@ func catFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufi cancel() }() - _, filename, line, _ := runtime.Caller(2) - filename = strings.TrimPrefix(filename, callerPrefix) - go func() { stderr := strings.Builder{} err := NewCommand(ctx, "cat-file", "--batch"). - SetDescription(fmt.Sprintf("%s cat-file --batch [repo_path: %s] (%s:%d)", GitExecutable, repoPath, filename, line)). Run(&RunOpts{ Dir: repoPath, Stdin: batchStdinReader, @@ -320,13 +309,6 @@ func ParseTreeLine(objectFormat ObjectFormat, rd *bufio.Reader, modeBuf, fnameBu return mode, fname, sha, n, err } -var callerPrefix string - -func init() { - _, filename, _, _ := runtime.Caller(0) - callerPrefix = strings.TrimSuffix(filename, "modules/git/batch_reader.go") -} - func DiscardFull(rd *bufio.Reader, discard int64) error { if discard > math.MaxInt32 { n, err := rd.Discard(math.MaxInt32) diff --git a/modules/git/blame.go b/modules/git/blame.go index a9b2706f21..cad720edf4 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -7,7 +7,6 @@ import ( "bufio" "bytes" "context" - "fmt" "io" "os" @@ -142,9 +141,7 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath // There is no equivalent on Windows. May be implemented if Gitea uses an external git backend. cmd.AddOptionValues("--ignore-revs-file", *ignoreRevsFile) } - cmd.AddDynamicArguments(commit.ID.String()). - AddDashesAndList(file). - SetDescription(fmt.Sprintf("GetBlame [repo_path: %s]", repoPath)) + cmd.AddDynamicArguments(commit.ID.String()).AddDashesAndList(file) reader, stdout, err := os.Pipe() if err != nil { if ignoreRevsFile != nil { diff --git a/modules/git/command.go b/modules/git/command.go index 22cb275ab2..b231c3beea 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -12,6 +12,7 @@ import ( "io" "os" "os/exec" + "path/filepath" "runtime" "strings" "time" @@ -43,18 +44,24 @@ type Command struct { prog string args []string parentContext context.Context - desc string globalArgsLength int brokenArgs []string } -func (c *Command) String() string { - return c.toString(false) +func logArgSanitize(arg string) string { + if strings.Contains(arg, "://") && strings.Contains(arg, "@") { + return util.SanitizeCredentialURLs(arg) + } else if filepath.IsAbs(arg) { + base := filepath.Base(arg) + dir := filepath.Dir(arg) + return filepath.Join(filepath.Base(dir), base) + } + return arg } -func (c *Command) toString(sanitizing bool) string { +func (c *Command) LogString() string { // WARNING: this function is for debugging purposes only. It's much better than old code (which only joins args with space), - // It's impossible to make a simple and 100% correct implementation of argument quoting for different platforms. + // It's impossible to make a simple and 100% correct implementation of argument quoting for different platforms here. debugQuote := func(s string) string { if strings.ContainsAny(s, " `'\"\t\r\n") { return fmt.Sprintf("%q", s) @@ -63,12 +70,11 @@ func (c *Command) toString(sanitizing bool) string { } a := make([]string, 0, len(c.args)+1) a = append(a, debugQuote(c.prog)) - for _, arg := range c.args { - if sanitizing && (strings.Contains(arg, "://") && strings.Contains(arg, "@")) { - a = append(a, debugQuote(util.SanitizeCredentialURLs(arg))) - } else { - a = append(a, debugQuote(arg)) - } + if c.globalArgsLength > 0 { + a = append(a, "...global...") + } + for i := c.globalArgsLength; i < len(c.args); i++ { + a = append(a, debugQuote(logArgSanitize(c.args[i]))) } return strings.Join(a, " ") } @@ -112,12 +118,6 @@ func (c *Command) SetParentContext(ctx context.Context) *Command { return c } -// SetDescription sets the description for this command which be returned on c.String() -func (c *Command) SetDescription(desc string) *Command { - c.desc = desc - return c -} - // isSafeArgumentValue checks if the argument is safe to be used as a value (not an option) func isSafeArgumentValue(s string) bool { return s == "" || s[0] != '-' @@ -271,8 +271,12 @@ var ErrBrokenCommand = errors.New("git command is broken") // Run runs the command with the RunOpts func (c *Command) Run(opts *RunOpts) error { + return c.run(1, opts) +} + +func (c *Command) run(skip int, opts *RunOpts) error { if len(c.brokenArgs) != 0 { - log.Error("git command is broken: %s, broken args: %s", c.String(), strings.Join(c.brokenArgs, " ")) + log.Error("git command is broken: %s, broken args: %s", c.LogString(), strings.Join(c.brokenArgs, " ")) return ErrBrokenCommand } if opts == nil { @@ -285,20 +289,14 @@ func (c *Command) Run(opts *RunOpts) error { timeout = defaultCommandExecutionTimeout } - if len(opts.Dir) == 0 { - log.Debug("git.Command.Run: %s", c) - } else { - log.Debug("git.Command.RunDir(%s): %s", opts.Dir, c) - } - - desc := c.desc - if desc == "" { - if opts.Dir == "" { - desc = fmt.Sprintf("git: %s", c.toString(true)) - } else { - desc = fmt.Sprintf("git(dir:%s): %s", opts.Dir, c.toString(true)) - } + var desc string + callerInfo := util.CallerFuncName(1 /* util */ + 1 /* this */ + skip /* parent */) + if pos := strings.LastIndex(callerInfo, "/"); pos >= 0 { + callerInfo = callerInfo[pos+1:] } + // these logs are for debugging purposes only, so no guarantee of correctness or stability + desc = fmt.Sprintf("git.Run(by:%s, repo:%s): %s", callerInfo, logArgSanitize(opts.Dir), c.LogString()) + log.Debug("git.Command: %s", desc) var ctx context.Context var cancel context.CancelFunc @@ -401,7 +399,7 @@ func IsErrorExitCode(err error, code int) bool { // RunStdString runs the command with options and returns stdout/stderr as string. and store stderr to returned error (err combined with stderr). func (c *Command) RunStdString(opts *RunOpts) (stdout, stderr string, runErr RunStdError) { - stdoutBytes, stderrBytes, err := c.RunStdBytes(opts) + stdoutBytes, stderrBytes, err := c.runStdBytes(opts) stdout = util.UnsafeBytesToString(stdoutBytes) stderr = util.UnsafeBytesToString(stderrBytes) if err != nil { @@ -413,6 +411,10 @@ func (c *Command) RunStdString(opts *RunOpts) (stdout, stderr string, runErr Run // RunStdBytes runs the command with options and returns stdout/stderr as bytes. and store stderr to returned error (err combined with stderr). func (c *Command) RunStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunStdError) { + return c.runStdBytes(opts) +} + +func (c *Command) runStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunStdError) { if opts == nil { opts = &RunOpts{} } @@ -435,7 +437,7 @@ func (c *Command) RunStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunS PipelineFunc: opts.PipelineFunc, } - err := c.Run(newOpts) + err := c.run(2, newOpts) stderr = stderrBuf.Bytes() if err != nil { return nil, stderr, &runStdError{err: err, stderr: util.UnsafeBytesToString(stderr)} diff --git a/modules/git/command_test.go b/modules/git/command_test.go index 9a6228c9ad..0823afd7f7 100644 --- a/modules/git/command_test.go +++ b/modules/git/command_test.go @@ -55,8 +55,8 @@ func TestGitArgument(t *testing.T) { func TestCommandString(t *testing.T) { cmd := NewCommandContextNoGlobals(context.Background(), "a", "-m msg", "it's a test", `say "hello"`) - assert.EqualValues(t, cmd.prog+` a "-m msg" "it's a test" "say \"hello\""`, cmd.String()) + assert.EqualValues(t, cmd.prog+` a "-m msg" "it's a test" "say \"hello\""`, cmd.LogString()) - cmd = NewCommandContextNoGlobals(context.Background(), "url: https://a:b@c/") - assert.EqualValues(t, cmd.prog+` "url: https://sanitized-credential@c/"`, cmd.toString(true)) + cmd = NewCommandContextNoGlobals(context.Background(), "url: https://a:b@c/", "/root/dir-a/dir-b") + assert.EqualValues(t, cmd.prog+` "url: https://sanitized-credential@c/" dir-a/dir-b`, cmd.LogString()) } diff --git a/modules/git/repo.go b/modules/git/repo.go index fc6e6e7acc..0993a4ac37 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -18,7 +18,6 @@ import ( "time" "code.gitea.io/gitea/modules/proxy" - "code.gitea.io/gitea/modules/util" ) // GPGSettings represents the default GPG settings for this repository @@ -160,12 +159,6 @@ func CloneWithArgs(ctx context.Context, args TrustedCmdArgs, from, to string, op } cmd.AddDashesAndList(from, to) - if strings.Contains(from, "://") && strings.Contains(from, "@") { - cmd.SetDescription(fmt.Sprintf("clone branch %s from %s to %s (shared: %t, mirror: %t, depth: %d)", opts.Branch, util.SanitizeCredentialURLs(from), to, opts.Shared, opts.Mirror, opts.Depth)) - } else { - cmd.SetDescription(fmt.Sprintf("clone branch %s from %s to %s (shared: %t, mirror: %t, depth: %d)", opts.Branch, from, to, opts.Shared, opts.Mirror, opts.Depth)) - } - if opts.Timeout <= 0 { opts.Timeout = -1 } @@ -213,12 +206,6 @@ func Push(ctx context.Context, repoPath string, opts PushOptions) error { } cmd.AddDashesAndList(remoteBranchArgs...) - if strings.Contains(opts.Remote, "://") && strings.Contains(opts.Remote, "@") { - cmd.SetDescription(fmt.Sprintf("push branch %s to %s (force: %t, mirror: %t)", opts.Branch, util.SanitizeCredentialURLs(opts.Remote), opts.Force, opts.Mirror)) - } else { - cmd.SetDescription(fmt.Sprintf("push branch %s to %s (force: %t, mirror: %t)", opts.Branch, opts.Remote, opts.Force, opts.Mirror)) - } - stdout, stderr, err := cmd.RunStdString(&RunOpts{Env: opts.Env, Timeout: opts.Timeout, Dir: repoPath}) if err != nil { if strings.Contains(stderr, "non-fast-forward") { diff --git a/modules/graceful/manager.go b/modules/graceful/manager.go index cac6ce62d6..433e8c4c27 100644 --- a/modules/graceful/manager.go +++ b/modules/graceful/manager.go @@ -9,6 +9,7 @@ import ( "sync" "time" + "code.gitea.io/gitea/modules/gtprof" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" @@ -136,7 +137,7 @@ func (g *Manager) doShutdown() { } g.lock.Lock() g.shutdownCtxCancel() - atShutdownCtx := pprof.WithLabels(g.hammerCtx, pprof.Labels(LifecyclePProfLabel, "post-shutdown")) + atShutdownCtx := pprof.WithLabels(g.hammerCtx, pprof.Labels(gtprof.LabelGracefulLifecycle, "post-shutdown")) pprof.SetGoroutineLabels(atShutdownCtx) for _, fn := range g.toRunAtShutdown { go fn() @@ -167,7 +168,7 @@ func (g *Manager) doHammerTime(d time.Duration) { default: log.Warn("Setting Hammer condition") g.hammerCtxCancel() - atHammerCtx := pprof.WithLabels(g.terminateCtx, pprof.Labels(LifecyclePProfLabel, "post-hammer")) + atHammerCtx := pprof.WithLabels(g.terminateCtx, pprof.Labels(gtprof.LabelGracefulLifecycle, "post-hammer")) pprof.SetGoroutineLabels(atHammerCtx) } g.lock.Unlock() @@ -183,7 +184,7 @@ func (g *Manager) doTerminate() { default: log.Warn("Terminating") g.terminateCtxCancel() - atTerminateCtx := pprof.WithLabels(g.managerCtx, pprof.Labels(LifecyclePProfLabel, "post-terminate")) + atTerminateCtx := pprof.WithLabels(g.managerCtx, pprof.Labels(gtprof.LabelGracefulLifecycle, "post-terminate")) pprof.SetGoroutineLabels(atTerminateCtx) for _, fn := range g.toRunAtTerminate { diff --git a/modules/graceful/manager_common.go b/modules/graceful/manager_common.go index 15dd4406d5..7cfbdfbeb0 100644 --- a/modules/graceful/manager_common.go +++ b/modules/graceful/manager_common.go @@ -8,6 +8,8 @@ import ( "runtime/pprof" "sync" "time" + + "code.gitea.io/gitea/modules/gtprof" ) // FIXME: it seems that there is a bug when using systemd Type=notify: the "Install Page" (INSTALL_LOCK=false) doesn't notify properly. @@ -22,12 +24,6 @@ const ( watchdogMsg systemdNotifyMsg = "WATCHDOG=1" ) -// LifecyclePProfLabel is a label marking manager lifecycle phase -// Making it compliant with prometheus key regex https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels -// would enable someone interested to be able to to continuously gather profiles into pyroscope. -// Other labels for pprof (in "modules/process" package) should also follow this rule. -const LifecyclePProfLabel = "graceful_lifecycle" - func statusMsg(msg string) systemdNotifyMsg { return systemdNotifyMsg("STATUS=" + msg) } @@ -71,10 +67,10 @@ func (g *Manager) prepare(ctx context.Context) { g.hammerCtx, g.hammerCtxCancel = context.WithCancel(ctx) g.managerCtx, g.managerCtxCancel = context.WithCancel(ctx) - g.terminateCtx = pprof.WithLabels(g.terminateCtx, pprof.Labels(LifecyclePProfLabel, "with-terminate")) - g.shutdownCtx = pprof.WithLabels(g.shutdownCtx, pprof.Labels(LifecyclePProfLabel, "with-shutdown")) - g.hammerCtx = pprof.WithLabels(g.hammerCtx, pprof.Labels(LifecyclePProfLabel, "with-hammer")) - g.managerCtx = pprof.WithLabels(g.managerCtx, pprof.Labels(LifecyclePProfLabel, "with-manager")) + g.terminateCtx = pprof.WithLabels(g.terminateCtx, pprof.Labels(gtprof.LabelGracefulLifecycle, "with-terminate")) + g.shutdownCtx = pprof.WithLabels(g.shutdownCtx, pprof.Labels(gtprof.LabelGracefulLifecycle, "with-shutdown")) + g.hammerCtx = pprof.WithLabels(g.hammerCtx, pprof.Labels(gtprof.LabelGracefulLifecycle, "with-hammer")) + g.managerCtx = pprof.WithLabels(g.managerCtx, pprof.Labels(gtprof.LabelGracefulLifecycle, "with-manager")) if !g.setStateTransition(stateInit, stateRunning) { panic("invalid graceful manager state: transition from init to running failed") diff --git a/modules/gtprof/gtprof.go b/modules/gtprof/gtprof.go new file mode 100644 index 0000000000..974b2c9757 --- /dev/null +++ b/modules/gtprof/gtprof.go @@ -0,0 +1,25 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package gtprof + +// This is a Gitea-specific profiling package, +// the name is chosen to distinguish it from the standard pprof tool and "GNU gprof" + +// LabelGracefulLifecycle is a label marking manager lifecycle phase +// Making it compliant with prometheus key regex https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels +// would enable someone interested to be able to continuously gather profiles into pyroscope. +// Other labels for pprof should also follow this rule. +const LabelGracefulLifecycle = "graceful_lifecycle" + +// LabelPid is a label set on goroutines that have a process attached +const LabelPid = "pid" + +// LabelPpid is a label set on goroutines that have a process attached +const LabelPpid = "ppid" + +// LabelProcessType is a label set on goroutines that have a process attached +const LabelProcessType = "process_type" + +// LabelProcessDescription is a label set on goroutines that have a process attached +const LabelProcessDescription = "process_description" diff --git a/modules/log/event_format.go b/modules/log/event_format.go index 0b8d1cec79..8fda0a4980 100644 --- a/modules/log/event_format.go +++ b/modules/log/event_format.go @@ -13,10 +13,9 @@ import ( type Event struct { Time time.Time - GoroutinePid string - Caller string - Filename string - Line int + Caller string + Filename string + Line int Level Level @@ -218,17 +217,16 @@ func EventFormatTextMessage(mode *WriterMode, event *Event, msgFormat string, ms } if flags&Lgopid == Lgopid { - if event.GoroutinePid != "" { - buf = append(buf, '[') - if mode.Colorize { - buf = append(buf, ColorBytes(FgHiYellow)...) - } - buf = append(buf, event.GoroutinePid...) - if mode.Colorize { - buf = append(buf, resetBytes...) - } - buf = append(buf, ']', ' ') + deprecatedGoroutinePid := "no-gopid" // use a dummy value to avoid breaking the log format + buf = append(buf, '[') + if mode.Colorize { + buf = append(buf, ColorBytes(FgHiYellow)...) } + buf = append(buf, deprecatedGoroutinePid...) + if mode.Colorize { + buf = append(buf, resetBytes...) + } + buf = append(buf, ']', ' ') } buf = append(buf, msg...) diff --git a/modules/log/event_format_test.go b/modules/log/event_format_test.go index 7c299a607d..6fd0f36d48 100644 --- a/modules/log/event_format_test.go +++ b/modules/log/event_format_test.go @@ -24,34 +24,32 @@ func TestItoa(t *testing.T) { func TestEventFormatTextMessage(t *testing.T) { res := EventFormatTextMessage(&WriterMode{Prefix: "[PREFIX] ", Colorize: false, Flags: Flags{defined: true, flags: 0xffffffff}}, &Event{ - Time: time.Date(2020, 1, 2, 3, 4, 5, 6, time.UTC), - Caller: "caller", - Filename: "filename", - Line: 123, - GoroutinePid: "pid", - Level: ERROR, - Stacktrace: "stacktrace", + Time: time.Date(2020, 1, 2, 3, 4, 5, 6, time.UTC), + Caller: "caller", + Filename: "filename", + Line: 123, + Level: ERROR, + Stacktrace: "stacktrace", }, "msg format: %v %v", "arg0", NewColoredValue("arg1", FgBlue), ) - assert.Equal(t, `[PREFIX] 2020/01/02 03:04:05.000000 filename:123:caller [E] [pid] msg format: arg0 arg1 + assert.Equal(t, `[PREFIX] 2020/01/02 03:04:05.000000 filename:123:caller [E] [no-gopid] msg format: arg0 arg1 stacktrace `, string(res)) res = EventFormatTextMessage(&WriterMode{Prefix: "[PREFIX] ", Colorize: true, Flags: Flags{defined: true, flags: 0xffffffff}}, &Event{ - Time: time.Date(2020, 1, 2, 3, 4, 5, 6, time.UTC), - Caller: "caller", - Filename: "filename", - Line: 123, - GoroutinePid: "pid", - Level: ERROR, - Stacktrace: "stacktrace", + Time: time.Date(2020, 1, 2, 3, 4, 5, 6, time.UTC), + Caller: "caller", + Filename: "filename", + Line: 123, + Level: ERROR, + Stacktrace: "stacktrace", }, "msg format: %v %v", "arg0", NewColoredValue("arg1", FgBlue), ) - assert.Equal(t, "[PREFIX] \x1b[36m2020/01/02 03:04:05.000000 \x1b[0m\x1b[32mfilename:123:\x1b[32mcaller\x1b[0m \x1b[1;31m[E]\x1b[0m [\x1b[93mpid\x1b[0m] msg format: arg0 \x1b[34marg1\x1b[0m\n\tstacktrace\n\n", string(res)) + assert.Equal(t, "[PREFIX] \x1b[36m2020/01/02 03:04:05.000000 \x1b[0m\x1b[32mfilename:123:\x1b[32mcaller\x1b[0m \x1b[1;31m[E]\x1b[0m [\x1b[93mno-gopid\x1b[0m] msg format: arg0 \x1b[34marg1\x1b[0m\n\tstacktrace\n\n", string(res)) } diff --git a/modules/log/flags.go b/modules/log/flags.go index f025159d53..8064c91745 100644 --- a/modules/log/flags.go +++ b/modules/log/flags.go @@ -30,7 +30,7 @@ const ( LUTC // if Ldate or Ltime is set, use UTC rather than the local time zone Llevelinitial // Initial character of the provided level in brackets, eg. [I] for info Llevel // Provided level in brackets [INFO] - Lgopid // the Goroutine-PID of the context + Lgopid // the Goroutine-PID of the context, deprecated and it is always a const value Lmedfile = Lshortfile | Llongfile // last 20 characters of the filename LstdFlags = Ldate | Ltime | Lmedfile | Lshortfuncname | Llevelinitial // default diff --git a/modules/log/groutinelabel.go b/modules/log/groutinelabel.go deleted file mode 100644 index 56d7af42da..0000000000 --- a/modules/log/groutinelabel.go +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package log - -import "unsafe" - -//go:linkname runtime_getProfLabel runtime/pprof.runtime_getProfLabel -func runtime_getProfLabel() unsafe.Pointer //nolint - -type labelMap map[string]string - -func getGoroutineLabels() map[string]string { - l := (*labelMap)(runtime_getProfLabel()) - if l == nil { - return nil - } - return *l -} diff --git a/modules/log/groutinelabel_test.go b/modules/log/groutinelabel_test.go deleted file mode 100644 index 34e99653d6..0000000000 --- a/modules/log/groutinelabel_test.go +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package log - -import ( - "context" - "runtime/pprof" - "testing" - - "github.com/stretchr/testify/assert" -) - -func Test_getGoroutineLabels(t *testing.T) { - pprof.Do(context.Background(), pprof.Labels(), func(ctx context.Context) { - currentLabels := getGoroutineLabels() - pprof.ForLabels(ctx, func(key, value string) bool { - assert.EqualValues(t, value, currentLabels[key]) - return true - }) - - pprof.Do(ctx, pprof.Labels("Test_getGoroutineLabels", "Test_getGoroutineLabels_child1"), func(ctx context.Context) { - currentLabels := getGoroutineLabels() - pprof.ForLabels(ctx, func(key, value string) bool { - assert.EqualValues(t, value, currentLabels[key]) - return true - }) - if assert.NotNil(t, currentLabels) { - assert.EqualValues(t, "Test_getGoroutineLabels_child1", currentLabels["Test_getGoroutineLabels"]) - } - }) - }) -} diff --git a/modules/log/logger_impl.go b/modules/log/logger_impl.go index d38c6516ed..76dd5f43fb 100644 --- a/modules/log/logger_impl.go +++ b/modules/log/logger_impl.go @@ -200,11 +200,6 @@ func (l *LoggerImpl) Log(skip int, level Level, format string, logArgs ...any) { event.Stacktrace = Stack(skip + 1) } - labels := getGoroutineLabels() - if labels != nil { - event.GoroutinePid = labels["pid"] - } - // get a simple text message without color msgArgs := make([]any, len(logArgs)) copy(msgArgs, logArgs) diff --git a/modules/process/manager.go b/modules/process/manager.go index 7b8ada786e..661511ce8d 100644 --- a/modules/process/manager.go +++ b/modules/process/manager.go @@ -11,6 +11,8 @@ import ( "sync" "sync/atomic" "time" + + "code.gitea.io/gitea/modules/gtprof" ) // TODO: This packages still uses a singleton for the Manager. @@ -25,18 +27,6 @@ var ( DefaultContext = context.Background() ) -// DescriptionPProfLabel is a label set on goroutines that have a process attached -const DescriptionPProfLabel = "process_description" - -// PIDPProfLabel is a label set on goroutines that have a process attached -const PIDPProfLabel = "pid" - -// PPIDPProfLabel is a label set on goroutines that have a process attached -const PPIDPProfLabel = "ppid" - -// ProcessTypePProfLabel is a label set on goroutines that have a process attached -const ProcessTypePProfLabel = "process_type" - // IDType is a pid type type IDType string @@ -187,7 +177,12 @@ func (pm *Manager) Add(ctx context.Context, description string, cancel context.C Trace(true, pid, description, parentPID, processType) - pprofCtx := pprof.WithLabels(ctx, pprof.Labels(DescriptionPProfLabel, description, PPIDPProfLabel, string(parentPID), PIDPProfLabel, string(pid), ProcessTypePProfLabel, processType)) + pprofCtx := pprof.WithLabels(ctx, pprof.Labels( + gtprof.LabelProcessDescription, description, + gtprof.LabelPpid, string(parentPID), + gtprof.LabelPid, string(pid), + gtprof.LabelProcessType, processType, + )) if currentlyRunning { pprof.SetGoroutineLabels(pprofCtx) } diff --git a/modules/process/manager_stacktraces.go b/modules/process/manager_stacktraces.go index e260893113..d83060f6ee 100644 --- a/modules/process/manager_stacktraces.go +++ b/modules/process/manager_stacktraces.go @@ -10,6 +10,8 @@ import ( "sort" "time" + "code.gitea.io/gitea/modules/gtprof" + "github.com/google/pprof/profile" ) @@ -202,7 +204,7 @@ func (pm *Manager) ProcessStacktraces(flat, noSystem bool) ([]*Process, int, int // Add the non-process associated labels from the goroutine sample to the Stack for name, value := range sample.Label { - if name == DescriptionPProfLabel || name == PIDPProfLabel || (!flat && name == PPIDPProfLabel) || name == ProcessTypePProfLabel { + if name == gtprof.LabelProcessDescription || name == gtprof.LabelPid || (!flat && name == gtprof.LabelPpid) || name == gtprof.LabelProcessType { continue } @@ -224,7 +226,7 @@ func (pm *Manager) ProcessStacktraces(flat, noSystem bool) ([]*Process, int, int var process *Process // Try to get the PID from the goroutine labels - if pidvalue, ok := sample.Label[PIDPProfLabel]; ok && len(pidvalue) == 1 { + if pidvalue, ok := sample.Label[gtprof.LabelPid]; ok && len(pidvalue) == 1 { pid := IDType(pidvalue[0]) // Now try to get the process from our map @@ -238,20 +240,20 @@ func (pm *Manager) ProcessStacktraces(flat, noSystem bool) ([]*Process, int, int // get the parent PID ppid := IDType("") - if value, ok := sample.Label[PPIDPProfLabel]; ok && len(value) == 1 { + if value, ok := sample.Label[gtprof.LabelPpid]; ok && len(value) == 1 { ppid = IDType(value[0]) } // format the description description := "(dead process)" - if value, ok := sample.Label[DescriptionPProfLabel]; ok && len(value) == 1 { + if value, ok := sample.Label[gtprof.LabelProcessDescription]; ok && len(value) == 1 { description = value[0] + " " + description } // override the type of the process to "code" but add the old type as a label on the first stack ptype := NoneProcessType - if value, ok := sample.Label[ProcessTypePProfLabel]; ok && len(value) == 1 { - stack.Labels = append(stack.Labels, &Label{Name: ProcessTypePProfLabel, Value: value[0]}) + if value, ok := sample.Label[gtprof.LabelProcessType]; ok && len(value) == 1 { + stack.Labels = append(stack.Labels, &Label{Name: gtprof.LabelProcessType, Value: value[0]}) } process = &Process{ PID: pid, diff --git a/modules/queue/workerqueue.go b/modules/queue/workerqueue.go index 672e9a4114..0f5b105551 100644 --- a/modules/queue/workerqueue.go +++ b/modules/queue/workerqueue.go @@ -6,6 +6,7 @@ package queue import ( "context" "fmt" + "runtime/pprof" "sync" "sync/atomic" "time" @@ -241,6 +242,9 @@ func NewWorkerPoolQueueWithContext[T any](ctx context.Context, name string, queu w.origHandler = handler w.safeHandler = func(t ...T) (unhandled []T) { defer func() { + // FIXME: there is no ctx support in the handler, so process manager is unable to restore the labels + // so here we explicitly set the "queue ctx" labels again after the handler is done + pprof.SetGoroutineLabels(w.ctxRun) err := recover() if err != nil { log.Error("Recovered from panic in queue %q handler: %v\n%s", name, err, log.Stack(2)) diff --git a/modules/util/runtime.go b/modules/util/runtime.go new file mode 100644 index 0000000000..91ec3c869c --- /dev/null +++ b/modules/util/runtime.go @@ -0,0 +1,13 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package util + +import "runtime" + +func CallerFuncName(skip int) string { + pc := make([]uintptr, 1) + runtime.Callers(skip+1, pc) + funcName := runtime.FuncForPC(pc[0]).Name() + return funcName +} diff --git a/modules/util/runtime_test.go b/modules/util/runtime_test.go new file mode 100644 index 0000000000..20f9063b0b --- /dev/null +++ b/modules/util/runtime_test.go @@ -0,0 +1,32 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package util + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCallerFuncName(t *testing.T) { + s := CallerFuncName(1) + assert.Equal(t, "code.gitea.io/gitea/modules/util.TestCallerFuncName", s) +} + +func BenchmarkCallerFuncName(b *testing.B) { + // BenchmarkCaller/sprintf-12 12744829 95.49 ns/op + b.Run("sprintf", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = fmt.Sprintf("aaaaaaaaaaaaaaaa %s %s %s", "bbbbbbbbbbbbbbbbbbb", b.Name(), "ccccccccccccccccccccc") + } + }) + // BenchmarkCaller/caller-12 10625133 113.6 ns/op + // It is almost as fast as fmt.Sprintf + b.Run("caller", func(b *testing.B) { + for i := 0; i < b.N; i++ { + CallerFuncName(1) + } + }) +} diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index 58a2bdbab1..a8a7a4bd79 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -458,7 +458,6 @@ func serviceRPC(ctx *context.Context, h *serviceHandler, service string) { var stderr bytes.Buffer cmd.AddArguments("--stateless-rpc").AddDynamicArguments(h.getRepoDir()) - cmd.SetDescription(fmt.Sprintf("%s %s %s [repo_path: %s]", git.GitExecutable, service, "--stateless-rpc", h.getRepoDir())) if err := cmd.Run(&git.RunOpts{ Dir: h.getRepoDir(), Env: append(os.Environ(), h.environ...), diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index bb1722039e..0b5a855d42 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1156,7 +1156,6 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi go func() { stderr := &bytes.Buffer{} - cmdDiff.SetDescription(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath)) if err := cmdDiff.Run(&git.RunOpts{ Timeout: time.Duration(setting.Git.Timeout.Default) * time.Second, Dir: repoPath, diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index a81fe6e9bd..22d380e8e6 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -46,11 +46,6 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error } cmd := git.NewCommand(ctx, "remote", "add").AddDynamicArguments(remoteName).AddArguments("--mirror=fetch").AddDynamicArguments(addr) - if strings.Contains(addr, "://") && strings.Contains(addr, "@") { - cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, util.SanitizeCredentialURLs(addr), repoPath)) - } else { - cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, addr, repoPath)) - } _, _, err = cmd.RunStdString(&git.RunOpts{Dir: repoPath}) if err != nil && !strings.HasPrefix(err.Error(), "exit status 128 - fatal: No such remote ") { return err @@ -66,11 +61,6 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error } cmd = git.NewCommand(ctx, "remote", "add").AddDynamicArguments(remoteName).AddArguments("--mirror=fetch").AddDynamicArguments(wikiRemotePath) - if strings.Contains(wikiRemotePath, "://") && strings.Contains(wikiRemotePath, "@") { - cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, util.SanitizeCredentialURLs(wikiRemotePath), wikiPath)) - } else { - cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, wikiRemotePath, wikiPath)) - } _, _, err = cmd.RunStdString(&git.RunOpts{Dir: wikiPath}) if err != nil && !strings.HasPrefix(err.Error(), "exit status 128 - fatal: No such remote ") { return err @@ -197,7 +187,6 @@ func pruneBrokenReferences(ctx context.Context, stderrBuilder.Reset() stdoutBuilder.Reset() pruneErr := git.NewCommand(ctx, "remote", "prune").AddDynamicArguments(m.GetRemoteName()). - SetDescription(fmt.Sprintf("Mirror.runSync %ssPrune references: %s ", wiki, m.Repo.FullName())). Run(&git.RunOpts{ Timeout: timeout, Dir: repoPath, @@ -248,15 +237,13 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo stdoutBuilder := strings.Builder{} stderrBuilder := strings.Builder{} - if err := cmd. - SetDescription(fmt.Sprintf("Mirror.runSync: %s", m.Repo.FullName())). - Run(&git.RunOpts{ - Timeout: timeout, - Dir: repoPath, - Env: envs, - Stdout: &stdoutBuilder, - Stderr: &stderrBuilder, - }); err != nil { + if err := cmd.Run(&git.RunOpts{ + Timeout: timeout, + Dir: repoPath, + Env: envs, + Stdout: &stdoutBuilder, + Stderr: &stderrBuilder, + }); err != nil { stdout := stdoutBuilder.String() stderr := stderrBuilder.String() @@ -275,14 +262,12 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo // Successful prune - reattempt mirror stderrBuilder.Reset() stdoutBuilder.Reset() - if err = cmd. - SetDescription(fmt.Sprintf("Mirror.runSync: %s", m.Repo.FullName())). - Run(&git.RunOpts{ - Timeout: timeout, - Dir: repoPath, - Stdout: &stdoutBuilder, - Stderr: &stderrBuilder, - }); err != nil { + if err = cmd.Run(&git.RunOpts{ + Timeout: timeout, + Dir: repoPath, + Stdout: &stdoutBuilder, + Stderr: &stderrBuilder, + }); err != nil { stdout := stdoutBuilder.String() stderr := stderrBuilder.String() @@ -346,7 +331,6 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo stderrBuilder.Reset() stdoutBuilder.Reset() if err := git.NewCommand(ctx, "remote", "update", "--prune").AddDynamicArguments(m.GetRemoteName()). - SetDescription(fmt.Sprintf("Mirror.runSync Wiki: %s ", m.Repo.FullName())). Run(&git.RunOpts{ Timeout: timeout, Dir: wikiPath, @@ -373,7 +357,6 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo stdoutBuilder.Reset() if err = git.NewCommand(ctx, "remote", "update", "--prune").AddDynamicArguments(m.GetRemoteName()). - SetDescription(fmt.Sprintf("Mirror.runSync Wiki: %s ", m.Repo.FullName())). Run(&git.RunOpts{ Timeout: timeout, Dir: wikiPath, diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go index 21ba0afeff..02ff97b1f0 100644 --- a/services/mirror/mirror_push.go +++ b/services/mirror/mirror_push.go @@ -9,7 +9,6 @@ import ( "fmt" "io" "regexp" - "strings" "time" "code.gitea.io/gitea/models/db" @@ -31,11 +30,6 @@ var stripExitStatus = regexp.MustCompile(`exit status \d+ - `) func AddPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr string) error { addRemoteAndConfig := func(addr, path string) error { cmd := git.NewCommand(ctx, "remote", "add", "--mirror=push").AddDynamicArguments(m.RemoteName, addr) - if strings.Contains(addr, "://") && strings.Contains(addr, "@") { - cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=push %s [repo_path: %s]", m.RemoteName, util.SanitizeCredentialURLs(addr), path)) - } else { - cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=push %s [repo_path: %s]", m.RemoteName, addr, path)) - } if _, _, err := cmd.RunStdString(&git.RunOpts{Dir: path}); err != nil { return err } diff --git a/services/release/release.go b/services/release/release.go index 980a5e98e7..123e9ab7c7 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -326,7 +326,6 @@ func DeleteReleaseByID(ctx context.Context, repo *repo_model.Repository, rel *re } if stdout, _, err := git.NewCommand(ctx, "tag", "-d").AddDashesAndList(rel.TagName). - SetDescription(fmt.Sprintf("DeleteReleaseByID (git tag -d): %d", rel.ID)). RunStdString(&git.RunOpts{Dir: repo.RepoPath()}); err != nil && !strings.Contains(err.Error(), "not found") { log.Error("DeleteReleaseByID (git tag -d): %d in %v Failed:\nStdout: %s\nError: %v", rel.ID, repo, stdout, err) return fmt.Errorf("git tag -d: %w", err) diff --git a/services/repository/adopt.go b/services/repository/adopt.go index 615f4d482c..e37909e7ab 100644 --- a/services/repository/adopt.go +++ b/services/repository/adopt.go @@ -92,7 +92,6 @@ func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts CreateR } if stdout, _, err := git.NewCommand(ctx, "update-server-info"). - SetDescription(fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath)). RunStdString(&git.RunOpts{Dir: repoPath}); err != nil { log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err) return fmt.Errorf("CreateRepository(git update-server-info): %w", err) diff --git a/services/repository/check.go b/services/repository/check.go index 5cdcc14679..acca15daf2 100644 --- a/services/repository/check.go +++ b/services/repository/check.go @@ -86,8 +86,7 @@ func GitGcRepos(ctx context.Context, timeout time.Duration, args git.TrustedCmdA // GitGcRepo calls 'git gc' to remove unnecessary files and optimize the local repository func GitGcRepo(ctx context.Context, repo *repo_model.Repository, timeout time.Duration, args git.TrustedCmdArgs) error { log.Trace("Running git gc on %-v", repo) - command := git.NewCommand(ctx, "gc").AddArguments(args...). - SetDescription(fmt.Sprintf("Repository Garbage Collection: %s", repo.FullName())) + command := git.NewCommand(ctx, "gc").AddArguments(args...) var stdout string var err error stdout, _, err = command.RunStdString(&git.RunOpts{Timeout: timeout, Dir: repo.RepoPath()}) diff --git a/services/repository/create.go b/services/repository/create.go index 14e625d962..a3199f2a40 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -68,7 +68,6 @@ func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir, // Clone to temporary path and do the init commit. if stdout, _, err := git.NewCommand(ctx, "clone").AddDynamicArguments(repoPath, tmpDir). - SetDescription(fmt.Sprintf("prepareRepoCommit (git clone): %s to %s", repoPath, tmpDir)). RunStdString(&git.RunOpts{Dir: "", Env: env}); err != nil { log.Error("Failed to clone from %v into %s: stdout: %s\nError: %v", repo, tmpDir, stdout, err) return fmt.Errorf("git clone: %w", err) @@ -301,7 +300,6 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt } if stdout, _, err := git.NewCommand(ctx, "update-server-info"). - SetDescription(fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath)). RunStdString(&git.RunOpts{Dir: repoPath}); err != nil { log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err) rollbackRepo = repo @@ -314,9 +312,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt if len(opts.License) > 0 { licenses = append(licenses, ConvertLicenseName(opts.License)) - stdout, _, err := git.NewCommand(ctx, "rev-parse", "HEAD"). - SetDescription(fmt.Sprintf("CreateRepository(git rev-parse HEAD): %s", repoPath)). - RunStdString(&git.RunOpts{Dir: repoPath}) + stdout, _, err := git.NewCommand(ctx, "rev-parse", "HEAD").RunStdString(&git.RunOpts{Dir: repoPath}) if err != nil { log.Error("CreateRepository(git rev-parse HEAD) in %v: Stdout: %s\nError: %v", repo, stdout, err) rollbackRepo = repo diff --git a/services/repository/fork.go b/services/repository/fork.go index bc4fdf8562..cff0b1a403 100644 --- a/services/repository/fork.go +++ b/services/repository/fork.go @@ -158,7 +158,6 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork } repoPath := repo_model.RepoPath(owner.Name, repo.Name) if stdout, _, err := cloneCmd.AddDynamicArguments(oldRepoPath, repoPath). - SetDescription(fmt.Sprintf("ForkRepository(git clone): %s to %s", opts.BaseRepo.FullName(), repo.FullName())). RunStdBytes(&git.RunOpts{Timeout: 10 * time.Minute}); err != nil { log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, opts.BaseRepo, stdout, err) return fmt.Errorf("git clone: %w", err) @@ -169,7 +168,6 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork } if stdout, _, err := git.NewCommand(txCtx, "update-server-info"). - SetDescription(fmt.Sprintf("ForkRepository(git update-server-info): %s", repo.FullName())). RunStdString(&git.RunOpts{Dir: repoPath}); err != nil { log.Error("Fork Repository (git update-server-info) failed for %v:\nStdout: %s\nError: %v", repo, stdout, err) return fmt.Errorf("git update-server-info: %w", err) diff --git a/services/repository/generate.go b/services/repository/generate.go index f2280de8b2..24cf9d1b9b 100644 --- a/services/repository/generate.go +++ b/services/repository/generate.go @@ -237,7 +237,6 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r repoPath := repo.RepoPath() if stdout, _, err := git.NewCommand(ctx, "remote", "add", "origin").AddDynamicArguments(repoPath). - SetDescription(fmt.Sprintf("generateRepoCommit (git remote add): %s to %s", templateRepoPath, tmpDir)). RunStdString(&git.RunOpts{Dir: tmpDir, Env: env}); err != nil { log.Error("Unable to add %v as remote origin to temporary repo to %s: stdout %s\nError: %v", repo, tmpDir, stdout, err) return fmt.Errorf("git remote add: %w", err) @@ -369,7 +368,6 @@ func generateRepository(ctx context.Context, doer, owner *user_model.User, templ } if stdout, _, err := git.NewCommand(ctx, "update-server-info"). - SetDescription(fmt.Sprintf("GenerateRepository(git update-server-info): %s", repoPath)). RunStdString(&git.RunOpts{Dir: repoPath}); err != nil { log.Error("GenerateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", generateRepo, stdout, err) return generateRepo, fmt.Errorf("error in GenerateRepository(git update-server-info): %w", err) diff --git a/services/repository/init.go b/services/repository/init.go index 817fa4abd7..c719e11786 100644 --- a/services/repository/init.go +++ b/services/repository/init.go @@ -34,7 +34,6 @@ func initRepoCommit(ctx context.Context, tmpPath string, repo *repo_model.Reposi committerEmail := sig.Email if stdout, _, err := git.NewCommand(ctx, "add", "--all"). - SetDescription(fmt.Sprintf("initRepoCommit (git add): %s", tmpPath)). RunStdString(&git.RunOpts{Dir: tmpPath}); err != nil { log.Error("git add --all failed: Stdout: %s\nError: %v", stdout, err) return fmt.Errorf("git add --all: %w", err) @@ -62,9 +61,8 @@ func initRepoCommit(ctx context.Context, tmpPath string, repo *repo_model.Reposi ) if stdout, _, err := cmd. - SetDescription(fmt.Sprintf("initRepoCommit (git commit): %s", tmpPath)). RunStdString(&git.RunOpts{Dir: tmpPath, Env: env}); err != nil { - log.Error("Failed to commit: %v: Stdout: %s\nError: %v", cmd.String(), stdout, err) + log.Error("Failed to commit: %v: Stdout: %s\nError: %v", cmd.LogString(), stdout, err) return fmt.Errorf("git commit: %w", err) } @@ -73,7 +71,6 @@ func initRepoCommit(ctx context.Context, tmpPath string, repo *repo_model.Reposi } if stdout, _, err := git.NewCommand(ctx, "push", "origin").AddDynamicArguments("HEAD:" + defaultBranch). - SetDescription(fmt.Sprintf("initRepoCommit (git push): %s", tmpPath)). RunStdString(&git.RunOpts{Dir: tmpPath, Env: repo_module.InternalPushingEnvironment(u, repo)}); err != nil { log.Error("Failed to push back to HEAD: Stdout: %s\nError: %v", stdout, err) return fmt.Errorf("git push: %w", err) diff --git a/services/repository/migrate.go b/services/repository/migrate.go index c627b46fab..7fe63eb5ca 100644 --- a/services/repository/migrate.go +++ b/services/repository/migrate.go @@ -122,7 +122,6 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, } if stdout, _, err := git.NewCommand(ctx, "update-server-info"). - SetDescription(fmt.Sprintf("MigrateRepositoryGitData(git update-server-info): %s", repoPath)). RunStdString(&git.RunOpts{Dir: repoPath}); err != nil { log.Error("MigrateRepositoryGitData(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err) return repo, fmt.Errorf("error in MigrateRepositoryGitData(git update-server-info): %w", err) From 7580bd98c1c9c108500b0c295d5fb6aa057170d4 Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 20 Dec 2024 16:39:19 +0100 Subject: [PATCH 02/18] show warning on navigation if currently editing comment or title (#32920) This PR fixes the issue https://github.com/go-gitea/gitea/issues/32223 Make the browser to show the confirm popup, as it does with other forms. --------- Co-authored-by: Tim Wundenberg Co-authored-by: wxiaoguang --- templates/repo/issue/view_content.tmpl | 8 ++++---- templates/repo/issue/view_title.tmpl | 10 +++++----- web_src/js/features/repo-issue-edit.ts | 6 +++++- web_src/js/features/repo-issue.ts | 6 ++++-- web_src/js/vendor/jquery.are-you-sure.ts | 13 +++++++++---- 5 files changed, 27 insertions(+), 16 deletions(-) diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index 69b5a11a14..1a68781ecd 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -139,7 +139,7 @@ {{template "repo/issue/view_content/reference_issue_dialog" .}} diff --git a/templates/repo/issue/view_title.tmpl b/templates/repo/issue/view_title.tmpl index bf2ede58e4..0354f6ef22 100644 --- a/templates/repo/issue/view_title.tmpl +++ b/templates/repo/issue/view_title.tmpl @@ -26,17 +26,17 @@ {{if $canEditIssueTitle}} -
+
- +
- - +
-
+ {{end}}
{{if .HasMerged}} diff --git a/web_src/js/features/repo-issue-edit.ts b/web_src/js/features/repo-issue-edit.ts index 9d146951bd..cf4c223e03 100644 --- a/web_src/js/features/repo-issue-edit.ts +++ b/web_src/js/features/repo-issue-edit.ts @@ -7,6 +7,7 @@ import {attachRefIssueContextPopup} from './contextpopup.ts'; import {initCommentContent, initMarkupContent} from '../markup/content.ts'; import {triggerUploadStateChanged} from './comp/EditorUpload.ts'; import {convertHtmlToMarkdown} from '../markup/html2markdown.ts'; +import {applyAreYouSure, reinitializeAreYouSure} from '../vendor/jquery.are-you-sure.ts'; async function tryOnEditContent(e) { const clickTarget = e.target.closest('.edit-content'); @@ -48,6 +49,7 @@ async function tryOnEditContent(e) { showErrorToast(data.errorMessage); return; } + reinitializeAreYouSure(editContentZone.querySelector('form')); // the form is no longer dirty editContentZone.setAttribute('data-content-version', data.contentVersion); if (!data.content) { renderContent.innerHTML = document.querySelector('#no-content').innerHTML; @@ -86,13 +88,15 @@ async function tryOnEditContent(e) { comboMarkdownEditor = getComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor')); if (!comboMarkdownEditor) { editContentZone.innerHTML = document.querySelector('#issue-comment-editor-template').innerHTML; + const form = editContentZone.querySelector('form'); + applyAreYouSure(form); const saveButton = querySingleVisibleElem(editContentZone, '.ui.primary.button'); const cancelButton = querySingleVisibleElem(editContentZone, '.ui.cancel.button'); comboMarkdownEditor = await initComboMarkdownEditor(editContentZone.querySelector('.combo-markdown-editor')); const syncUiState = () => saveButton.disabled = comboMarkdownEditor.isUploading(); comboMarkdownEditor.container.addEventListener(ComboMarkdownEditor.EventUploadStateChanged, syncUiState); cancelButton.addEventListener('click', cancelAndReset); - saveButton.addEventListener('click', saveAndRefresh); + form.addEventListener('submit', saveAndRefresh); } // FIXME: ideally here should reload content and attachment list from backend for existing editor, to avoid losing data diff --git a/web_src/js/features/repo-issue.ts b/web_src/js/features/repo-issue.ts index 7541039786..1e1bbd64f0 100644 --- a/web_src/js/features/repo-issue.ts +++ b/web_src/js/features/repo-issue.ts @@ -532,7 +532,7 @@ export function initRepoIssueWipToggle() { export function initRepoIssueTitleEdit() { const issueTitleDisplay = document.querySelector('#issue-title-display'); - const issueTitleEditor = document.querySelector('#issue-title-editor'); + const issueTitleEditor = document.querySelector('#issue-title-editor'); if (!issueTitleEditor) return; const issueTitleInput = issueTitleEditor.querySelector('input'); @@ -558,7 +558,8 @@ export function initRepoIssueTitleEdit() { const prTargetUpdateUrl = pullDescEditor?.getAttribute('data-target-update-url'); const editSaveButton = issueTitleEditor.querySelector('.ui.primary.button'); - editSaveButton.addEventListener('click', async () => { + issueTitleEditor.addEventListener('submit', async (e) => { + e.preventDefault(); const newTitle = issueTitleInput.value.trim(); try { if (newTitle && newTitle !== oldTitle) { @@ -577,6 +578,7 @@ export function initRepoIssueTitleEdit() { } } } + issueTitleEditor.classList.remove('dirty'); window.location.reload(); } catch (error) { console.error(error); diff --git a/web_src/js/vendor/jquery.are-you-sure.ts b/web_src/js/vendor/jquery.are-you-sure.ts index 9efe783c54..7f0bef8040 100644 --- a/web_src/js/vendor/jquery.are-you-sure.ts +++ b/web_src/js/vendor/jquery.are-you-sure.ts @@ -2,6 +2,7 @@ // Fork of the upstream module. The only changes are: // * use export to make it work with ES6 modules. // * the addition of `const` to make it strict mode compatible. +// * ignore forms with "ignore-dirty" class, ignore hidden forms (closest('.tw-hidden')) /*! * jQuery Plugin: Are-You-Sure (Dirty Form Detection) @@ -161,10 +162,10 @@ export function initAreYouSure($) { if (!settings.silent && !window.aysUnloadSet) { window.aysUnloadSet = true; $(window).bind('beforeunload', function() { - const $dirtyForms = $("form").filter('.' + settings.dirtyClass); - if ($dirtyForms.length == 0) { - return; - } + const $forms = $("form:not(.ignore-dirty)").filter('.' + settings.dirtyClass); + const dirtyFormCount = Array.from($forms).reduce((res, form) => form.closest('.tw-hidden') ? res : res + 1, 0); + if (dirtyFormCount === 0) return; + // Prevent multiple prompts - seen on Chrome and IE if (navigator.userAgent.toLowerCase().match(/msie|chrome/)) { if (window.aysHasPrompted) { @@ -199,3 +200,7 @@ export function initAreYouSure($) { export function applyAreYouSure(selectorOrEl: string|Element|$, opts = {}) { $(selectorOrEl).areYouSure(opts); } + +export function reinitializeAreYouSure(selectorOrEl: string|Element|$) { + $(selectorOrEl).trigger('reinitialize.areYouSure'); +} From 1c9b022c4d9174c3a96fb323593241b19fc245aa Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 21 Dec 2024 00:11:38 +0800 Subject: [PATCH 03/18] Refactor db package and remove unnecessary `DumpTables` (#32930) --- models/auth/source_test.go | 7 +- models/db/collation.go | 2 +- models/db/context.go | 8 +- models/db/convert.go | 16 +-- models/db/engine.go | 220 +++---------------------------------- models/db/engine_dump.go | 33 ++++++ models/db/engine_hook.go | 34 ++++++ models/db/engine_init.go | 140 +++++++++++++++++++++++ models/db/name.go | 2 +- models/db/sequence.go | 6 +- 10 files changed, 247 insertions(+), 221 deletions(-) create mode 100644 models/db/engine_dump.go create mode 100644 models/db/engine_hook.go create mode 100644 models/db/engine_init.go diff --git a/models/auth/source_test.go b/models/auth/source_test.go index 36e76d5e28..84aede0a6b 100644 --- a/models/auth/source_test.go +++ b/models/auth/source_test.go @@ -13,6 +13,8 @@ import ( "code.gitea.io/gitea/modules/json" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "xorm.io/xorm" "xorm.io/xorm/schemas" ) @@ -54,7 +56,8 @@ func TestDumpAuthSource(t *testing.T) { sb := new(strings.Builder) - db.DumpTables([]*schemas.Table{authSourceSchema}, sb) - + // TODO: this test is quite hacky, it should use a low-level "select" (without model processors) but not a database dump + engine := db.GetEngine(db.DefaultContext).(*xorm.Engine) + require.NoError(t, engine.DumpTables([]*schemas.Table{authSourceSchema}, sb)) assert.Contains(t, sb.String(), `"Provider":"ConvertibleSourceName"`) } diff --git a/models/db/collation.go b/models/db/collation.go index a7db9f5442..79ade87380 100644 --- a/models/db/collation.go +++ b/models/db/collation.go @@ -140,7 +140,7 @@ func CheckCollations(x *xorm.Engine) (*CheckCollationsResult, error) { } func CheckCollationsDefaultEngine() (*CheckCollationsResult, error) { - return CheckCollations(x) + return CheckCollations(xormEngine) } func alterDatabaseCollation(x *xorm.Engine, collation string) error { diff --git a/models/db/context.go b/models/db/context.go index 171e26b933..51627712b1 100644 --- a/models/db/context.go +++ b/models/db/context.go @@ -94,7 +94,7 @@ func GetEngine(ctx context.Context) Engine { if e := getExistingEngine(ctx); e != nil { return e } - return x.Context(ctx) + return xormEngine.Context(ctx) } // getExistingEngine gets an existing db Engine/Statement from this context or returns nil @@ -155,7 +155,7 @@ func TxContext(parentCtx context.Context) (*Context, Committer, error) { return newContext(parentCtx, sess), &halfCommitter{committer: sess}, nil } - sess := x.NewSession() + sess := xormEngine.NewSession() if err := sess.Begin(); err != nil { _ = sess.Close() return nil, nil, err @@ -179,7 +179,7 @@ func WithTx(parentCtx context.Context, f func(ctx context.Context) error) error } func txWithNoCheck(parentCtx context.Context, f func(ctx context.Context) error) error { - sess := x.NewSession() + sess := xormEngine.NewSession() defer sess.Close() if err := sess.Begin(); err != nil { return err @@ -322,7 +322,7 @@ func CountByBean(ctx context.Context, bean any) (int64, error) { // TableName returns the table name according a bean object func TableName(bean any) string { - return x.TableName(bean) + return xormEngine.TableName(bean) } // InTransaction returns true if the engine is in a transaction otherwise return false diff --git a/models/db/convert.go b/models/db/convert.go index 8c124471ab..80b0f7b04b 100644 --- a/models/db/convert.go +++ b/models/db/convert.go @@ -16,30 +16,30 @@ import ( // ConvertDatabaseTable converts database and tables from utf8 to utf8mb4 if it's mysql and set ROW_FORMAT=dynamic func ConvertDatabaseTable() error { - if x.Dialect().URI().DBType != schemas.MYSQL { + if xormEngine.Dialect().URI().DBType != schemas.MYSQL { return nil } - r, err := CheckCollations(x) + r, err := CheckCollations(xormEngine) if err != nil { return err } - _, err = x.Exec(fmt.Sprintf("ALTER DATABASE `%s` CHARACTER SET utf8mb4 COLLATE %s", setting.Database.Name, r.ExpectedCollation)) + _, err = xormEngine.Exec(fmt.Sprintf("ALTER DATABASE `%s` CHARACTER SET utf8mb4 COLLATE %s", setting.Database.Name, r.ExpectedCollation)) if err != nil { return err } - tables, err := x.DBMetas() + tables, err := xormEngine.DBMetas() if err != nil { return err } for _, table := range tables { - if _, err := x.Exec(fmt.Sprintf("ALTER TABLE `%s` ROW_FORMAT=dynamic", table.Name)); err != nil { + if _, err := xormEngine.Exec(fmt.Sprintf("ALTER TABLE `%s` ROW_FORMAT=dynamic", table.Name)); err != nil { return err } - if _, err := x.Exec(fmt.Sprintf("ALTER TABLE `%s` CONVERT TO CHARACTER SET utf8mb4 COLLATE %s", table.Name, r.ExpectedCollation)); err != nil { + if _, err := xormEngine.Exec(fmt.Sprintf("ALTER TABLE `%s` CONVERT TO CHARACTER SET utf8mb4 COLLATE %s", table.Name, r.ExpectedCollation)); err != nil { return err } } @@ -49,11 +49,11 @@ func ConvertDatabaseTable() error { // ConvertVarcharToNVarchar converts database and tables from varchar to nvarchar if it's mssql func ConvertVarcharToNVarchar() error { - if x.Dialect().URI().DBType != schemas.MSSQL { + if xormEngine.Dialect().URI().DBType != schemas.MSSQL { return nil } - sess := x.NewSession() + sess := xormEngine.NewSession() defer sess.Close() res, err := sess.QuerySliceString(`SELECT 'ALTER TABLE ' + OBJECT_NAME(SC.object_id) + ' MODIFY SC.name NVARCHAR(' + CONVERT(VARCHAR(5),SC.max_length) + ')' FROM SYS.columns SC diff --git a/models/db/engine.go b/models/db/engine.go index b17188945a..91015f7038 100755 --- a/models/db/engine.go +++ b/models/db/engine.go @@ -8,17 +8,10 @@ import ( "context" "database/sql" "fmt" - "io" "reflect" "strings" - "time" - - "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" "xorm.io/xorm" - "xorm.io/xorm/contexts" - "xorm.io/xorm/names" "xorm.io/xorm/schemas" _ "github.com/go-sql-driver/mysql" // Needed for the MySQL driver @@ -27,9 +20,9 @@ import ( ) var ( - x *xorm.Engine - tables []any - initFuncs []func() error + xormEngine *xorm.Engine + registeredModels []any + registeredInitFuncs []func() error ) // Engine represents a xorm engine or session. @@ -70,167 +63,38 @@ type Engine interface { // TableInfo returns table's information via an object func TableInfo(v any) (*schemas.Table, error) { - return x.TableInfo(v) + return xormEngine.TableInfo(v) } -// DumpTables dump tables information -func DumpTables(tables []*schemas.Table, w io.Writer, tp ...schemas.DBType) error { - return x.DumpTables(tables, w, tp...) -} - -// RegisterModel registers model, if initfunc provided, it will be invoked after data model sync +// RegisterModel registers model, if initFuncs provided, it will be invoked after data model sync func RegisterModel(bean any, initFunc ...func() error) { - tables = append(tables, bean) - if len(initFuncs) > 0 && initFunc[0] != nil { - initFuncs = append(initFuncs, initFunc[0]) + registeredModels = append(registeredModels, bean) + if len(registeredInitFuncs) > 0 && initFunc[0] != nil { + registeredInitFuncs = append(registeredInitFuncs, initFunc[0]) } } -func init() { - gonicNames := []string{"SSL", "UID"} - for _, name := range gonicNames { - names.LintGonicMapper[name] = true - } -} - -// newXORMEngine returns a new XORM engine from the configuration -func newXORMEngine() (*xorm.Engine, error) { - connStr, err := setting.DBConnStr() - if err != nil { - return nil, err - } - - var engine *xorm.Engine - - if setting.Database.Type.IsPostgreSQL() && len(setting.Database.Schema) > 0 { - // OK whilst we sort out our schema issues - create a schema aware postgres - registerPostgresSchemaDriver() - engine, err = xorm.NewEngine("postgresschema", connStr) - } else { - engine, err = xorm.NewEngine(setting.Database.Type.String(), connStr) - } - - if err != nil { - return nil, err - } - if setting.Database.Type == "mysql" { - engine.Dialect().SetParams(map[string]string{"rowFormat": "DYNAMIC"}) - } else if setting.Database.Type == "mssql" { - engine.Dialect().SetParams(map[string]string{"DEFAULT_VARCHAR": "nvarchar"}) - } - engine.SetSchema(setting.Database.Schema) - return engine, nil -} - // SyncAllTables sync the schemas of all tables, is required by unit test code func SyncAllTables() error { - _, err := x.StoreEngine("InnoDB").SyncWithOptions(xorm.SyncOptions{ + _, err := xormEngine.StoreEngine("InnoDB").SyncWithOptions(xorm.SyncOptions{ WarnIfDatabaseColumnMissed: true, - }, tables...) + }, registeredModels...) return err } -// InitEngine initializes the xorm.Engine and sets it as db.DefaultContext -func InitEngine(ctx context.Context) error { - xormEngine, err := newXORMEngine() - if err != nil { - if strings.Contains(err.Error(), "SQLite3 support") { - return fmt.Errorf(`sqlite3 requires: -tags sqlite,sqlite_unlock_notify%s%w`, "\n", err) - } - return fmt.Errorf("failed to connect to database: %w", err) - } - - xormEngine.SetMapper(names.GonicMapper{}) - // WARNING: for serv command, MUST remove the output to os.stdout, - // so use log file to instead print to stdout. - xormEngine.SetLogger(NewXORMLogger(setting.Database.LogSQL)) - xormEngine.ShowSQL(setting.Database.LogSQL) - xormEngine.SetMaxOpenConns(setting.Database.MaxOpenConns) - xormEngine.SetMaxIdleConns(setting.Database.MaxIdleConns) - xormEngine.SetConnMaxLifetime(setting.Database.ConnMaxLifetime) - xormEngine.SetDefaultContext(ctx) - - if setting.Database.SlowQueryThreshold > 0 { - xormEngine.AddHook(&SlowQueryHook{ - Threshold: setting.Database.SlowQueryThreshold, - Logger: log.GetLogger("xorm"), - }) - } - - SetDefaultEngine(ctx, xormEngine) - return nil -} - -// SetDefaultEngine sets the default engine for db -func SetDefaultEngine(ctx context.Context, eng *xorm.Engine) { - x = eng - DefaultContext = &Context{Context: ctx, engine: x} -} - -// UnsetDefaultEngine closes and unsets the default engine -// We hope the SetDefaultEngine and UnsetDefaultEngine can be paired, but it's impossible now, -// there are many calls to InitEngine -> SetDefaultEngine directly to overwrite the `x` and DefaultContext without close -// Global database engine related functions are all racy and there is no graceful close right now. -func UnsetDefaultEngine() { - if x != nil { - _ = x.Close() - x = nil - } - DefaultContext = nil -} - -// InitEngineWithMigration initializes a new xorm.Engine and sets it as the db.DefaultContext -// This function must never call .Sync() if the provided migration function fails. -// When called from the "doctor" command, the migration function is a version check -// that prevents the doctor from fixing anything in the database if the migration level -// is different from the expected value. -func InitEngineWithMigration(ctx context.Context, migrateFunc func(*xorm.Engine) error) (err error) { - if err = InitEngine(ctx); err != nil { - return err - } - - if err = x.Ping(); err != nil { - return err - } - - preprocessDatabaseCollation(x) - - // We have to run migrateFunc here in case the user is re-running installation on a previously created DB. - // If we do not then table schemas will be changed and there will be conflicts when the migrations run properly. - // - // Installation should only be being re-run if users want to recover an old database. - // However, we should think carefully about should we support re-install on an installed instance, - // as there may be other problems due to secret reinitialization. - if err = migrateFunc(x); err != nil { - return fmt.Errorf("migrate: %w", err) - } - - if err = SyncAllTables(); err != nil { - return fmt.Errorf("sync database struct error: %w", err) - } - - for _, initFunc := range initFuncs { - if err := initFunc(); err != nil { - return fmt.Errorf("initFunc failed: %w", err) - } - } - - return nil -} - // NamesToBean return a list of beans or an error func NamesToBean(names ...string) ([]any, error) { beans := []any{} if len(names) == 0 { - beans = append(beans, tables...) + beans = append(beans, registeredModels...) return beans, nil } // Need to map provided names to beans... beanMap := make(map[string]any) - for _, bean := range tables { + for _, bean := range registeredModels { beanMap[strings.ToLower(reflect.Indirect(reflect.ValueOf(bean)).Type().Name())] = bean - beanMap[strings.ToLower(x.TableName(bean))] = bean - beanMap[strings.ToLower(x.TableName(bean, true))] = bean + beanMap[strings.ToLower(xormEngine.TableName(bean))] = bean + beanMap[strings.ToLower(xormEngine.TableName(bean, true))] = bean } gotBean := make(map[any]bool) @@ -247,36 +111,9 @@ func NamesToBean(names ...string) ([]any, error) { return beans, nil } -// DumpDatabase dumps all data from database according the special database SQL syntax to file system. -func DumpDatabase(filePath, dbType string) error { - var tbs []*schemas.Table - for _, t := range tables { - t, err := x.TableInfo(t) - if err != nil { - return err - } - tbs = append(tbs, t) - } - - type Version struct { - ID int64 `xorm:"pk autoincr"` - Version int64 - } - t, err := x.TableInfo(&Version{}) - if err != nil { - return err - } - tbs = append(tbs, t) - - if len(dbType) > 0 { - return x.DumpTablesToFile(tbs, filePath, schemas.DBType(dbType)) - } - return x.DumpTablesToFile(tbs, filePath) -} - // MaxBatchInsertSize returns the table's max batch insert size func MaxBatchInsertSize(bean any) int { - t, err := x.TableInfo(bean) + t, err := xormEngine.TableInfo(bean) if err != nil { return 50 } @@ -285,18 +122,18 @@ func MaxBatchInsertSize(bean any) int { // IsTableNotEmpty returns true if table has at least one record func IsTableNotEmpty(beanOrTableName any) (bool, error) { - return x.Table(beanOrTableName).Exist() + return xormEngine.Table(beanOrTableName).Exist() } // DeleteAllRecords will delete all the records of this table func DeleteAllRecords(tableName string) error { - _, err := x.Exec(fmt.Sprintf("DELETE FROM %s", tableName)) + _, err := xormEngine.Exec(fmt.Sprintf("DELETE FROM %s", tableName)) return err } // GetMaxID will return max id of the table func GetMaxID(beanOrTableName any) (maxID int64, err error) { - _, err = x.Select("MAX(id)").Table(beanOrTableName).Get(&maxID) + _, err = xormEngine.Select("MAX(id)").Table(beanOrTableName).Get(&maxID) return maxID, err } @@ -308,24 +145,3 @@ func SetLogSQL(ctx context.Context, on bool) { sess.Engine().ShowSQL(on) } } - -type SlowQueryHook struct { - Threshold time.Duration - Logger log.Logger -} - -var _ contexts.Hook = &SlowQueryHook{} - -func (SlowQueryHook) BeforeProcess(c *contexts.ContextHook) (context.Context, error) { - return c.Ctx, nil -} - -func (h *SlowQueryHook) AfterProcess(c *contexts.ContextHook) error { - if c.ExecuteTime >= h.Threshold { - // 8 is the amount of skips passed to runtime.Caller, so that in the log the correct function - // is being displayed (the function that ultimately wants to execute the query in the code) - // instead of the function of the slow query hook being called. - h.Logger.Log(8, log.WARN, "[Slow SQL Query] %s %v - %v", c.SQL, c.Args, c.ExecuteTime) - } - return nil -} diff --git a/models/db/engine_dump.go b/models/db/engine_dump.go new file mode 100644 index 0000000000..63f2d4e093 --- /dev/null +++ b/models/db/engine_dump.go @@ -0,0 +1,33 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package db + +import "xorm.io/xorm/schemas" + +// DumpDatabase dumps all data from database according the special database SQL syntax to file system. +func DumpDatabase(filePath, dbType string) error { + var tbs []*schemas.Table + for _, t := range registeredModels { + t, err := xormEngine.TableInfo(t) + if err != nil { + return err + } + tbs = append(tbs, t) + } + + type Version struct { + ID int64 `xorm:"pk autoincr"` + Version int64 + } + t, err := xormEngine.TableInfo(&Version{}) + if err != nil { + return err + } + tbs = append(tbs, t) + + if dbType != "" { + return xormEngine.DumpTablesToFile(tbs, filePath, schemas.DBType(dbType)) + } + return xormEngine.DumpTablesToFile(tbs, filePath) +} diff --git a/models/db/engine_hook.go b/models/db/engine_hook.go new file mode 100644 index 0000000000..b4c543c3dd --- /dev/null +++ b/models/db/engine_hook.go @@ -0,0 +1,34 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package db + +import ( + "context" + "time" + + "code.gitea.io/gitea/modules/log" + + "xorm.io/xorm/contexts" +) + +type SlowQueryHook struct { + Threshold time.Duration + Logger log.Logger +} + +var _ contexts.Hook = (*SlowQueryHook)(nil) + +func (*SlowQueryHook) BeforeProcess(c *contexts.ContextHook) (context.Context, error) { + return c.Ctx, nil +} + +func (h *SlowQueryHook) AfterProcess(c *contexts.ContextHook) error { + if c.ExecuteTime >= h.Threshold { + // 8 is the amount of skips passed to runtime.Caller, so that in the log the correct function + // is being displayed (the function that ultimately wants to execute the query in the code) + // instead of the function of the slow query hook being called. + h.Logger.Log(8, log.WARN, "[Slow SQL Query] %s %v - %v", c.SQL, c.Args, c.ExecuteTime) + } + return nil +} diff --git a/models/db/engine_init.go b/models/db/engine_init.go new file mode 100644 index 0000000000..da85018957 --- /dev/null +++ b/models/db/engine_init.go @@ -0,0 +1,140 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package db + +import ( + "context" + "fmt" + "strings" + + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" + + "xorm.io/xorm" + "xorm.io/xorm/names" +) + +func init() { + gonicNames := []string{"SSL", "UID"} + for _, name := range gonicNames { + names.LintGonicMapper[name] = true + } +} + +// newXORMEngine returns a new XORM engine from the configuration +func newXORMEngine() (*xorm.Engine, error) { + connStr, err := setting.DBConnStr() + if err != nil { + return nil, err + } + + var engine *xorm.Engine + + if setting.Database.Type.IsPostgreSQL() && len(setting.Database.Schema) > 0 { + // OK whilst we sort out our schema issues - create a schema aware postgres + registerPostgresSchemaDriver() + engine, err = xorm.NewEngine("postgresschema", connStr) + } else { + engine, err = xorm.NewEngine(setting.Database.Type.String(), connStr) + } + + if err != nil { + return nil, err + } + if setting.Database.Type == "mysql" { + engine.Dialect().SetParams(map[string]string{"rowFormat": "DYNAMIC"}) + } else if setting.Database.Type == "mssql" { + engine.Dialect().SetParams(map[string]string{"DEFAULT_VARCHAR": "nvarchar"}) + } + engine.SetSchema(setting.Database.Schema) + return engine, nil +} + +// InitEngine initializes the xorm.Engine and sets it as db.DefaultContext +func InitEngine(ctx context.Context) error { + xe, err := newXORMEngine() + if err != nil { + if strings.Contains(err.Error(), "SQLite3 support") { + return fmt.Errorf(`sqlite3 requires: -tags sqlite,sqlite_unlock_notify%s%w`, "\n", err) + } + return fmt.Errorf("failed to connect to database: %w", err) + } + + xe.SetMapper(names.GonicMapper{}) + // WARNING: for serv command, MUST remove the output to os.stdout, + // so use log file to instead print to stdout. + xe.SetLogger(NewXORMLogger(setting.Database.LogSQL)) + xe.ShowSQL(setting.Database.LogSQL) + xe.SetMaxOpenConns(setting.Database.MaxOpenConns) + xe.SetMaxIdleConns(setting.Database.MaxIdleConns) + xe.SetConnMaxLifetime(setting.Database.ConnMaxLifetime) + xe.SetDefaultContext(ctx) + + if setting.Database.SlowQueryThreshold > 0 { + xe.AddHook(&SlowQueryHook{ + Threshold: setting.Database.SlowQueryThreshold, + Logger: log.GetLogger("xorm"), + }) + } + + SetDefaultEngine(ctx, xe) + return nil +} + +// SetDefaultEngine sets the default engine for db +func SetDefaultEngine(ctx context.Context, eng *xorm.Engine) { + xormEngine = eng + DefaultContext = &Context{Context: ctx, engine: xormEngine} +} + +// UnsetDefaultEngine closes and unsets the default engine +// We hope the SetDefaultEngine and UnsetDefaultEngine can be paired, but it's impossible now, +// there are many calls to InitEngine -> SetDefaultEngine directly to overwrite the `xormEngine` and DefaultContext without close +// Global database engine related functions are all racy and there is no graceful close right now. +func UnsetDefaultEngine() { + if xormEngine != nil { + _ = xormEngine.Close() + xormEngine = nil + } + DefaultContext = nil +} + +// InitEngineWithMigration initializes a new xorm.Engine and sets it as the db.DefaultContext +// This function must never call .Sync() if the provided migration function fails. +// When called from the "doctor" command, the migration function is a version check +// that prevents the doctor from fixing anything in the database if the migration level +// is different from the expected value. +func InitEngineWithMigration(ctx context.Context, migrateFunc func(*xorm.Engine) error) (err error) { + if err = InitEngine(ctx); err != nil { + return err + } + + if err = xormEngine.Ping(); err != nil { + return err + } + + preprocessDatabaseCollation(xormEngine) + + // We have to run migrateFunc here in case the user is re-running installation on a previously created DB. + // If we do not then table schemas will be changed and there will be conflicts when the migrations run properly. + // + // Installation should only be being re-run if users want to recover an old database. + // However, we should think carefully about should we support re-install on an installed instance, + // as there may be other problems due to secret reinitialization. + if err = migrateFunc(xormEngine); err != nil { + return fmt.Errorf("migrate: %w", err) + } + + if err = SyncAllTables(); err != nil { + return fmt.Errorf("sync database struct error: %w", err) + } + + for _, initFunc := range registeredInitFuncs { + if err := initFunc(); err != nil { + return fmt.Errorf("initFunc failed: %w", err) + } + } + + return nil +} diff --git a/models/db/name.go b/models/db/name.go index 51be33a8bc..55c9dffb6a 100644 --- a/models/db/name.go +++ b/models/db/name.go @@ -16,7 +16,7 @@ var ( // ErrNameEmpty name is empty error ErrNameEmpty = util.SilentWrap{Message: "name is empty", Err: util.ErrInvalidArgument} - // AlphaDashDotPattern characters prohibited in a user name (anything except A-Za-z0-9_.-) + // AlphaDashDotPattern characters prohibited in a username (anything except A-Za-z0-9_.-) AlphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`) ) diff --git a/models/db/sequence.go b/models/db/sequence.go index f49ad935de..9adc5113ac 100644 --- a/models/db/sequence.go +++ b/models/db/sequence.go @@ -17,11 +17,11 @@ func CountBadSequences(_ context.Context) (int64, error) { return 0, nil } - sess := x.NewSession() + sess := xormEngine.NewSession() defer sess.Close() var sequences []string - schema := x.Dialect().URI().Schema + schema := xormEngine.Dialect().URI().Schema sess.Engine().SetSchema("") if err := sess.Table("information_schema.sequences").Cols("sequence_name").Where("sequence_name LIKE 'tmp_recreate__%_id_seq%' AND sequence_catalog = ?", setting.Database.Name).Find(&sequences); err != nil { @@ -38,7 +38,7 @@ func FixBadSequences(_ context.Context) error { return nil } - sess := x.NewSession() + sess := xormEngine.NewSession() defer sess.Close() if err := sess.Begin(); err != nil { return err From 4774151e5339431dcd7fde70f084e7a0ff0b6cf6 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 21 Dec 2024 00:38:56 +0800 Subject: [PATCH 04/18] Improve navbar: add "admin" tip, add "active" style (#32927) By the way, remove all "tw-" patches and unused styles. --- templates/base/head_navbar.tmpl | 27 +++++++------ web_src/css/modules/navbar.css | 70 +++++++++++++++------------------ 2 files changed, 46 insertions(+), 51 deletions(-) diff --git a/templates/base/head_navbar.tmpl b/templates/base/head_navbar.tmpl index 951ee590d1..ed7a8d6f24 100644 --- a/templates/base/head_navbar.tmpl +++ b/templates/base/head_navbar.tmpl @@ -11,9 +11,9 @@ -