-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Closed
Labels
bugSomething isn't workingSomething isn't workinggh-extensionrelating to the gh extension commandrelating to the gh extension command
Description
Describe the bug
After #9934 was merged, @williammartin called out a problem where the logic handling extension updates is blocking, which will cause the user to wait an indeterminate amount of time for gh to check if a new version is available:
Lines 53 to 70 in 112552f
| // PostRun handles communicating extension release information if found | |
| PostRun: func(c *cobra.Command, args []string) { | |
| releaseInfo := <-updateMessageChan | |
| if releaseInfo != nil { | |
| stderr := io.ErrOut | |
| fmt.Fprintf(stderr, "\n\n%s %s → %s\n", | |
| cs.Yellowf("A new release of %s is available:", ext.Name()), | |
| cs.Cyan(strings.TrimPrefix(ext.CurrentVersion(), "v")), | |
| cs.Cyan(strings.TrimPrefix(releaseInfo.Version, "v"))) | |
| if ext.IsPinned() { | |
| fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s --force\n", ext.Name()) | |
| } else { | |
| fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s\n", ext.Name()) | |
| } | |
| fmt.Fprintf(stderr, "%s\n\n", | |
| cs.Yellow(releaseInfo.URL)) | |
| } | |
| }, |
This was a mistake on the author's part as the update checking logic should be non-blocking like it is with core gh update checking:
Lines 174 to 191 in 112552f
| updateCancel() // if the update checker hasn't completed by now, abort it | |
| newRelease := <-updateMessageChan | |
| if newRelease != nil { | |
| isHomebrew := isUnderHomebrew(cmdFactory.Executable()) | |
| if isHomebrew && isRecentRelease(newRelease.PublishedAt) { | |
| // do not notify Homebrew users before the version bump had a chance to get merged into homebrew-core | |
| return exitOK | |
| } | |
| fmt.Fprintf(stderr, "\n\n%s %s → %s\n", | |
| ansi.Color("A new release of gh is available:", "yellow"), | |
| ansi.Color(strings.TrimPrefix(buildVersion, "v"), "cyan"), | |
| ansi.Color(strings.TrimPrefix(newRelease.Version, "v"), "cyan")) | |
| if isHomebrew { | |
| fmt.Fprintf(stderr, "To upgrade, run: %s\n", "brew upgrade gh") | |
| } | |
| fmt.Fprintf(stderr, "%s\n\n", | |
| ansi.Color(newRelease.URL, "yellow")) | |
| } |
Steps to reproduce the behavior
-
Create simple extension for testing
gh ext create gh-sleep cd gh-sleep gh repo create --push --private --source . gh ext install andyfeller/gh-sleep
-
Artificially extend extension update check behavior
diff --git a/pkg/cmd/root/extension.go b/pkg/cmd/root/extension.go index 7f2325e1..95d4669d 100644 --- a/pkg/cmd/root/extension.go +++ b/pkg/cmd/root/extension.go @@ -32,6 +32,9 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex // PreRun handles looking up whether extension has a latest version only when the command is ran. PreRun: func(c *cobra.Command, args []string) { go func() { + fmt.Fprintf(io.ErrOut, "Artifically delaying update check logic, sleeping for 3 minutes") + time.Sleep(3 * time.Minute) + fmt.Fprintf(io.ErrOut, "Artifically delay up!") releaseInfo, err := checkExtensionReleaseInfo(em, ext) if err != nil && hasDebug { fmt.Fprintf(io.ErrOut, "warning: checking for update failed: %v", err) -
Build and run extension, confirming blocking behavior
make time ./bin/gh sleepresulting in:
Artifically delaying update check logic, sleeping for 3 minutes Hello gh-sleep! Artifically delay up! ./bin/gh sleep 0.05s user 0.02s system 0% cpu 3:00.06 total
Expected vs actual behavior
Again, extension update checking logic should be non-blocking.
Logs
Paste the activity from your command line. Redact if needed.
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't workinggh-extensionrelating to the gh extension commandrelating to the gh extension command