Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8589aed
First try to add progress with status bars
mrnugget Oct 2, 2020
1196b37
Refactoring and renaming
mrnugget Oct 2, 2020
7e01f56
Tiny step and refactoring
mrnugget Oct 2, 2020
d53fbae
Get ProgressWithStatusBars working
mrnugget Oct 5, 2020
bbbff3d
Change API of progressWithStatusbars
mrnugget Oct 5, 2020
d863ac4
Refactor StatusBar printing
mrnugget Oct 5, 2020
488f932
Embed ProgressTTY in ProgressWithStatusBarsTTY
mrnugget Oct 5, 2020
a88257b
More merging of progressTTY and progressWithStatusbarsTTY
mrnugget Oct 5, 2020
44d8678
Add simple progress with status bars
mrnugget Oct 5, 2020
b9062e8
Move demo of progress with bars to _examples
mrnugget Oct 5, 2020
8b73335
Restore overwritten methods
mrnugget Oct 5, 2020
58ee488
Restore more overwritten methods
mrnugget Oct 6, 2020
f2dd25b
Restore another overwritten method
mrnugget Oct 6, 2020
e2b03e8
WIP: Wire up status bars with executor
mrnugget Oct 6, 2020
5ef4682
Print status bars below progress bar and add ASCII box characters
mrnugget Oct 6, 2020
fb40ebd
WIP1: Introduce campaign progress printer
mrnugget Oct 6, 2020
e924abd
WIP2: More refactoring of campaign status printing
mrnugget Oct 6, 2020
e40066d
More cleanup and refactoring
mrnugget Oct 6, 2020
1d74968
Change styling of progress bar with status
mrnugget Oct 6, 2020
ecdeded
Add time to StatusBar and display right-aligned
mrnugget Oct 7, 2020
644f6d2
Use a const
mrnugget Oct 7, 2020
5dacfdd
Add CHANGELOG entry
mrnugget Oct 7, 2020
fbd4e63
Ignore invisible characters for text length in progress bar
mrnugget Oct 8, 2020
c3aec3f
Update CHANGELOG.md
mrnugget Oct 8, 2020
0f78474
Erase previous content with spaces on blank line
mrnugget Oct 8, 2020
90a7c3e
Thin box drawing lines
mrnugget Oct 8, 2020
7240453
Improve non-TTY output for progress indicators with status bars.
LawnGnome Oct 8, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ All notable changes to `src-cli` are documented in this file.
- Error reporting by `src campaign [preview|apply]` has been improved and now includes more information about which step failed in which repository. [#325](https://github.com/sourcegraph/src-cli/pull/325)
- The default behaviour of `src campaigns [preview|apply]` has been changed to retain downloaded archives of repositories for better performance across re-runs of the command. To use the old behaviour and delete the archives use the `-clean-archives` flag. Repository archives are also not stored in the directory for temp data (see `-tmp` flag) anymore but in the cache directory, which can be configured with the `-cache` flag. To manually delete archives between runs, delete the `*.zip` files in the `-cache` directory (see `src campaigns -help` for its default location).
- `src campaign [preview|apply]` now check whether `git` and `docker` are available before trying to execute a campaign spec's steps. [#326](https://github.com/sourcegraph/src-cli/pull/326)
- The progress bar displayed by `src campaign [preview|apply]` has been extended by status bars that show which steps are currently being executed for each repository. [#338](https://github.com/sourcegraph/src-cli/pull/338)

### Fixed

Expand Down
188 changes: 188 additions & 0 deletions cmd/src/campaign_progress_printer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
package main

import (
"fmt"
"strings"

"github.com/sourcegraph/go-diff/diff"
"github.com/sourcegraph/src-cli/internal/campaigns"
"github.com/sourcegraph/src-cli/internal/output"
)

func newCampaignProgressPrinter(out *output.Output, numParallelism int) *campaignProgressPrinter {
return &campaignProgressPrinter{
out: out,

numParallelism: numParallelism,

completedTasks: map[string]bool{},
runningTasks: map[string]*campaigns.TaskStatus{},

repoStatusBar: map[string]int{},
statusBarRepo: map[int]string{},
}
}

type campaignProgressPrinter struct {
out *output.Output
progress output.ProgressWithStatusBars

maxRepoName int
numParallelism int

completedTasks map[string]bool
runningTasks map[string]*campaigns.TaskStatus

repoStatusBar map[string]int
statusBarRepo map[int]string
}

func (p *campaignProgressPrinter) initProgressBar(statuses []*campaigns.TaskStatus) {
statusBars := []*output.StatusBar{}
for i := 0; i < p.numParallelism; i++ {
statusBars = append(statusBars, output.NewStatusBarWithLabel("Starting worker..."))
}

p.progress = p.out.ProgressWithStatusBars([]output.ProgressBar{{
Label: fmt.Sprintf("Executing steps in %d repositories", len(statuses)),
Max: float64(len(statuses)),
}}, statusBars, nil)
}

func (p *campaignProgressPrinter) Complete() {
if p.progress != nil {
p.progress.Complete()
}
}

func (p *campaignProgressPrinter) PrintStatuses(statuses []*campaigns.TaskStatus) {
if p.progress == nil {
p.initProgressBar(statuses)
}

newlyCompleted := []*campaigns.TaskStatus{}
currentlyRunning := []*campaigns.TaskStatus{}

for _, ts := range statuses {
if len(ts.RepoName) > p.maxRepoName {
p.maxRepoName = len(ts.RepoName)
}

if ts.IsCompleted() {
if !p.completedTasks[ts.RepoName] {
p.completedTasks[ts.RepoName] = true
newlyCompleted = append(newlyCompleted, ts)
}

if _, ok := p.runningTasks[ts.RepoName]; ok {
delete(p.runningTasks, ts.RepoName)

// Free slot
idx := p.repoStatusBar[ts.RepoName]
delete(p.statusBarRepo, idx)
}
}

if ts.IsRunning() {
currentlyRunning = append(currentlyRunning, ts)
}

}

p.progress.SetValue(0, float64(len(p.completedTasks)))

newlyStarted := map[string]*campaigns.TaskStatus{}
statusBarIndex := 0
for _, ts := range currentlyRunning {
if _, ok := p.runningTasks[ts.RepoName]; ok {
continue
}

newlyStarted[ts.RepoName] = ts
p.runningTasks[ts.RepoName] = ts

// Find free slot
_, ok := p.statusBarRepo[statusBarIndex]
for ok {
statusBarIndex += 1
_, ok = p.statusBarRepo[statusBarIndex]
}

p.statusBarRepo[statusBarIndex] = ts.RepoName
p.repoStatusBar[ts.RepoName] = statusBarIndex
}

for _, ts := range newlyCompleted {
statusText, err := taskStatusText(ts)
if err != nil {
p.progress.Verbosef("%-*s failed to display status: %s", p.maxRepoName, ts.RepoName, err)
continue
}

p.progress.Verbosef("%-*s %s", p.maxRepoName, ts.RepoName, statusText)

if idx, ok := p.repoStatusBar[ts.RepoName]; ok {
// Log that this task completed, but only if there is no
// currently executing one in this bar, to avoid flicker.
if _, ok := p.statusBarRepo[idx]; !ok {
p.progress.StatusBarCompletef(idx, statusText)
}
delete(p.repoStatusBar, ts.RepoName)
}
}

for statusBar, repo := range p.statusBarRepo {
ts, ok := p.runningTasks[repo]
if !ok {
// This should not happen
continue
}

statusText, err := taskStatusText(ts)
if err != nil {
p.progress.Verbosef("%-*s failed to display status: %s", p.maxRepoName, ts.RepoName, err)
continue
}

if _, ok := newlyStarted[repo]; ok {
p.progress.StatusBarResetf(statusBar, ts.RepoName, statusText)
} else {
p.progress.StatusBarUpdatef(statusBar, statusText)
}
}
}

func taskStatusText(ts *campaigns.TaskStatus) (string, error) {
var statusText string

if ts.IsCompleted() {
if ts.ChangesetSpec == nil {
statusText = "No changes"
} else {
fileDiffs, err := diff.ParseMultiFileDiff([]byte(ts.ChangesetSpec.Commits[0].Diff))
if err != nil {
return "", err
}

statusText = diffStatDescription(fileDiffs) + " " + diffStatDiagram(sumDiffStats(fileDiffs))
}

if ts.Cached {
statusText += " (cached)"
}
} else if ts.IsRunning() {
if ts.CurrentlyExecuting != "" {
lines := strings.Split(ts.CurrentlyExecuting, "\n")
escapedLine := strings.ReplaceAll(lines[0], "%", "%%")
if len(lines) > 1 {
statusText = fmt.Sprintf("%s ...", escapedLine)
} else {
statusText = fmt.Sprintf("%s", escapedLine)
}
} else {
statusText = fmt.Sprintf("...")
}
}

return statusText, nil
}
84 changes: 5 additions & 79 deletions cmd/src/campaigns_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,12 @@ func campaignsExecute(ctx context.Context, out *output.Output, svc *campaigns.Se
campaignsCompletePending(pending, "Resolved repositories")
}

execProgress, execProgressComplete := executeCampaignSpecProgress(out)
specs, err := svc.ExecuteCampaignSpec(ctx, repos, executor, campaignSpec, execProgress)
p := newCampaignProgressPrinter(out, opts.Parallelism)
specs, err := svc.ExecuteCampaignSpec(ctx, repos, executor, campaignSpec, p.PrintStatuses)
if err != nil {
return "", "", err
}
execProgressComplete()
p.Complete()

if logFiles := executor.LogFiles(); len(logFiles) > 0 && flags.keepLogs {
func() {
Expand Down Expand Up @@ -372,9 +372,10 @@ func diffStatDiagram(stat diff.Stat) string {
added *= x
deleted *= x
}
return fmt.Sprintf("%s%s%s%s",
return fmt.Sprintf("%s%s%s%s%s",
output.StyleLinesAdded, strings.Repeat("+", int(added)),
output.StyleLinesDeleted, strings.Repeat("-", int(deleted)),
output.StyleReset,
)
}

Expand Down Expand Up @@ -409,78 +410,3 @@ func contextCancelOnInterrupt(parent context.Context) (context.Context, func())
ctxCancel()
}
}

// executeCampaignSpecProgress returns a function that can be passed to
// (*Service).ExecuteCampaignSpec as the "progress" function.
//
// It prints a progress bar and, if verbose mode is activated, diff stats of
// the produced diffs.
//
// The second return value is the "complete" function that completes the
// progress bar and should be called after ExecuteCampaignSpec returns
// successfully.
func executeCampaignSpecProgress(out *output.Output) (func(statuses []*campaigns.TaskStatus), func()) {
var (
progress output.Progress
maxRepoName int
completedTasks = map[string]bool{}
)

complete := func() {
if progress != nil {
progress.Complete()
}
}

progressFunc := func(statuses []*campaigns.TaskStatus) {
if progress == nil {
progress = out.Progress([]output.ProgressBar{{
Label: fmt.Sprintf("Executing steps in %d repositories", len(statuses)),
Max: float64(len(statuses)),
}}, nil)
}

unloggedCompleted := []*campaigns.TaskStatus{}

for _, ts := range statuses {
if len(ts.RepoName) > maxRepoName {
maxRepoName = len(ts.RepoName)
}

if ts.FinishedAt.IsZero() {
continue
}

if !completedTasks[ts.RepoName] {
completedTasks[ts.RepoName] = true
unloggedCompleted = append(unloggedCompleted, ts)
}

}

progress.SetValue(0, float64(len(completedTasks)))

for _, ts := range unloggedCompleted {
var statusText string

if ts.ChangesetSpec == nil {
statusText = "No changes"
} else {
fileDiffs, err := diff.ParseMultiFileDiff([]byte(ts.ChangesetSpec.Commits[0].Diff))
if err != nil {
panic(err)
}

statusText = diffStatDescription(fileDiffs) + " " + diffStatDiagram(sumDiffStats(fileDiffs))
}

if ts.Cached {
statusText += " (cached)"
}

progress.Verbosef("%-*s %s", maxRepoName, ts.RepoName, statusText)
}
}

return progressFunc, complete
}
15 changes: 14 additions & 1 deletion internal/campaigns/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,21 @@ type TaskStatus struct {
FinishedAt time.Time

// TODO: add current step and progress fields.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this todo comment related to the newly added line? If not, I'd add a newline here to avoid confusion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let @LawnGnome decide. He added the TODO and I wasn't sure whether he had something else in mind, like

TotalSteps int
CurrentStep int

or something like that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my intention, but I think it's unnecessary now with the work that you've done since I wrote that, so we could remove the TODO.

CurrentlyExecuting string

// Result fields.
ChangesetSpec *ChangesetSpec
Err error
}

func (ts *TaskStatus) IsRunning() bool {
return !ts.StartedAt.IsZero() && ts.FinishedAt.IsZero()
}

func (ts *TaskStatus) IsCompleted() bool {
return !ts.StartedAt.IsZero() && !ts.FinishedAt.IsZero()
}

type executor struct {
ExecutorOpts

Expand Down Expand Up @@ -156,6 +165,7 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) {
// Ensure that the status is updated when we're done.
defer func() {
status.FinishedAt = time.Now()
status.CurrentlyExecuting = ""
status.Err = err
x.updateTaskStatus(task, status)
}()
Expand Down Expand Up @@ -231,7 +241,10 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) {
defer cancel()

// Actually execute the steps.
diff, err := runSteps(runCtx, x.creator, task.Repository, task.Steps, log, x.tempDir)
diff, err := runSteps(runCtx, x.creator, task.Repository, task.Steps, log, x.tempDir, func(currentlyExecuting string) {
status.CurrentlyExecuting = currentlyExecuting
x.updateTaskStatus(task, status)
})
if err != nil {
if reachedTimeout(runCtx, err) {
err = &errTimeoutReached{timeout: x.Timeout}
Expand Down
7 changes: 6 additions & 1 deletion internal/campaigns/run_steps.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import (
"github.com/sourcegraph/src-cli/internal/campaigns/graphql"
)

func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repository, steps []Step, logger *TaskLogger, tempDir string) ([]byte, error) {
func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repository, steps []Step, logger *TaskLogger, tempDir string, reportProgress func(string)) ([]byte, error) {
reportProgress("Downloading archive")

volumeDir, err := wc.Create(ctx, repo)
if err != nil {
return nil, errors.Wrap(err, "creating workspace")
Expand All @@ -34,6 +36,7 @@ func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repositor
return out, nil
}

reportProgress("Initializing workspace")
if _, err := runGitCmd("init"); err != nil {
return nil, errors.Wrap(err, "git init failed")
}
Expand All @@ -59,6 +62,7 @@ func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repositor

for i, step := range steps {
logger.Logf("[Step %d] docker run %s %q", i+1, step.Container, step.Run)
reportProgress(step.Run)

cidFile, err := ioutil.TempFile(tempDir, repo.Slug()+"-container-id")
if err != nil {
Expand Down Expand Up @@ -143,6 +147,7 @@ func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repositor
return nil, errors.Wrap(err, "git add failed")
}

reportProgress("Calculating diff")
// As of Sourcegraph 3.14 we only support unified diff format.
// That means we need to strip away the `a/` and `/b` prefixes with `--no-prefix`.
// See: https://github.com/sourcegraph/sourcegraph/blob/82d5e7e1562fef6be5c0b17f18631040fd330835/enterprise/internal/campaigns/service.go#L324-L329
Expand Down
Loading