Repository: goreleaser/goreleaser
Pull request #1425 (2020-04-07 01:11:48)

feat: split brew tap in 2 steps
currently its hard to find out what the end brew tap will look like without publishing it.

this worked once but was broken a couple of times already, so I went one step ahead and tried to fix it properly by splitting it in:

- Piper implementation that only creates the file on `dist`
- Publisher implementation that actually pushes that

To make that work, I had to do some changes on the `env` pipe, most of them related to token validation. Basically it will only error if its a regular release and no token was provided, otherwise it will just go along...

eventually need to do the same thing with the scoop pipe.


closes https://github.com/goreleaser/goreleaser/issues/1237

codecov-io: # [Codecov](https://codecov.io/gh/goreleaser/goreleaser/pull/1425?src=pr&el=h1) Report
> Merging [#1425](https://codecov.io/gh/goreleaser/goreleaser/pull/1425?src=pr&el=desc) into [master](https://codecov.io/gh/goreleaser/goreleaser/commit/88213a50fcacd0ea17b7bf8c08df2dae33835f60&el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `84.84%`.

[![Impacted file tree graph](https://codecov.io/gh/goreleaser/goreleaser/pull/1425/graphs/tree.svg?width=650&height=150&src=pr&token=3bJvv6b7hN)](https://codecov.io/gh/goreleaser/goreleaser/pull/1425?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1425      +/-   ##
==========================================
- Coverage   83.83%   83.83%   -0.01%     
==========================================
  Files          57       57              
  Lines        3360     3390      +30     
==========================================
+ Hits         2817     2842      +25     
- Misses        462      465       +3     
- Partials       81       83       +2     
```


| [Impacted Files](https://codecov.io/gh/goreleaser/goreleaser/pull/1425?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/artifact/artifact.go](https://codecov.io/gh/goreleaser/goreleaser/pull/1425/diff?src=pr&el=tree#diff-aW50ZXJuYWwvYXJ0aWZhY3QvYXJ0aWZhY3QuZ28=) | `82.20% <0.00%> (-1.42%)` | :arrow_down: |
| [internal/pipe/scoop/scoop.go](https://codecov.io/gh/goreleaser/goreleaser/pull/1425/diff?src=pr&el=tree#diff-aW50ZXJuYWwvcGlwZS9zY29vcC9zY29vcC5nbw==) | `92.23% <ø> (ø)` | |
| [internal/pipe/brew/brew.go](https://codecov.io/gh/goreleaser/goreleaser/pull/1425/diff?src=pr&el=tree#diff-aW50ZXJuYWwvcGlwZS9icmV3L2JyZXcuZ28=) | `89.16% <82.97%> (-0.29%)` | :arrow_down: |
| [internal/pipe/env/env.go](https://codecov.io/gh/goreleaser/goreleaser/pull/1425/diff?src=pr&el=tree#diff-aW50ZXJuYWwvcGlwZS9lbnYvZW52Lmdv) | `97.01% <100.00%> (+0.24%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/goreleaser/goreleaser/pull/1425?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/goreleaser/goreleaser/pull/1425?src=pr&el=footer). Last update [88213a5...e29a572](https://codecov.io/gh/goreleaser/goreleaser/pull/1425?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go
index fb161ad2..8f4f44a9 100644
--- a/internal/artifact/artifact.go
+++ b/internal/artifact/artifact.go
@@ -29,6 +29,8 @@ const (
 	UploadableBinary
 	// UploadableFile is any file that can be uploaded
 	UploadableFile
+	// UploadableBrewTap is a .rb file that can be uploaded
+	UploadableBrewTap
 	// Binary is a binary (output of a gobuild)
 	Binary
 	// LinuxPackage is a linux package generated by nfpm
@@ -54,6 +56,8 @@ func (t Type) String() string {
 	case UploadableBinary:
 	case Binary:
 		return "Binary"
+	case UploadableBrewTap:
+		return "Brew Tap"
 	case LinuxPackage:
 		return "Linux Package"
 	case DockerImage:
diff --git a/internal/pipe/brew/brew.go b/internal/pipe/brew/brew.go
index 3f4745c0..0d2e8aeb 100644
--- a/internal/pipe/brew/brew.go
+++ b/internal/pipe/brew/brew.go
@@ -2,7 +2,6 @@ package brew
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"path"
@@ -17,6 +16,7 @@ import (
 	"github.com/goreleaser/goreleaser/internal/tmpl"
 	"github.com/goreleaser/goreleaser/pkg/config"
 	"github.com/goreleaser/goreleaser/pkg/context"
+	"github.com/pkg/errors"
 )
 
 // ErrNoArchivesFound happens when 0 archives are found
@@ -36,14 +36,64 @@ func (Pipe) String() string {
 	return "homebrew tap formula"
 }
 
+func (Pipe) Run(ctx *context.Context) error {
+	for _, brew := range ctx.Config.Brews {
+		if err := doRun(ctx, brew); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // Publish brew formula
 func (Pipe) Publish(ctx *context.Context) error {
 	client, err := client.New(ctx)
 	if err != nil {
 		return err
 	}
-	for _, brew := range ctx.Config.Brews {
-		if err := doRun(ctx, brew, client); err != nil {
+
+	return doPublish(ctx, client)
+}
+
+func doPublish(ctx *context.Context, client client.Client) error {
+	if ctx.SkipPublish {
+		return pipe.ErrSkipPublishEnabled
+	}
+	var taps = ctx.Artifacts.Filter(artifact.ByType(artifact.UploadableBrewTap)).List()
+	if len(taps) == 0 {
+		return pipe.Skip("no brew taps found")
+	}
+	for _, tap := range taps {
+		brew := tap.Extra["config"].(config.Homebrew)
+		if strings.TrimSpace(brew.SkipUpload) == "true" {
+			return pipe.Skip("brew.skip_upload is set")
+		}
+		if strings.TrimSpace(brew.SkipUpload) == "auto" && ctx.Semver.Prerelease != "" {
+			return pipe.Skip("prerelease detected with 'auto' upload, skipping homebrew publish")
+		}
+
+		var repo config.Repo
+		switch ctx.TokenType {
+		case context.TokenTypeGitHub:
+			repo = brew.GitHub
+		case context.TokenTypeGitLab:
+			repo = brew.GitLab
+		default:
+			return ErrTokenTypeNotImplementedForBrew
+		}
+
+		var gpath = buildFormulaPath(brew.Folder, tap.Name)
+		log.WithField("formula", gpath).
+			WithField("repo", repo.String()).
+			Info("pushing")
+
+		content, err := ioutil.ReadFile(tap.Path)
+		if err != nil {
+			return errors.Wrap(err, "failed to read tap")
+		}
+
+		var msg = fmt.Sprintf("Brew formula update for %s version %s", ctx.Config.ProjectName, ctx.Git.CurrentTag)
+		if err := client.CreateFile(ctx, brew.CommitAuthor, repo, content, gpath, msg); err != nil {
 			return err
 		}
 	}
@@ -105,7 +155,7 @@ func contains(ss []string, s string) bool {
 	return false
 }
 
-func doRun(ctx *context.Context, brew config.Homebrew, client client.Client) error {
+func doRun(ctx *context.Context, brew config.Homebrew) error {
 	if brew.GitHub.Name == "" && brew.GitLab.Name == "" {
 		return pipe.Skip("brew section is not configured")
 	}
@@ -145,36 +195,19 @@ func doRun(ctx *context.Context, brew config.Homebrew, client client.Client) err
 	var path = filepath.Join(ctx.Config.Dist, filename)
 	log.WithField("formula", path).Info("writing")
 	if err := ioutil.WriteFile(path, []byte(content), 0644); err != nil {
-		return err
+		return errors.Wrap(err, "failed to write brew tap")
 	}
 
-	if strings.TrimSpace(brew.SkipUpload) == "true" {
-		return pipe.Skip("brew.skip_upload is set")
-	}
-	if ctx.SkipPublish {
-		return pipe.ErrSkipPublishEnabled
-	}
-	if strings.TrimSpace(brew.SkipUpload) == "auto" && ctx.Semver.Prerelease != "" {
-		return pipe.Skip("prerelease detected with 'auto' upload, skipping homebrew publish")
-	}
+	ctx.Artifacts.Add(&artifact.Artifact{
+		Name: filename,
+		Path: path,
+		Type: artifact.UploadableBrewTap,
+		Extra: map[string]interface{}{
+			"config": brew,
+		},
+	})
 
-	var repo config.Repo
-	switch ctx.TokenType {
-	case context.TokenTypeGitHub:
-		repo = brew.GitHub
-	case context.TokenTypeGitLab:
-		repo = brew.GitLab
-	default:
-		return ErrTokenTypeNotImplementedForBrew
-	}
-
-	var gpath = buildFormulaPath(brew.Folder, filename)
-	log.WithField("formula", gpath).
-		WithField("repo", repo.String()).
-		Info("pushing")
-
-	var msg = fmt.Sprintf("Brew formula update for %s version %s", ctx.Config.ProjectName, ctx.Git.CurrentTag)
-	return client.CreateFile(ctx, brew.CommitAuthor, repo, []byte(content), gpath, msg)
+	return nil
 }
 
 func buildFormulaPath(folder, filename string) string {
@@ -241,6 +274,7 @@ func dataFor(ctx *context.Context, cfg config.Homebrew, tokenType context.TokenT
 					ctx.Config.Release.GitLab.Name,
 				)
 			default:
+				log.WithField("type", tokenType).Info("here")
 				return result, ErrTokenTypeNotImplementedForBrew
 			}
 		}
diff --git a/internal/pipe/env/env.go b/internal/pipe/env/env.go
index a437023e..0c3b4333 100644
--- a/internal/pipe/env/env.go
+++ b/internal/pipe/env/env.go
@@ -6,7 +6,7 @@ import (
 	"bufio"
 	"os"
 
-	"github.com/goreleaser/goreleaser/internal/pipe"
+	"github.com/apex/log"
 	"github.com/goreleaser/goreleaser/pkg/context"
 	homedir "github.com/mitchellh/go-homedir"
 	"github.com/pkg/errors"
@@ -46,13 +46,6 @@ func (Pipe) Run(ctx *context.Context) error {
 	gitlabToken, gitlabTokenErr := loadEnv("GITLAB_TOKEN", ctx.Config.EnvFiles.GitLabToken)
 	giteaToken, giteaTokenErr := loadEnv("GITEA_TOKEN", ctx.Config.EnvFiles.GiteaToken)
 
-	if ctx.SkipPublish {
-		return pipe.ErrSkipPublishEnabled
-	}
-	if ctx.Config.Release.Disable {
-		return pipe.Skip("release pipe is disabled")
-	}
-
 	numOfTokens := 0
 	if githubToken != "" {
 		numOfTokens++
@@ -70,33 +63,24 @@ func (Pipe) Run(ctx *context.Context) error {
 	noTokens := githubToken == "" && gitlabToken == "" && giteaToken == ""
 	noTokenErrs := githubTokenErr == nil && gitlabTokenErr == nil && giteaTokenErr == nil
 
-	if noTokens && noTokenErrs {
-		return ErrMissingToken
-	}
-
-	if gitlabTokenErr != nil {
-		return errors.Wrap(gitlabTokenErr, "failed to load gitlab token")
-	}
-
-	if githubTokenErr != nil {
-		return errors.Wrap(githubTokenErr, "failed to load github token")
-	}
-
-	if giteaTokenErr != nil {
-		return errors.Wrap(giteaTokenErr, "failed to load gitea token")
+	if err := checkErrors(ctx, noTokens, noTokenErrs, gitlabTokenErr, githubTokenErr, giteaTokenErr); err != nil {
+		return err
 	}
 
 	if githubToken != "" {
+		log.Debug("token type: github")
 		ctx.TokenType = context.TokenTypeGitHub
 		ctx.Token = githubToken
 	}
 
 	if gitlabToken != "" {
+		log.Debug("token type: gitlab")
 		ctx.TokenType = context.TokenTypeGitLab
 		ctx.Token = gitlabToken
 	}
 
 	if giteaToken != "" {
+		log.Debug("token type: gitea")
 		ctx.TokenType = context.TokenTypeGitea
 		ctx.Token = giteaToken
 	}
@@ -104,6 +88,29 @@ func (Pipe) Run(ctx *context.Context) error {
 	return nil
 }
 
+func checkErrors(ctx *context.Context, noTokens, noTokenErrs bool, gitlabTokenErr, githubTokenErr, giteaTokenErr error) error {
+	if ctx.SkipPublish || ctx.Config.Release.Disable {
+		return nil
+	}
+
+	if noTokens && noTokenErrs {
+		return ErrMissingToken
+	}
+
+	if gitlabTokenErr != nil {
+		return errors.Wrap(gitlabTokenErr, "failed to load gitlab token")
+	}
+
+	if githubTokenErr != nil {
+		return errors.Wrap(githubTokenErr, "failed to load github token")
+	}
+
+	if giteaTokenErr != nil {
+		return errors.Wrap(giteaTokenErr, "failed to load gitea token")
+	}
+	return nil
+}
+
 func loadEnv(env, path string) (string, error) {
 	val := os.Getenv(env)
 	if val != "" {
diff --git a/internal/pipe/scoop/scoop.go b/internal/pipe/scoop/scoop.go
index 6d695b21..27bed73e 100644
--- a/internal/pipe/scoop/scoop.go
+++ b/internal/pipe/scoop/scoop.go
@@ -30,6 +30,7 @@ func (Pipe) String() string {
 
 // Publish scoop manifest
 func (Pipe) Publish(ctx *context.Context) error {
+	// TODO(caarlos0): split the creation of the json file and the publishing in two steps, like the brew pipe.
 	client, err := client.New(ctx)
 	if err != nil {
 		return err
diff --git a/internal/pipeline/pipeline.go b/internal/pipeline/pipeline.go
index 9d754076..56a19199 100644
--- a/internal/pipeline/pipeline.go
+++ b/internal/pipeline/pipeline.go
@@ -4,6 +4,7 @@ package pipeline
 import (
 	"fmt"
 
+	"github.com/goreleaser/goreleaser/internal/pipe/brew"
 	"github.com/goreleaser/goreleaser/internal/pipe/semver"
 
 	"github.com/goreleaser/goreleaser/internal/pipe/archive"
@@ -52,5 +53,6 @@ var Pipeline = []Piper{
 	checksums.Pipe{},       // checksums of the files
 	sign.Pipe{},            // sign artifacts
 	docker.Pipe{},          // create and push docker images
+	brew.Pipe{},            // create the formula file on dist
 	publish.Pipe{},         // publishes artifacts
 }


Test patch:
diff --git a/internal/pipe/brew/brew_test.go b/internal/pipe/brew/brew_test.go
index cd21bbab..ce466a34 100644
--- a/internal/pipe/brew/brew_test.go
+++ b/internal/pipe/brew/brew_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/goreleaser/goreleaser/pkg/config"
 	"github.com/goreleaser/goreleaser/pkg/context"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 var update = flag.Bool("update", false, "update .golden files")
@@ -248,7 +249,8 @@ func TestRunPipe(t *testing.T) {
 			client := &DummyClient{}
 			var distFile = filepath.Join(folder, name+".rb")
 
-			assert.NoError(t, doRun(ctx, ctx.Config.Brews[0], client))
+			require.NoError(t, Pipe{}.Run(ctx))
+			assert.NoError(t, doPublish(ctx, client))
 			assert.True(t, client.CreatedFile)
 			var golden = fmt.Sprintf("testdata/%s.rb.golden", name)
 			if *update {
@@ -375,7 +377,8 @@ func TestRunPipeForMultipleArmVersions(t *testing.T) {
 		client := &DummyClient{}
 		var distFile = filepath.Join(folder, name+".rb")
 
-		assert.NoError(t, doRun(ctx, ctx.Config.Brews[0], client))
+		require.NoError(t, Pipe{}.Run(ctx))
+		assert.NoError(t, doPublish(ctx, client))
 		assert.True(t, client.CreatedFile)
 		var golden = fmt.Sprintf("testdata/%s.rb.golden", name)
 		if *update {
@@ -406,8 +409,9 @@ func TestRunPipeNoDarwin64Build(t *testing.T) {
 		},
 	}
 	client := &DummyClient{}
-	assert.Equal(t, ErrNoArchivesFound, doRun(ctx, ctx.Config.Brews[0], client))
-	assert.False(t, client.CreatedFile)
+	require.Equal(t, ErrNoArchivesFound, Pipe{}.Run(ctx))
+	testlib.AssertSkipped(t, doPublish(ctx, client))
+	require.False(t, client.CreatedFile)
 }
 
 func TestRunPipeMultipleArchivesSameOsBuild(t *testing.T) {
@@ -550,8 +554,9 @@ func TestRunPipeMultipleArchivesSameOsBuild(t *testing.T) {
 			})
 		}
 		client := &DummyClient{}
-		assert.Equal(t, test.expectedError, doRun(ctx, ctx.Config.Brews[0], client))
-		assert.False(t, client.CreatedFile)
+		require.Equal(t, test.expectedError, Pipe{}.Run(ctx))
+		testlib.AssertSkipped(t, doPublish(ctx, client))
+		require.False(t, client.CreatedFile)
 		// clean the artifacts for the next run
 		ctx.Artifacts = artifact.New()
 	}
@@ -562,7 +567,8 @@ func TestRunPipeBrewNotSetup(t *testing.T) {
 		Config: config.Project{},
 	}
 	client := &DummyClient{}
-	testlib.AssertSkipped(t, doRun(ctx, config.Homebrew{}, client))
+	require.NoError(t, Pipe{}.Run(ctx))
+	testlib.AssertSkipped(t, doPublish(ctx, client))
 	assert.False(t, client.CreatedFile)
 }
 
@@ -587,59 +593,70 @@ func TestRunPipeBinaryRelease(t *testing.T) {
 		Type:   artifact.Binary,
 	})
 	client := &DummyClient{}
-	assert.Equal(t, ErrNoArchivesFound, doRun(ctx, ctx.Config.Brews[0], client))
-	assert.False(t, client.CreatedFile)
+	require.Equal(t, ErrNoArchivesFound, Pipe{}.Run(ctx))
+	testlib.AssertSkipped(t, doPublish(ctx, client))
+	require.False(t, client.CreatedFile)
 }
 
 func TestRunPipeNoUpload(t *testing.T) {
 	folder, err := ioutil.TempDir("", "goreleasertest")
 	assert.NoError(t, err)
-	var ctx = context.New(config.Project{
-		Dist:        folder,
-		ProjectName: "foo",
-		Release:     config.Release{},
-		Brews: []config.Homebrew{
-			{
-				GitHub: config.Repo{
-					Owner: "test",
-					Name:  "test",
+
+	var newCtx = func() *context.Context {
+		var ctx = context.New(config.Project{
+			Dist:        folder,
+			ProjectName: "foo",
+			Release:     config.Release{},
+			Brews: []config.Homebrew{
+				{
+					GitHub: config.Repo{
+						Owner: "test",
+						Name:  "test",
+					},
 				},
 			},
-		},
-	})
-	ctx.TokenType = context.TokenTypeGitHub
-	ctx.Git = context.GitInfo{CurrentTag: "v1.0.1"}
-	var path = filepath.Join(folder, "whatever.tar.gz")
-	_, err = os.Create(path)
-	assert.NoError(t, err)
-	ctx.Artifacts.Add(&artifact.Artifact{
-		Name:   "bin",
-		Path:   path,
-		Goos:   "darwin",
-		Goarch: "amd64",
-		Type:   artifact.UploadableArchive,
-		Extra: map[string]interface{}{
-			"ID":     "foo",
-			"Format": "tar.gz",
-		},
-	})
-	client := &DummyClient{}
+		})
+		ctx.TokenType = context.TokenTypeGitHub
+		ctx.Git = context.GitInfo{CurrentTag: "v1.0.1"}
+		var path = filepath.Join(folder, "whatever.tar.gz")
+		_, err = os.Create(path)
+		assert.NoError(t, err)
+		ctx.Artifacts.Add(&artifact.Artifact{
+			Name:   "bin",
+			Path:   path,
+			Goos:   "darwin",
+			Goarch: "amd64",
+			Type:   artifact.UploadableArchive,
+			Extra: map[string]interface{}{
+				"ID":     "foo",
+				"Format": "tar.gz",
+			},
+		})
+		return ctx
+	}
+	var client = &DummyClient{}
 
-	var assertNoPublish = func(t *testing.T) {
-		testlib.AssertSkipped(t, doRun(ctx, ctx.Config.Brews[0], client))
+	var assertNoPublish = func(t *testing.T, ctx *context.Context) {
+		require.NoError(t, Pipe{}.Run(ctx))
+		testlib.AssertSkipped(t, doPublish(ctx, client))
 		assert.False(t, client.CreatedFile)
 	}
+
 	t.Run("skip upload", func(tt *testing.T) {
+		var ctx = newCtx()
 		ctx.Config.Release.Draft = false
 		ctx.Config.Brews[0].SkipUpload = "true"
 		ctx.SkipPublish = false
-		assertNoPublish(tt)
+		require.NoError(t, Pipe{}.Run(ctx))
+		assertNoPublish(tt, ctx)
 	})
 	t.Run("skip publish", func(tt *testing.T) {
+		var ctx = newCtx()
 		ctx.Config.Release.Draft = false
 		ctx.Config.Brews[0].SkipUpload = "false"
 		ctx.SkipPublish = true
-		assertNoPublish(tt)
+		require.NoError(t, Pipe{}.Run(ctx))
+		assertNoPublish(tt, ctx)
 	})
 }
 
@@ -675,7 +692,8 @@ func TestRunTokenTypeNotImplementedForBrew(t *testing.T) {
 		},
 	})
 	client := &DummyClient{}
-	assert.Equal(t, ErrTokenTypeNotImplementedForBrew, doRun(ctx, ctx.Config.Brews[0], client))
+	require.Equal(t, ErrTokenTypeNotImplementedForBrew, Pipe{}.Run(ctx))
+	testlib.AssertSkipped(t, doPublish(ctx, client))
 }
 
 func TestDefault(t *testing.T) {
diff --git a/internal/pipe/env/env_test.go b/internal/pipe/env/env_test.go
index 94c6b7be..91c11105 100644
--- a/internal/pipe/env/env_test.go
+++ b/internal/pipe/env/env_test.go
@@ -6,7 +6,6 @@ import (
 	"os"
 	"testing"
 
-	"github.com/goreleaser/goreleaser/internal/testlib"
 	"github.com/goreleaser/goreleaser/pkg/config"
 	"github.com/goreleaser/goreleaser/pkg/context"
 	"github.com/stretchr/testify/assert"
@@ -172,7 +171,7 @@ func TestInvalidEnvChecksSkipped(t *testing.T) {
 		Config:      config.Project{},
 		SkipPublish: true,
 	}
-	testlib.AssertSkipped(t, Pipe{}.Run(ctx))
+	assert.NoError(t, Pipe{}.Run(ctx))
 }
 
 func TestInvalidEnvReleaseDisabled(t *testing.T) {
@@ -184,7 +183,7 @@ func TestInvalidEnvReleaseDisabled(t *testing.T) {
 			},
 		},
 	}
-	testlib.AssertSkipped(t, Pipe{}.Run(ctx))
+	assert.NoError(t, Pipe{}.Run(ctx))
 }
 
 func TestLoadEnv(t *testing.T) {