From db262359b704948de8b1db446fa0d5261a7e2c69 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 7 Jan 2020 22:11:25 +0000 Subject: [PATCH 1/5] Delay printing hook statuses until after 1 second --- cmd/hook.go | 117 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 94 insertions(+), 23 deletions(-) diff --git a/cmd/hook.go b/cmd/hook.go index aabd9637c2a0..aa472373cdd5 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -12,6 +12,7 @@ import ( "os" "strconv" "strings" + "time" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/git" @@ -101,6 +102,37 @@ Gitea or set your environment appropriately.`, "") total := 0 lastline := 0 + commsChan := make(chan string, 10) + tickerChan := make(chan struct{}) + go func() { + sb := strings.Builder{} + hasWritten := false + ticker := time.NewTicker(1 * time.Second) + loop: + for { + select { + case s, ok := <-commsChan: + if ok { + sb.WriteString(s) + } else if hasWritten { + hasWritten = true + os.Stdout.WriteString(sb.String()) + os.Stdout.Sync() + sb.Reset() + break loop + } + case <-ticker.C: + hasWritten = true + os.Stdout.WriteString(sb.String()) + os.Stdout.Sync() + sb.Reset() + } + } + sb.Reset() + ticker.Stop() + close(tickerChan) + }() + for scanner.Scan() { // TODO: support news feeds for wiki if isWiki { @@ -124,12 +156,10 @@ Gitea or set your environment appropriately.`, "") newCommitIDs[count] = newCommitID refFullNames[count] = refFullName count++ - fmt.Fprintf(os.Stdout, "*") - os.Stdout.Sync() + commsChan <- "*" if count >= hookBatchSize { fmt.Fprintf(os.Stdout, " Checking %d branches\n", count) - os.Stdout.Sync() hookOptions.OldCommitIDs = oldCommitIDs hookOptions.NewCommitIDs = newCommitIDs @@ -139,20 +169,22 @@ Gitea or set your environment appropriately.`, "") case http.StatusOK: // no-op case http.StatusInternalServerError: + close(commsChan) + <-tickerChan fail("Internal Server Error", msg) default: + close(commsChan) + <-tickerChan fail(msg, "") } count = 0 lastline = 0 } } else { - fmt.Fprintf(os.Stdout, ".") - os.Stdout.Sync() + commsChan <- "." } if lastline >= hookBatchSize { - fmt.Fprintf(os.Stdout, "\n") - os.Stdout.Sync() + commsChan <- "\n" lastline = 0 } } @@ -162,25 +194,28 @@ Gitea or set your environment appropriately.`, "") hookOptions.NewCommitIDs = newCommitIDs[:count] hookOptions.RefFullNames = refFullNames[:count] - fmt.Fprintf(os.Stdout, " Checking %d branches\n", count) + commsChan <- fmt.Sprintf(" Checking %d branches\n", count) os.Stdout.Sync() statusCode, msg := private.HookPreReceive(username, reponame, hookOptions) switch statusCode { case http.StatusInternalServerError: + close(commsChan) + <-tickerChan fail("Internal Server Error", msg) case http.StatusForbidden: + close(commsChan) + <-tickerChan fail(msg, "") } } else if lastline > 0 { - fmt.Fprintf(os.Stdout, "\n") - os.Stdout.Sync() + commsChan <- "\n" lastline = 0 } - fmt.Fprintf(os.Stdout, "Checked %d references in total\n", total) - os.Stdout.Sync() - + commsChan <- fmt.Sprintf("Checked %d references in total\n", total) + close(commsChan) + <-tickerChan return nil } @@ -206,6 +241,37 @@ Gitea or set your environment appropriately.`, "") } } + commsChan := make(chan string, 10) + tickerChan := make(chan struct{}) + go func() { + sb := strings.Builder{} + hasWritten := false + ticker := time.NewTicker(1 * time.Second) + loop: + for { + select { + case s, ok := <-commsChan: + if ok { + sb.WriteString(s) + } else if hasWritten { + hasWritten = true + os.Stdout.WriteString(sb.String()) + os.Stdout.Sync() + sb.Reset() + break loop + } + case <-ticker.C: + hasWritten = true + os.Stdout.WriteString(sb.String()) + os.Stdout.Sync() + sb.Reset() + } + } + sb.Reset() + ticker.Stop() + close(tickerChan) + }() + // the environment setted on serv command repoUser := os.Getenv(models.EnvRepoUsername) isWiki := (os.Getenv(models.EnvRepoIsWiki) == "true") @@ -241,7 +307,7 @@ Gitea or set your environment appropriately.`, "") continue } - fmt.Fprintf(os.Stdout, ".") + commsChan <- "." oldCommitIDs[count] = string(fields[0]) newCommitIDs[count] = string(fields[1]) refFullNames[count] = string(fields[2]) @@ -250,16 +316,16 @@ Gitea or set your environment appropriately.`, "") } count++ total++ - os.Stdout.Sync() if count >= hookBatchSize { - fmt.Fprintf(os.Stdout, " Processing %d references\n", count) - os.Stdout.Sync() + commsChan <- fmt.Sprintf(" Processing %d references\n", count) hookOptions.OldCommitIDs = oldCommitIDs hookOptions.NewCommitIDs = newCommitIDs hookOptions.RefFullNames = refFullNames resp, err := private.HookPostReceive(repoUser, repoName, hookOptions) if resp == nil { + close(commsChan) + <-tickerChan hookPrintResults(results) fail("Internal Server Error", err) } @@ -274,12 +340,15 @@ Gitea or set your environment appropriately.`, "") // We need to tell the repo to reset the default branch to master err := private.SetDefaultBranch(repoUser, repoName, "master") if err != nil { + close(commsChan) + <-tickerChan fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err) } } - fmt.Fprintf(os.Stdout, "Processed %d references in total\n", total) - os.Stdout.Sync() + commsChan <- fmt.Sprintf("Processed %d references in total\n", total) + close(commsChan) + <-tickerChan hookPrintResults(results) return nil } @@ -288,19 +357,19 @@ Gitea or set your environment appropriately.`, "") hookOptions.NewCommitIDs = newCommitIDs[:count] hookOptions.RefFullNames = refFullNames[:count] - fmt.Fprintf(os.Stdout, " Processing %d references\n", count) - os.Stdout.Sync() + commsChan <- fmt.Sprintf(" Processing %d references\n", count) resp, err := private.HookPostReceive(repoUser, repoName, hookOptions) if resp == nil { + close(commsChan) + <-tickerChan hookPrintResults(results) fail("Internal Server Error", err) } wasEmpty = wasEmpty || resp.RepoWasEmpty results = append(results, resp.Results...) - fmt.Fprintf(os.Stdout, "Processed %d references in total\n", total) - os.Stdout.Sync() + commsChan <- fmt.Sprintf("Processed %d references in total\n", total) if wasEmpty && masterPushed { // We need to tell the repo to reset the default branch to master @@ -310,6 +379,8 @@ Gitea or set your environment appropriately.`, "") } } + close(commsChan) + <-tickerChan hookPrintResults(results) return nil From 5ebf17316bc1394f2524ca2c9e33d16089678144 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 11 Jan 2020 11:15:01 +0000 Subject: [PATCH 2/5] Move to a 5s delay, wrapped writer structure and add config --- cmd/hook.go | 205 ++++++++++-------- .../doc/advanced/config-cheat-sheet.en-us.md | 2 + modules/setting/git.go | 4 + 3 files changed, 119 insertions(+), 92 deletions(-) diff --git a/cmd/hook.go b/cmd/hook.go index aa472373cdd5..9fd7d0a0df82 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -8,6 +8,7 @@ import ( "bufio" "bytes" "fmt" + "io" "net/http" "os" "strconv" @@ -59,6 +60,81 @@ var ( } ) +type delayWriter struct { + internal io.Writer + buf *bytes.Buffer + timer *time.Timer +} + +func newDelayWriter(internal io.Writer, delay time.Duration) *delayWriter { + timer := time.NewTimer(delay) + return &delayWriter{ + internal: internal, + buf: &bytes.Buffer{}, + timer: timer, + } +} + +func (d *delayWriter) Write(p []byte) (n int, err error) { + if d.buf != nil { + select { + case <-d.timer.C: + _, err := d.internal.Write(d.buf.Bytes()) + if err != nil { + return 0, err + } + d.buf = nil + return d.internal.Write(p) + default: + return d.buf.Write(p) + } + } + return d.internal.Write(p) +} + +func (d *delayWriter) WriteString(s string) (n int, err error) { + if d.buf != nil { + select { + case <-d.timer.C: + _, err := d.internal.Write(d.buf.Bytes()) + if err != nil { + return 0, err + } + d.buf = nil + return d.internal.Write([]byte(s)) + default: + return d.buf.WriteString(s) + } + } + return d.internal.Write([]byte(s)) +} + +func (d *delayWriter) Close() error { + if d == nil { + return nil + } + stopped := d.timer.Stop() + if stopped { + return nil + } + <-d.timer.C + if d.buf == nil { + return nil + } + _, err := d.internal.Write(d.buf.Bytes()) + return err +} + +type nilWriter struct{} + +func (n *nilWriter) Write(p []byte) (int, error) { + return len(p), nil +} + +func (n *nilWriter) WriteString(s string) (int, error) { + return len(s), nil +} + func runHookPreReceive(c *cli.Context) error { if os.Getenv(models.EnvIsInternal) == "true" { return nil @@ -102,36 +178,17 @@ Gitea or set your environment appropriately.`, "") total := 0 lastline := 0 - commsChan := make(chan string, 10) - tickerChan := make(chan struct{}) - go func() { - sb := strings.Builder{} - hasWritten := false - ticker := time.NewTicker(1 * time.Second) - loop: - for { - select { - case s, ok := <-commsChan: - if ok { - sb.WriteString(s) - } else if hasWritten { - hasWritten = true - os.Stdout.WriteString(sb.String()) - os.Stdout.Sync() - sb.Reset() - break loop - } - case <-ticker.C: - hasWritten = true - os.Stdout.WriteString(sb.String()) - os.Stdout.Sync() - sb.Reset() - } + var out io.Writer + out = &nilWriter{} + if setting.Git.VerbosePush { + if setting.Git.VerbosePushDelay > 0 { + dWriter := newDelayWriter(os.Stdout, setting.Git.VerbosePushDelay) + defer dWriter.Close() + out = dWriter + } else { + out = os.Stdout } - sb.Reset() - ticker.Stop() - close(tickerChan) - }() + } for scanner.Scan() { // TODO: support news feeds for wiki @@ -156,10 +213,10 @@ Gitea or set your environment appropriately.`, "") newCommitIDs[count] = newCommitID refFullNames[count] = refFullName count++ - commsChan <- "*" + fmt.Fprintf(out, "*") if count >= hookBatchSize { - fmt.Fprintf(os.Stdout, " Checking %d branches\n", count) + fmt.Fprintf(out, " Checking %d branches\n", count) hookOptions.OldCommitIDs = oldCommitIDs hookOptions.NewCommitIDs = newCommitIDs @@ -169,22 +226,18 @@ Gitea or set your environment appropriately.`, "") case http.StatusOK: // no-op case http.StatusInternalServerError: - close(commsChan) - <-tickerChan fail("Internal Server Error", msg) default: - close(commsChan) - <-tickerChan fail(msg, "") } count = 0 lastline = 0 } } else { - commsChan <- "." + fmt.Fprintf(out, ".") } if lastline >= hookBatchSize { - commsChan <- "\n" + fmt.Fprintf(out, "\n") lastline = 0 } } @@ -194,28 +247,21 @@ Gitea or set your environment appropriately.`, "") hookOptions.NewCommitIDs = newCommitIDs[:count] hookOptions.RefFullNames = refFullNames[:count] - commsChan <- fmt.Sprintf(" Checking %d branches\n", count) - os.Stdout.Sync() + fmt.Fprintf(out, " Checking %d branches\n", count) statusCode, msg := private.HookPreReceive(username, reponame, hookOptions) switch statusCode { case http.StatusInternalServerError: - close(commsChan) - <-tickerChan fail("Internal Server Error", msg) case http.StatusForbidden: - close(commsChan) - <-tickerChan fail(msg, "") } } else if lastline > 0 { - commsChan <- "\n" + fmt.Fprintf(out, "\n") lastline = 0 } - commsChan <- fmt.Sprintf("Checked %d references in total\n", total) - close(commsChan) - <-tickerChan + fmt.Fprintf(out, "Checked %d references in total\n", total) return nil } @@ -241,36 +287,18 @@ Gitea or set your environment appropriately.`, "") } } - commsChan := make(chan string, 10) - tickerChan := make(chan struct{}) - go func() { - sb := strings.Builder{} - hasWritten := false - ticker := time.NewTicker(1 * time.Second) - loop: - for { - select { - case s, ok := <-commsChan: - if ok { - sb.WriteString(s) - } else if hasWritten { - hasWritten = true - os.Stdout.WriteString(sb.String()) - os.Stdout.Sync() - sb.Reset() - break loop - } - case <-ticker.C: - hasWritten = true - os.Stdout.WriteString(sb.String()) - os.Stdout.Sync() - sb.Reset() - } + var out io.Writer + var dWriter *delayWriter + out = &nilWriter{} + if setting.Git.VerbosePush { + if setting.Git.VerbosePushDelay > 0 { + dWriter = newDelayWriter(os.Stdout, setting.Git.VerbosePushDelay) + defer dWriter.Close() + out = dWriter + } else { + out = os.Stdout } - sb.Reset() - ticker.Stop() - close(tickerChan) - }() + } // the environment setted on serv command repoUser := os.Getenv(models.EnvRepoUsername) @@ -307,7 +335,7 @@ Gitea or set your environment appropriately.`, "") continue } - commsChan <- "." + fmt.Fprintf(out, ".") oldCommitIDs[count] = string(fields[0]) newCommitIDs[count] = string(fields[1]) refFullNames[count] = string(fields[2]) @@ -318,14 +346,13 @@ Gitea or set your environment appropriately.`, "") total++ if count >= hookBatchSize { - commsChan <- fmt.Sprintf(" Processing %d references\n", count) + fmt.Fprintf(out, " Processing %d references\n", count) hookOptions.OldCommitIDs = oldCommitIDs hookOptions.NewCommitIDs = newCommitIDs hookOptions.RefFullNames = refFullNames resp, err := private.HookPostReceive(repoUser, repoName, hookOptions) if resp == nil { - close(commsChan) - <-tickerChan + dWriter.Close() hookPrintResults(results) fail("Internal Server Error", err) } @@ -340,15 +367,12 @@ Gitea or set your environment appropriately.`, "") // We need to tell the repo to reset the default branch to master err := private.SetDefaultBranch(repoUser, repoName, "master") if err != nil { - close(commsChan) - <-tickerChan fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err) } } - commsChan <- fmt.Sprintf("Processed %d references in total\n", total) + fmt.Fprintf(out, "Processed %d references in total\n", total) - close(commsChan) - <-tickerChan + dWriter.Close() hookPrintResults(results) return nil } @@ -357,19 +381,18 @@ Gitea or set your environment appropriately.`, "") hookOptions.NewCommitIDs = newCommitIDs[:count] hookOptions.RefFullNames = refFullNames[:count] - commsChan <- fmt.Sprintf(" Processing %d references\n", count) + fmt.Fprintf(out, " Processing %d references\n", count) resp, err := private.HookPostReceive(repoUser, repoName, hookOptions) if resp == nil { - close(commsChan) - <-tickerChan + dWriter.Close() hookPrintResults(results) fail("Internal Server Error", err) } wasEmpty = wasEmpty || resp.RepoWasEmpty results = append(results, resp.Results...) - commsChan <- fmt.Sprintf("Processed %d references in total\n", total) + fmt.Fprintf(out, "Processed %d references in total\n", total) if wasEmpty && masterPushed { // We need to tell the repo to reset the default branch to master @@ -378,9 +401,7 @@ Gitea or set your environment appropriately.`, "") fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err) } } - - close(commsChan) - <-tickerChan + dWriter.Close() hookPrintResults(results) return nil diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index dc6a1ba34697..ea17096ea574 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -522,6 +522,8 @@ NB: You must `REDIRECT_MACARON_LOG` and have `DISABLE_ROUTER_LOG` set to `false` - `MAX_GIT_DIFF_FILES`: **100**: Max number of files shown in diff view. - `GC_ARGS`: **\**: Arguments for command `git gc`, e.g. `--aggressive --auto`. See more on http://git-scm.com/docs/git-gc/ - `ENABLE_AUTO_GIT_WIRE_PROTOCOL`: **true**: If use git wire protocol version 2 when git version >= 2.18, default is true, set to false when you always want git wire protocol version 1 +- `VERBOSE_PUSH`: **true**: Print status information about pushes as they are being processed. +- `VERBOSE_PUSH_DELAY`: **5s**: Only print verbose information if push takes longer than this delay. ## Git - Timeout settings (`git.timeout`) - `DEFAUlT`: **360**: Git operations default timeout seconds. diff --git a/modules/setting/git.go b/modules/setting/git.go index 8495be8836f1..8c8179cba6b8 100644 --- a/modules/setting/git.go +++ b/modules/setting/git.go @@ -21,6 +21,8 @@ var ( MaxGitDiffLines int MaxGitDiffLineCharacters int MaxGitDiffFiles int + VerbosePush bool + VerbosePushDelay time.Duration GCArgs []string `ini:"GC_ARGS" delim:" "` EnableAutoGitWireProtocol bool Timeout struct { @@ -36,6 +38,8 @@ var ( MaxGitDiffLines: 1000, MaxGitDiffLineCharacters: 5000, MaxGitDiffFiles: 100, + VerbosePush: true, + VerbosePushDelay: 5 * time.Second, GCArgs: []string{}, EnableAutoGitWireProtocol: true, Timeout: struct { From d05d547bdd734b3098b8ae35f0b2a740114f60f9 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 11 Jan 2020 22:26:34 +0000 Subject: [PATCH 3/5] Update cmd/hook.go --- cmd/hook.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/hook.go b/cmd/hook.go index 9fd7d0a0df82..114897513a34 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -122,6 +122,7 @@ func (d *delayWriter) Close() error { return nil } _, err := d.internal.Write(d.buf.Bytes()) + d.buf = nil return err } From 6bc78103406167c204941a4325c072b865e82ac6 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 11 Jan 2020 22:31:19 +0000 Subject: [PATCH 4/5] Apply suggestions from code review --- cmd/hook.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/hook.go b/cmd/hook.go index 114897513a34..0f94e4b37083 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -353,7 +353,7 @@ Gitea or set your environment appropriately.`, "") hookOptions.RefFullNames = refFullNames resp, err := private.HookPostReceive(repoUser, repoName, hookOptions) if resp == nil { - dWriter.Close() + _ = dWriter.Close() hookPrintResults(results) fail("Internal Server Error", err) } @@ -373,7 +373,7 @@ Gitea or set your environment appropriately.`, "") } fmt.Fprintf(out, "Processed %d references in total\n", total) - dWriter.Close() + _ = dWriter.Close() hookPrintResults(results) return nil } @@ -386,7 +386,7 @@ Gitea or set your environment appropriately.`, "") resp, err := private.HookPostReceive(repoUser, repoName, hookOptions) if resp == nil { - dWriter.Close() + _ = dWriter.Close() hookPrintResults(results) fail("Internal Server Error", err) } @@ -402,7 +402,7 @@ Gitea or set your environment appropriately.`, "") fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err) } } - dWriter.Close() + _ = dWriter.Close() hookPrintResults(results) return nil From b87df1504904393237b669f66641a86a3b5ea4ff Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 12 Jan 2020 05:14:30 +0000 Subject: [PATCH 5/5] Update cmd/hook.go --- cmd/hook.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/hook.go b/cmd/hook.go index 0f94e4b37083..331f6a2d2ddf 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -117,7 +117,10 @@ func (d *delayWriter) Close() error { if stopped { return nil } - <-d.timer.C + select { + case <-d.timer.C: + default: + } if d.buf == nil { return nil }