From 49fd7a57564d7ed2525eaee3284959e3672952f7 Mon Sep 17 00:00:00 2001 From: Azeem Sajid Date: Sat, 8 Feb 2025 18:20:43 +0500 Subject: [PATCH 1/4] [gh release create] Fail when there are no new commits since the most recent release --- pkg/cmd/release/create/create.go | 12 +++ pkg/cmd/release/create/create_test.go | 110 ++++++++++++++++++++++++++ pkg/cmd/release/create/http.go | 46 +++++++++++ pkg/cmd/release/shared/fetch.go | 8 +- 4 files changed, 172 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 8e77546d0..dddd3066f 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -60,6 +60,7 @@ type CreateOptions struct { NotesStartTag string VerifyTag bool NotesFromTag bool + OnlyNew bool } func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { @@ -194,6 +195,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmdutil.NilBoolFlag(cmd, &opts.IsLatest, "latest", "", "Mark this release as \"Latest\" (default [automatic based on date and version]). --latest=false to explicitly NOT set as latest") cmd.Flags().BoolVarP(&opts.VerifyTag, "verify-tag", "", false, "Abort in case the git tag doesn't already exist in the remote repository") cmd.Flags().BoolVarP(&opts.NotesFromTag, "notes-from-tag", "", false, "Automatically generate notes from annotated tag") + cmd.Flags().BoolVar(&opts.OnlyNew, "only-new", false, "Create release only if there are new commits since the last release") _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "target") @@ -211,6 +213,16 @@ func createRun(opts *CreateOptions) error { return err } + if opts.OnlyNew { + isNew, err := isNewRelease(httpClient, baseRepo) + if err != nil { + return err + } + if !isNew { + return fmt.Errorf("no new commits since the last release") + } + } + var existingTag bool if opts.TagName == "" { tags, err := getTags(httpClient, baseRepo, 5) diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index c9e9a8c8a..252d28cbc 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -63,6 +63,7 @@ func Test_NewCmdCreate(t *testing.T) { Concurrency: 5, Assets: []*shared.AssetForUpload(nil), VerifyTag: false, + OnlyNew: false, }, }, { @@ -87,6 +88,7 @@ func Test_NewCmdCreate(t *testing.T) { Concurrency: 5, Assets: []*shared.AssetForUpload(nil), VerifyTag: false, + OnlyNew: false, }, }, { @@ -347,6 +349,19 @@ func Test_NewCmdCreate(t *testing.T) { isTTY: false, wantErr: "using `--notes-from-tag` with `--generate-notes` or `--notes-start-tag` is not supported", }, + { + name: "with --only-new", + args: "v1.2.3 --only-new", + isTTY: false, + want: CreateOptions{ + TagName: "v1.2.3", + BodyProvided: false, + Concurrency: 5, + Assets: []*shared.AssetForUpload(nil), + NotesFromTag: false, + OnlyNew: true, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -402,6 +417,7 @@ func Test_NewCmdCreate(t *testing.T) { assert.Equal(t, tt.want.IsLatest, opts.IsLatest) assert.Equal(t, tt.want.VerifyTag, opts.VerifyTag) assert.Equal(t, tt.want.NotesFromTag, opts.NotesFromTag) + assert.Equal(t, tt.want.OnlyNew, opts.OnlyNew) require.Equal(t, len(tt.want.Assets), len(opts.Assets)) for i := range tt.want.Assets { @@ -460,6 +476,100 @@ func Test_createRun(t *testing.T) { wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", wantStderr: ``, }, + { + name: "create a release if there are new commits and the last release does not exist", + isTTY: true, + opts: CreateOptions{ + TagName: "v1.2.3", + Name: "The Big 1.2", + Body: "* Fixed bugs", + BodyProvided: true, + Target: "", + OnlyNew: true, + }, + runStubs: defaultRunStubs, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/latest"), httpmock.StatusStringResponse(404, `{ + "message": "Not Found", + "documentation_url": "https://docs.github.com/rest/releases/releases#get-the-latest-release", + "status": "404" + }`)) + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "name": "The Big 1.2", + "body": "* Fixed bugs", + "draft": false, + "prerelease": false, + }, params) + })) + }, + wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", + wantStderr: ``, + }, + { + name: "create a release if there are new commits and the last release exists", + isTTY: true, + opts: CreateOptions{ + TagName: "v1.2.3", + Name: "The Big 1.2", + Body: "* Fixed bugs", + BodyProvided: true, + Target: "", + OnlyNew: true, + }, + runStubs: defaultRunStubs, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/latest"), httpmock.StatusStringResponse(200, `{ + "tag_name": "v1.2.2" + }`)) + reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/compare/v1.2.2...HEAD"), httpmock.StatusStringResponse(200, `{ + "status": "ahead" + }`)) + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "name": "The Big 1.2", + "body": "* Fixed bugs", + "draft": false, + "prerelease": false, + }, params) + })) + }, + wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", + wantStderr: ``, + }, + { + name: "create a release if there are no new commits but the last release exists", + isTTY: true, + opts: CreateOptions{ + TagName: "v1.2.3", + Name: "The Big 1.2", + Body: "* Fixed bugs", + BodyProvided: true, + Target: "", + OnlyNew: true, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/latest"), httpmock.StatusStringResponse(200, `{ + "tag_name": "v1.2.2" + }`)) + reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/compare/v1.2.2...HEAD"), httpmock.StatusStringResponse(200, `{ + "status": "identical" + }`)) + }, + wantErr: "no new commits since the last release", + wantStdout: "", + wantStderr: ``, + }, { name: "with discussion category", isTTY: true, diff --git a/pkg/cmd/release/create/http.go b/pkg/cmd/release/create/http.go index 3bb55f39e..0db8b1c0a 100644 --- a/pkg/cmd/release/create/http.go +++ b/pkg/cmd/release/create/http.go @@ -2,6 +2,7 @@ package create import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -299,3 +300,48 @@ func tokenHasWorkflowScope(resp *http.Response) bool { return slices.Contains(strings.Split(scopes, ","), "workflow") } + +// isNewRelease checks if there are new commits since the latest release. +func isNewRelease(httpClient *http.Client, repo ghrepo.Interface) (bool, error) { + ctx := context.Background() + release, err := shared.FetchLatestRelease(ctx, httpClient, repo) + if err != nil { + if errors.Is(err, shared.ErrReleaseNotFound) { + return true, nil + } else { + return false, err + } + } + + tagName := release.TagName + path := fmt.Sprintf("repos/%s/%s/compare/%s...HEAD?per_page=1", repo.RepoOwner(), repo.RepoName(), tagName) + url := ghinstance.RESTPrefix(repo.RepoHost()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return false, err + } + + req.Header.Set("Content-Type", "application/json; charset=utf-8") + + resp, err := httpClient.Do(req) + if err != nil { + return false, err + } + defer resp.Body.Close() + + if resp.StatusCode != 200 { + return false, api.HandleHTTPError(resp) + } + + type comparisonStatus struct { + Status string `json:"status"` + } + + var cmpStatus comparisonStatus + if err := json.NewDecoder(resp.Body).Decode(&cmpStatus); err != nil { + return false, err + } + + isNew := cmpStatus.Status == "ahead" + return isNew, nil +} diff --git a/pkg/cmd/release/shared/fetch.go b/pkg/cmd/release/shared/fetch.go index c9cf7a6a4..8db7e502a 100644 --- a/pkg/cmd/release/shared/fetch.go +++ b/pkg/cmd/release/shared/fetch.go @@ -124,7 +124,7 @@ func (rel *Release) ExportData(fields []string) map[string]interface{} { return data } -var errNotFound = errors.New("release not found") +var ErrReleaseNotFound = errors.New("release not found") type fetchResult struct { release *Release @@ -150,7 +150,7 @@ func FetchRelease(ctx context.Context, httpClient *http.Client, repo ghrepo.Inte }() res := <-results - if errors.Is(res.error, errNotFound) { + if errors.Is(res.error, ErrReleaseNotFound) { res = <-results cancel() // satisfy the linter even though no goroutines are running anymore } else { @@ -190,7 +190,7 @@ func fetchDraftRelease(ctx context.Context, httpClient *http.Client, repo ghrepo } if query.Repository.Release == nil || !query.Repository.Release.IsDraft { - return nil, errNotFound + return nil, ErrReleaseNotFound } // Then, use REST to get information about the draft release. In theory, we could have fetched @@ -213,7 +213,7 @@ func fetchReleasePath(ctx context.Context, httpClient *http.Client, host string, if resp.StatusCode == 404 { _, _ = io.Copy(io.Discard, resp.Body) - return nil, errNotFound + return nil, ErrReleaseNotFound } else if resp.StatusCode > 299 { return nil, api.HandleHTTPError(resp) } From 72364562dd90b8b905bd3c5a3187affc61a9cd27 Mon Sep 17 00:00:00 2001 From: Azeem Sajid Date: Tue, 18 Feb 2025 21:49:38 +0500 Subject: [PATCH 2/4] Use API REST Client --- pkg/cmd/release/create/http.go | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/pkg/cmd/release/create/http.go b/pkg/cmd/release/create/http.go index 0db8b1c0a..4311a3896 100644 --- a/pkg/cmd/release/create/http.go +++ b/pkg/cmd/release/create/http.go @@ -315,33 +315,16 @@ func isNewRelease(httpClient *http.Client, repo ghrepo.Interface) (bool, error) tagName := release.TagName path := fmt.Sprintf("repos/%s/%s/compare/%s...HEAD?per_page=1", repo.RepoOwner(), repo.RepoName(), tagName) - url := ghinstance.RESTPrefix(repo.RepoHost()) + path - req, err := http.NewRequest("GET", url, nil) - if err != nil { - return false, err - } - req.Header.Set("Content-Type", "application/json; charset=utf-8") - - resp, err := httpClient.Do(req) - if err != nil { - return false, err - } - defer resp.Body.Close() - - if resp.StatusCode != 200 { - return false, api.HandleHTTPError(resp) - } - - type comparisonStatus struct { + var comparisonStatus struct { Status string `json:"status"` } - var cmpStatus comparisonStatus - if err := json.NewDecoder(resp.Body).Decode(&cmpStatus); err != nil { + apiClient := api.NewClientFromHTTP(httpClient) + if err := apiClient.REST(repo.RepoHost(), "GET", path, nil, &comparisonStatus); err != nil { return false, err } - isNew := cmpStatus.Status == "ahead" + isNew := comparisonStatus.Status == "ahead" return isNew, nil } From 960017a4f11adb5b3db19b2df4aa67550c3c4224 Mon Sep 17 00:00:00 2001 From: Azeem Sajid Date: Wed, 19 Feb 2025 15:20:17 +0500 Subject: [PATCH 3/4] Rename flag to `--fail-on-no-commits` --- pkg/cmd/release/create/create.go | 14 +++- pkg/cmd/release/create/create_test.go | 102 +++++++++++++------------- 2 files changed, 62 insertions(+), 54 deletions(-) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index dddd3066f..0ddaaa166 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -60,7 +60,7 @@ type CreateOptions struct { NotesStartTag string VerifyTag bool NotesFromTag bool - OnlyNew bool + FailOnNoCommits bool } func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { @@ -100,6 +100,11 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co When using automatically generated release notes, a release title will also be automatically generated unless a title was explicitly passed. Additional release notes can be prepended to automatically generated notes by using the %[1]s--notes%[1]s flag. + + By default, the release is created even if there are no new commits since the last release. + This may result in the same or duplicate release which may not be desirable in some cases. + Use %[1]s--fail-on-no-commits%[1]s to fail if no new commits are available. This flag has no + effect if there are no existing releases or this is the very first release. `, "`"), Example: heredoc.Doc(` Interactively create a release @@ -131,6 +136,9 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co Create a release and start a discussion $ gh release create v1.2.3 --discussion-category "General" + + # Create a release only if there are new commits available since the last release + $ gh release create v1.2.3 --fail-on-no-commits `), Aliases: []string{"new"}, RunE: func(cmd *cobra.Command, args []string) error { @@ -195,7 +203,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmdutil.NilBoolFlag(cmd, &opts.IsLatest, "latest", "", "Mark this release as \"Latest\" (default [automatic based on date and version]). --latest=false to explicitly NOT set as latest") cmd.Flags().BoolVarP(&opts.VerifyTag, "verify-tag", "", false, "Abort in case the git tag doesn't already exist in the remote repository") cmd.Flags().BoolVarP(&opts.NotesFromTag, "notes-from-tag", "", false, "Automatically generate notes from annotated tag") - cmd.Flags().BoolVar(&opts.OnlyNew, "only-new", false, "Create release only if there are new commits since the last release") + cmd.Flags().BoolVar(&opts.FailOnNoCommits, "fail-on-no-commits", false, "Fail if there are no commits since the last release (no impact on the first release)") _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "target") @@ -213,7 +221,7 @@ func createRun(opts *CreateOptions) error { return err } - if opts.OnlyNew { + if opts.FailOnNoCommits { isNew, err := isNewRelease(httpClient, baseRepo) if err != nil { return err diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index 252d28cbc..a6e6cd7c6 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -52,18 +52,18 @@ func Test_NewCmdCreate(t *testing.T) { args: "", isTTY: true, want: CreateOptions{ - TagName: "", - Target: "", - Name: "", - Body: "", - BodyProvided: false, - Draft: false, - Prerelease: false, - RepoOverride: "", - Concurrency: 5, - Assets: []*shared.AssetForUpload(nil), - VerifyTag: false, - OnlyNew: false, + TagName: "", + Target: "", + Name: "", + Body: "", + BodyProvided: false, + Draft: false, + Prerelease: false, + RepoOverride: "", + Concurrency: 5, + Assets: []*shared.AssetForUpload(nil), + VerifyTag: false, + FailOnNoCommits: false, }, }, { @@ -77,18 +77,18 @@ func Test_NewCmdCreate(t *testing.T) { args: "v1.2.3", isTTY: true, want: CreateOptions{ - TagName: "v1.2.3", - Target: "", - Name: "", - Body: "", - BodyProvided: false, - Draft: false, - Prerelease: false, - RepoOverride: "", - Concurrency: 5, - Assets: []*shared.AssetForUpload(nil), - VerifyTag: false, - OnlyNew: false, + TagName: "v1.2.3", + Target: "", + Name: "", + Body: "", + BodyProvided: false, + Draft: false, + Prerelease: false, + RepoOverride: "", + Concurrency: 5, + Assets: []*shared.AssetForUpload(nil), + VerifyTag: false, + FailOnNoCommits: false, }, }, { @@ -350,16 +350,16 @@ func Test_NewCmdCreate(t *testing.T) { wantErr: "using `--notes-from-tag` with `--generate-notes` or `--notes-start-tag` is not supported", }, { - name: "with --only-new", - args: "v1.2.3 --only-new", + name: "with --fail-on-no-commits", + args: "v1.2.3 --fail-on-no-commits", isTTY: false, want: CreateOptions{ - TagName: "v1.2.3", - BodyProvided: false, - Concurrency: 5, - Assets: []*shared.AssetForUpload(nil), - NotesFromTag: false, - OnlyNew: true, + TagName: "v1.2.3", + BodyProvided: false, + Concurrency: 5, + Assets: []*shared.AssetForUpload(nil), + NotesFromTag: false, + FailOnNoCommits: true, }, }, } @@ -417,7 +417,7 @@ func Test_NewCmdCreate(t *testing.T) { assert.Equal(t, tt.want.IsLatest, opts.IsLatest) assert.Equal(t, tt.want.VerifyTag, opts.VerifyTag) assert.Equal(t, tt.want.NotesFromTag, opts.NotesFromTag) - assert.Equal(t, tt.want.OnlyNew, opts.OnlyNew) + assert.Equal(t, tt.want.FailOnNoCommits, opts.FailOnNoCommits) require.Equal(t, len(tt.want.Assets), len(opts.Assets)) for i := range tt.want.Assets { @@ -480,12 +480,12 @@ func Test_createRun(t *testing.T) { name: "create a release if there are new commits and the last release does not exist", isTTY: true, opts: CreateOptions{ - TagName: "v1.2.3", - Name: "The Big 1.2", - Body: "* Fixed bugs", - BodyProvided: true, - Target: "", - OnlyNew: true, + TagName: "v1.2.3", + Name: "The Big 1.2", + Body: "* Fixed bugs", + BodyProvided: true, + Target: "", + FailOnNoCommits: true, }, runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { @@ -515,12 +515,12 @@ func Test_createRun(t *testing.T) { name: "create a release if there are new commits and the last release exists", isTTY: true, opts: CreateOptions{ - TagName: "v1.2.3", - Name: "The Big 1.2", - Body: "* Fixed bugs", - BodyProvided: true, - Target: "", - OnlyNew: true, + TagName: "v1.2.3", + Name: "The Big 1.2", + Body: "* Fixed bugs", + BodyProvided: true, + Target: "", + FailOnNoCommits: true, }, runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { @@ -551,12 +551,12 @@ func Test_createRun(t *testing.T) { name: "create a release if there are no new commits but the last release exists", isTTY: true, opts: CreateOptions{ - TagName: "v1.2.3", - Name: "The Big 1.2", - Body: "* Fixed bugs", - BodyProvided: true, - Target: "", - OnlyNew: true, + TagName: "v1.2.3", + Name: "The Big 1.2", + Body: "* Fixed bugs", + BodyProvided: true, + Target: "", + FailOnNoCommits: true, }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/latest"), httpmock.StatusStringResponse(200, `{ From 8202471910f7746cd871e019661083569354bf3a Mon Sep 17 00:00:00 2001 From: Azeem Sajid Date: Wed, 19 Feb 2025 16:03:24 +0500 Subject: [PATCH 4/4] Wrap error --- pkg/cmd/release/create/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 0ddaaa166..74880a9de 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -224,7 +224,7 @@ func createRun(opts *CreateOptions) error { if opts.FailOnNoCommits { isNew, err := isNewRelease(httpClient, baseRepo) if err != nil { - return err + return fmt.Errorf("failed to check whether there were new commits since last release: %v", err) } if !isNew { return fmt.Errorf("no new commits since the last release")