Skip to content

Commit

Permalink
Introduce transformChanges to campaign spec (RFC 265) (#398)
Browse files Browse the repository at this point in the history
* Add transformChanges to campaign spec

* Update Executor integration test

* Add a failing test for code transformations

* Change execution cache to only cache diffs

* Rename from .patch to .diff

* Extract groupFileDiffs function

* Add test for GroupFileDiffs

* Add some comments

* Fix campaign progress printer for multiple changeset specs

* Display how many changeset specs were produced in one repo

* Add more tests for grouping changes

* Fix problems after rebase

* Switch from branchSuffix to branch

* Add a repository filter to the transformChanges.Group

* Check whether transformChanges is supported

* Validate that multiple changesets don't have same branch

* Add minLength to campaign spec

* Pluralize message correctly

* Update internal/campaigns/executor.go

Co-authored-by: Adam Harvey <aharvey@sourcegraph.com>

* Update feature date

* Add a changelog entry

Co-authored-by: Adam Harvey <aharvey@sourcegraph.com>
  • Loading branch information
mrnugget and LawnGnome authored Dec 11, 2020
1 parent bce34e8 commit 6cccdc7
Show file tree
Hide file tree
Showing 12 changed files with 631 additions and 152 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ All notable changes to `src-cli` are documented in this file.

### Added

- Experimental: [`transformChanges` in campaign specs](https://docs.sourcegraph.com/campaigns/references/campaign_spec_yaml_reference#transformchanges) is now available as a feature preview to allow users to create multiple changesets in a single repository. [#398](https://github.com/sourcegraph/src-cli/pull/398)

### Changed

- `src campaign [apply|preview]` now show the current execution progress in numbers next to the progress bar. [#396](https://github.com/sourcegraph/src-cli/pull/396)
Expand Down
34 changes: 16 additions & 18 deletions cmd/src/campaign_progress_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,21 +177,16 @@ func (p *campaignProgressPrinter) PrintStatuses(statuses []*campaigns.TaskStatus
}

for _, ts := range newlyCompleted {
var fileDiffs []*diff.FileDiff

if ts.ChangesetSpec != nil {
var err error
fileDiffs, err = diff.ParseMultiFileDiff([]byte(ts.ChangesetSpec.Commits[0].Diff))
if err != nil {
p.progress.Verbosef("%-*s failed to display status: %s", p.maxRepoName, ts.RepoName, err)
continue
}
fileDiffs, hasDiffs, err := ts.FileDiffs()
if err != nil {
p.progress.Verbosef("%-*s failed to display status: %s", p.maxRepoName, ts.RepoName, err)
continue
}

if p.verbose {
p.progress.WriteLine(output.Linef("", output.StylePending, "%s", ts.RepoName))

if ts.ChangesetSpec == nil {
if !hasDiffs {
p.progress.Verbosef(" No changes")
} else {
lines, err := verboseDiffSummary(fileDiffs)
Expand All @@ -205,6 +200,9 @@ func (p *campaignProgressPrinter) PrintStatuses(statuses []*campaigns.TaskStatus
}
}

if len(ts.ChangesetSpecs) > 1 {
p.progress.Verbosef(" %d changeset specs generated", len(ts.ChangesetSpecs))
}
p.progress.Verbosef(" Execution took %s", ts.ExecutionTime())
p.progress.Verbose("")
}
Expand Down Expand Up @@ -258,7 +256,14 @@ func taskStatusBarText(ts *campaigns.TaskStatus) (string, error) {
var statusText string

if ts.IsCompleted() {
if ts.ChangesetSpec == nil {
diffs, hasDiffs, err := ts.FileDiffs()
if err != nil {
return "", err
}

if hasDiffs {
statusText = diffStatDescription(diffs) + " " + diffStatDiagram(sumDiffStats(diffs))
} else {
if ts.Err != nil {
if texter, ok := ts.Err.(statusTexter); ok {
statusText = texter.StatusText()
Expand All @@ -268,13 +273,6 @@ func taskStatusBarText(ts *campaigns.TaskStatus) (string, error) {
} else {
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 {
Expand Down
30 changes: 16 additions & 14 deletions cmd/src/campaign_progress_printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,22 +95,24 @@ func TestCampaignProgressPrinterIntegration(t *testing.T) {
FinishedAt: now.Add(time.Duration(5) * time.Second),
CurrentlyExecuting: "",
Err: nil,
ChangesetSpec: &campaigns.ChangesetSpec{
BaseRepository: "graphql-id",
CreatedChangeset: &campaigns.CreatedChangeset{
BaseRef: "refs/heads/main",
BaseRev: "d34db33f",
HeadRepository: "graphql-id",
HeadRef: "refs/heads/my-campaign",
Title: "This is my campaign",
Body: "This is my campaign",
Commits: []campaigns.GitCommitDescription{
{
Message: "This is my campaign",
Diff: progressPrinterDiff,
ChangesetSpecs: []*campaigns.ChangesetSpec{
{
BaseRepository: "graphql-id",
CreatedChangeset: &campaigns.CreatedChangeset{
BaseRef: "refs/heads/main",
BaseRev: "d34db33f",
HeadRepository: "graphql-id",
HeadRef: "refs/heads/my-campaign",
Title: "This is my campaign",
Body: "This is my campaign",
Commits: []campaigns.GitCommitDescription{
{
Message: "This is my campaign",
Diff: progressPrinterDiff,
},
},
Published: false,
},
Published: false,
},
},
}
Expand Down
11 changes: 9 additions & 2 deletions cmd/src/campaigns_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,17 @@ func campaignsExecute(ctx context.Context, out *output.Output, svc *campaigns.Se
ids := make([]campaigns.ChangesetSpecID, len(specs))

if len(specs) > 0 {
var label string
if len(specs) == 1 {
label = "Sending changeset spec"
} else {
label = fmt.Sprintf("Sending %d changeset specs", len(specs))
}

progress := out.Progress([]output.ProgressBar{
{Label: "Sending changeset specs", Max: float64(len(specs))},
{Label: label, Max: float64(len(specs))},
}, nil)

for i, spec := range specs {
id, err := svc.CreateChangesetSpec(ctx, spec)
if err != nil {
Expand All @@ -276,7 +284,6 @@ func campaignsExecute(ctx context.Context, out *output.Output, svc *campaigns.Se
progress.SetValue(0, float64(i+1))
}
progress.Complete()

} else {
if len(repos) == 0 {
out.WriteLine(output.Linef(output.EmojiWarning, output.StyleWarning, `No changeset specs created`))
Expand Down
22 changes: 17 additions & 5 deletions internal/campaigns/campaign_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type CampaignSpec struct {
Description string `json:"description,omitempty" yaml:"description"`
On []OnQueryOrRepository `json:"on,omitempty" yaml:"on"`
Steps []Step `json:"steps,omitempty" yaml:"steps"`
TransformChanges *TransformChanges `json:"transformChanges,omitempty" yaml:"transformChanges,omitempty"`
ImportChangesets []ImportChangeset `json:"importChangesets,omitempty" yaml:"importChangesets"`
ChangesetTemplate *ChangesetTemplate `json:"changesetTemplate,omitempty" yaml:"changesetTemplate"`
}
Expand Down Expand Up @@ -74,26 +75,37 @@ type Step struct {
image string
}

type TransformChanges struct {
Group []Group `json:"group,omitempty" yaml:"group"`
}

type Group struct {
Directory string `json:"directory,omitempty" yaml:"directory"`
Branch string `json:"branch,omitempty" yaml:"branch"`
Repository string `json:"repository,omitempty" yaml:"repository"`
}

func ParseCampaignSpec(data []byte, features featureFlags) (*CampaignSpec, error) {
var spec CampaignSpec
if err := yaml.UnmarshalValidate(schema.CampaignSpecJSON, data, &spec); err != nil {
return nil, err
}

var errs *multierror.Error

if !features.allowArrayEnvironments {
var errs *multierror.Error
for i, step := range spec.Steps {
if !step.Env.IsStatic() {
errs = multierror.Append(errs, errors.Errorf("step %d includes one or more dynamic environment variables, which are unsupported in this Sourcegraph version", i+1))
}
}
}

if err := errs.ErrorOrNil(); err != nil {
return nil, err
}
if spec.TransformChanges != nil && !features.allowtransformChanges {
errs = multierror.Append(errs, errors.New("campaign spec includes transformChanges, which is not supported in this Sourcegraph version"))
}

return &spec, nil
return &spec, errs.ErrorOrNil()
}

func (on *OnQueryOrRepository) String() string {
Expand Down
55 changes: 32 additions & 23 deletions internal/campaigns/execution_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import (
"crypto/sha256"
"encoding/base64"
"encoding/json"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"strings"

"github.com/pkg/errors"
)
Expand Down Expand Up @@ -60,8 +62,8 @@ func (key ExecutionCacheKey) Key() (string, error) {
}

type ExecutionCache interface {
Get(ctx context.Context, key ExecutionCacheKey) (result *ChangesetSpec, err error)
Set(ctx context.Context, key ExecutionCacheKey, result *ChangesetSpec) error
Get(ctx context.Context, key ExecutionCacheKey) (diff string, found bool, err error)
Set(ctx context.Context, key ExecutionCacheKey, diff string) error
Clear(ctx context.Context, key ExecutionCacheKey) error
}

Expand All @@ -75,41 +77,48 @@ func (c ExecutionDiskCache) cacheFilePath(key ExecutionCacheKey) (string, error)
return "", errors.Wrap(err, "calculating execution cache key")
}

return filepath.Join(c.Dir, keyString+".json"), nil
return filepath.Join(c.Dir, keyString+".diff"), nil
}

func (c ExecutionDiskCache) Get(ctx context.Context, key ExecutionCacheKey) (*ChangesetSpec, error) {
func (c ExecutionDiskCache) Get(ctx context.Context, key ExecutionCacheKey) (string, bool, error) {
path, err := c.cacheFilePath(key)
if err != nil {
return nil, err
return "", false, err
}

data, err := ioutil.ReadFile(path)
if err != nil {
if os.IsNotExist(err) {
err = nil // treat as not-found
}
return nil, err
return "", false, err
}

var result ChangesetSpec
if err := json.Unmarshal(data, &result); err != nil {
// Delete the invalid data to avoid causing an error for next time.
if err := os.Remove(path); err != nil {
return nil, errors.Wrap(err, "while deleting cache file with invalid JSON")
// We previously cached complete ChangesetSpecs instead of just the diffs.
// To be backwards compatible, we keep reading these:
if strings.HasSuffix(path, ".json") {
var result ChangesetSpec
if err := json.Unmarshal(data, &result); err != nil {
// Delete the invalid data to avoid causing an error for next time.
if err := os.Remove(path); err != nil {
return "", false, errors.Wrap(err, "while deleting cache file with invalid JSON")
}
return "", false, errors.Wrapf(err, "reading cache file %s", path)
}
return nil, errors.Wrapf(err, "reading cache file %s", path)
if len(result.Commits) != 1 {
return "", false, errors.New("cached result has no commits")
}
return result.Commits[0].Diff, true, nil
}

return &result, nil
}

func (c ExecutionDiskCache) Set(ctx context.Context, key ExecutionCacheKey, result *ChangesetSpec) error {
data, err := json.Marshal(result)
if err != nil {
return err
if strings.HasSuffix(path, ".diff") {
return string(data), true, nil
}

return "", false, fmt.Errorf("unknown file format for cache file %q", path)
}

func (c ExecutionDiskCache) Set(ctx context.Context, key ExecutionCacheKey, diff string) error {
path, err := c.cacheFilePath(key)
if err != nil {
return err
Expand All @@ -119,7 +128,7 @@ func (c ExecutionDiskCache) Set(ctx context.Context, key ExecutionCacheKey, resu
return err
}

return ioutil.WriteFile(path, data, 0600)
return ioutil.WriteFile(path, []byte(diff), 0600)
}

func (c ExecutionDiskCache) Clear(ctx context.Context, key ExecutionCacheKey) error {
Expand All @@ -139,11 +148,11 @@ func (c ExecutionDiskCache) Clear(ctx context.Context, key ExecutionCacheKey) er
// retrieve cache entries.
type ExecutionNoOpCache struct{}

func (ExecutionNoOpCache) Get(ctx context.Context, key ExecutionCacheKey) (result *ChangesetSpec, err error) {
return nil, nil
func (ExecutionNoOpCache) Get(ctx context.Context, key ExecutionCacheKey) (diff string, found bool, err error) {
return "", false, nil
}

func (ExecutionNoOpCache) Set(ctx context.Context, key ExecutionCacheKey, result *ChangesetSpec) error {
func (ExecutionNoOpCache) Set(ctx context.Context, key ExecutionCacheKey, diff string) error {
return nil
}

Expand Down
Loading

0 comments on commit 6cccdc7

Please sign in to comment.