From 87d5fcb3630c46ab81ee2694c4af812e4c8a2502 Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Thu, 21 Apr 2022 11:21:09 -0500 Subject: [PATCH] Fix publish of checksum files When the checksum files are already exist in the bin directory, duplicate paths were recorded, and included in the gh release. This caused creating a release to fail. I have updated the logic to only get a list of unique files. I've also changed the release creation logic so that we aways create the release and upload the assets separately. The error handling in the upload command seems to be better than in the release create command. Signed-off-by: Carolyn Van Slyck --- releases/publish.go | 57 ++++++++++--------- releases/publish_test.go | 28 +++++++++ .../v1.2.3/mymixin-darwin-amd64.sha256sum | 1 + 3 files changed, 60 insertions(+), 26 deletions(-) create mode 100644 releases/testdata/mixins/v1.2.3/mymixin-darwin-amd64.sha256sum diff --git a/releases/publish.go b/releases/publish.go index ed766bb..bc8c938 100644 --- a/releases/publish.go +++ b/releases/publish.go @@ -183,39 +183,43 @@ func GeneratePluginFeed() error { // AddFilesToRelease uploads the files in the specified directory to a GitHub release. // If the release does not exist already, it will be created with empty release notes. func AddFilesToRelease(repo string, tag string, dir string) { + files, err := getReleaseAssets(dir) + mgx.Must(err) + + if !releaseExists(repo, tag) { + // Mark canary releases as a draft + draft := "" + if strings.HasPrefix(tag, "canary") { + draft = "-p" + } + + // Create the GH release + must.RunV("gh", "release", "create", "-R", repo, tag, "--notes=", draft) + } + + // Upload the release assets and overwrite existing assets + must.Command("gh", "release", "upload", "--clobber", "-R", repo, tag). + Args(files...).RunV() +} + +func getReleaseAssets(dir string) ([]string, error) { files := listFiles(dir) - checksumFiles := make([]string, len(files)) - for i, file := range files { + var releaseFiles []string + for _, file := range files { checksumFile, added := AddChecksumExt(file) if !added { - checksumFiles[i] = file + // This is a checksum file, skip continue } err := createChecksumFile(file, checksumFile) if err != nil { - mgx.Must(fmt.Errorf("failed to generate checksum file for asset %s: %w", file, err)) + return nil, fmt.Errorf("failed to generate checksum file for asset %s: %w", file, err) } - checksumFiles[i] = checksumFile - - } - - files = append(files, checksumFiles...) - - // Mark canary releases as a draft - draft := "" - if strings.HasPrefix(tag, "canary") { - draft = "-p" - } - - if releaseExists(repo, tag) { - must.Command("gh", "release", "upload", "--clobber", "-R", repo, tag). - Args(files...).RunV() - } else { - must.Command("gh", "release", "create", "-R", repo, "-t", tag, "--notes=", draft, tag). - CollapseArgs().Args(files...).RunV() + releaseFiles = append(releaseFiles, file, checksumFile) } + return releaseFiles, nil } func releaseExists(repo string, version string) bool { @@ -247,7 +251,7 @@ func AddChecksumExt(path string) (string, bool) { func GenerateChecksum(data io.Reader, path string) (string, error) { hash := sha256.New() if _, err := io.Copy(hash, data); err != nil { - return "", err + return "", fmt.Errorf("error generating checksum for %s: %w", path, err) } sum := hash.Sum(nil) @@ -263,7 +267,7 @@ func AppendDataPath(data []byte, path string) string { func createChecksumFile(contentPath string, checksumFile string) error { data, err := os.Open(contentPath) if err != nil { - return err + return fmt.Errorf("error reading release asset %s: %w", contentPath, err) } defer data.Close() @@ -274,10 +278,11 @@ func createChecksumFile(contentPath string, checksumFile string) error { f, err := os.Create(checksumFile) if err != nil { - return err + return fmt.Errorf("error creating checksum file %s: %w", checksumFile, err) } + defer f.Close() if _, err := f.WriteString(sum); err != nil { - return err + return fmt.Errorf("error writing checksum file %s: %w", checksumFile, err) } return nil diff --git a/releases/publish_test.go b/releases/publish_test.go index faced74..a519f48 100644 --- a/releases/publish_test.go +++ b/releases/publish_test.go @@ -3,6 +3,7 @@ package releases import ( "crypto/rand" "encoding/hex" + "github.com/carolynvs/magex/mgx" "io/ioutil" "os" "path/filepath" @@ -14,6 +15,33 @@ import ( "github.com/stretchr/testify/require" ) +func TestGetReleaseAssets(t *testing.T) { + tmp, err := ioutil.TempDir("", "magefiles") + require.NoError(t, err) + defer os.RemoveAll(tmp) + + mgx.Must(shx.Copy("testdata/mixins/v1.2.3/*", tmp, shx.CopyRecursive)) + + gotFiles, err := getReleaseAssets(tmp) + require.NoError(t, err) + + wantFiles := []string{ + filepath.Join(tmp, "mymixin-darwin-amd64"), + filepath.Join(tmp, "mymixin-darwin-amd64.sha256sum"), + filepath.Join(tmp, "mymixin-linux-amd64"), + filepath.Join(tmp, "mymixin-linux-amd64.sha256sum"), + filepath.Join(tmp, "mymixin-windows-amd64.exe"), + filepath.Join(tmp, "mymixin-windows-amd64.exe.sha256sum"), + } + assert.Equal(t, wantFiles, gotFiles) + + // Read the existing checksum file with stale contents, and ensure it was updated + gotChecksum, err := ioutil.ReadFile(filepath.Join(tmp, "mymixin-darwin-amd64.sha256sum")) + require.NoError(t, err) + wantCheckSum := "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 mymixin-darwin-amd64" + assert.Equal(t, wantCheckSum, string(gotChecksum)) +} + func TestAddChecksumExt(t *testing.T) { tests := []struct { input string diff --git a/releases/testdata/mixins/v1.2.3/mymixin-darwin-amd64.sha256sum b/releases/testdata/mixins/v1.2.3/mymixin-darwin-amd64.sha256sum new file mode 100644 index 0000000..ce7db21 --- /dev/null +++ b/releases/testdata/mixins/v1.2.3/mymixin-darwin-amd64.sha256sum @@ -0,0 +1 @@ +bad checksum file contents, should be overwritten \ No newline at end of file