Fix brew update notifications #11024

BagToad · 2025-05-26T15:45:30Z

Description

This attempts to follow the suggestion in #10242 (comment) to add a build tag for controlling the updaterEnabled value instead of a statically linked build property. I'm proposing this build tag be passed in via an environment variable, which means Brew's build would be edited to look something like this:

    with_env(
      "GH_VERSION" => gh_version,
      "GO_LDFLAGS" => "-s -w",
      "GO_BUILDTAGS" => "updateable",
    ) do
      system "make", "bin/gh", "manpages"
    end

I decided to put it in the same ghcmd package so the var can remain un-exported. I needed to create two files so that we get one value when the build tag is set and another when it is not set; without the two files for the two cases, we get a compile error, as you'd expect.

Testing

Note

This testing was performed on a Mac.

After checking out this branch...

# Delete the update cache
rm ~/.local/state/gh/state.yml

# Cleanup just in case
make clean 

# Build it with an "old" version and the correct build tag
GH_VERSION="0.0.1" GO_BUILDTAGS="updateable" make

# Run a somewhat long running command:
./bin/gh pr list

Demo

❯ ./bin/gh pr list              

Showing 30 of 42 open pull requests in cli/cli

ID      TITLE                                 BRANCH                               CREATED AT            
<truncated for brevity>


A new release of gh is available: 0.0.1 → 2.73.0
https://github.com/cli/cli/releases/tag/v2.73.0

BagToad · 2025-05-26T15:55:04Z

internal/ghcmd/update_disabled.go

+// updaterEnabled is a statically linked build property set in gh formula within homebrew/homebrew-core
+// used to control whether users are notified of newer GitHub CLI releases.
+// This needs to be set to 'cli/cli' as it affects where update.CheckForUpdate() checks for releases.
+// It is unclear whether this means that only homebrew builds will check for updates or not.
+// Development builds leave this empty as impossible to determine if newer or not.
+// For more information, <https://github.com/Homebrew/homebrew-core/blob/master/Formula/g/gh.rb>.


TODO: update this comment after implementation feedback.

I'd definitely like to reconsider whether this has to be a string cli/cli rather than a bool. I cannot for the life of me figure out why this specific string is required. I also don't believe the comment (that I know is a copy paste), since the only real check is that it is non empty.

Hmm yeah good point. I don't know why it's a string 🤔

Ah it gets passed in as a repo arg:

cli/internal/ghcmd/cmd.go

Line 233 in 3d27e61

return update.CheckForUpdate(ctx, httpClient, stateFilePath, updaterEnabled, currentVersion)

cli/internal/update/update.go

Line 90 in 3d27e61

func CheckForUpdate(ctx context.Context, client *http.Client, stateFilePath, repo, currentVersion string) (*ReleaseInfo, error) {

Which is eventually used in a request here:

cli/internal/update/update.go

Line 114 in 3d27e61

req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("https://api.github.com/repos/%s/releases/latest", repo), nil)

It was originally a boolean and it later changed to the repo in this commit: ba0a441

Unfortunately, it doesn't link to any issue explaining why this is desirable. I don't feel strongly if we want to leave it as-is, but I think a comment linking to these historical commits with an explanation would be valuable.

Also FWIW, here is the issue which disabled this functionality for our own packaged builds: 2097899 (PR: #6977)

I'm uncertain whether RPM and Debian packages have a way to detect updated packages on https://cli.github.com/ versus the logic around brew bumps, avoiding new releases having people jump on updates but nothing is there yet

I don't understand what you're trying to express here, with regards specifying the repo as the value of updaterEnabled.

My only strong opinion is we treat this as a Chesterton's Fence exercise, avoiding changing this until we have a better understanding of the need and the problem it creates for us.

I don't mind this, as long as the fact we don't know and people should make their own decision in future is documented beside the code.

I'll definitely be updating the comments with details about what has been found here :)

I'm uncertain whether RPM and Debian packages have a way to detect updated packages on https://cli.github.com/ versus the logic around brew bumps, avoiding new releases having people jump on updates but nothing is there yet

I don't understand what you're trying to express here, with regards specifying the repo as the value of updaterEnabled.

Sorry, I was contemplating why only Homebrew builds enable gh release checking while our RPM and Debian packages disable that, not whether the variable should be a string vs boolean. My preference would be for any gh user to be notified about new releases.

Expand for digging into origins of gh update behavior

I think the following comments on #166 are the closest to explaining the original thought:

Add an easier upgrade command #166 (comment)

This issue just describes a potential idea for gh, but we haven't fleshed out how this should work and therefore we haven't begun development.

Many people already install GitHub CLI via their package manager, such as Homebrew on macOS/Linux, scoop/Chocolatey on Windows, and various package managers on *nix platforms. For people who have installed gh via their package manager, should gh provide an upgrade command, and if so, what should that command do? How do we detect these cases?

Personally, I think it would be risky to try to side-step the package manager by providing an upgrade command, since a part of the reason why people use package managers is having a degree of control and security over their system.

Keeping all this in mind, for which populations should we then provide a gh upgrade command and what should that command do when executed?

Add an easier upgrade command #166 (comment)

Thanks @mislav! I agree with everything you said. My inclination is that the upgrading should be handled at the package manager level, and it seems (though I don't have data on this) that the vast majority of people using gh so far are installing it via a package manager, and therefore upgrading is handled there. I'm curious what I might be missing though.

Add an easier upgrade command #166 (comment)

This is my inclination as well, but I'm wondering whether others (including our users) have a different idea of what their upgrade experience should feel like and should it be handled by the gh executable itself.

For example: right now, manually downloading .deb/.rpm packages from the Releases page and installing them directly on every update is not a great upgrade experience (in my view) and we could potentially solve it by maintaining our own package repository. However, that approach still relies on 3rd-party package managers and doesn't require us to make any additions to gh command functionality.

Also, worth noting is that the preferred upgrade method via Homebrew is now brew upgrade gh and not brew upgrade gh --fetch-HEAD as it was in the time when we were still prototyping. This should make it easier for Homebrew users to upgrade, unless they want to keep building the latest version from master.

Sorry, I was contemplating why only Homebrew builds enable gh release checking while our RPM and Debian packages disable that, not whether the variable should be a string vs boolean

Ack.

#6977 for further reading as to why it was turned off, when it was initially turned on.

#6977 for further reading as to why it was turned off, when it was initially turned on.

Thank you!!!

Truthfully, I don't know how readily other package managers actually notify users when new packages are available. I don't see dropping the notice as that detracting or problematic that we disable it all together.

Copilot

Pull Request Overview

This PR introduces a build‐tag driven mechanism for toggling updaterEnabled instead of using a fixed build property, allowing Homebrew to pass a tag to enable update notifications.

Pass a custom GO_BUILDTAGS env var into go build as -tags in script/build.go
Provide two build‐tag files (update_enabled.go / update_disabled.go) that set updaterEnabled appropriately
Remove the old hardcoded updaterEnabled var in cmd.go

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
script/build.go	Reads `GO_BUILDTAGS` and injects it as a `-tags` flag to `go build`
internal/ghcmd/update_enabled.go	Defines `updaterEnabled = "cli/cli"` under the `updateable` build tag
internal/ghcmd/update_disabled.go	Defines `updaterEnabled = ""` when `updateable` tag is not set
internal/ghcmd/cmd.go	Removes the previous default `updaterEnabled` declaration

Comments suppressed due to low confidence (2)

script/build.go:56

[nitpick] Consider renaming the environment variable to GH_BUILDTAGS to avoid conflict with standard Go environment variables (e.g., GOOS, GOARCH).

buildTags, _ := os.LookupEnv("GO_BUILDTAGS")

internal/ghcmd/update_enabled.go:1

[nitpick] The build tag updateable is unconventional; consider using the standard spelling updatable for clarity and consistency.

//go:build updateable

script/build.go

internal/ghcmd/update_enabled.go

Co-authored-by: Copilot <[email protected]>

andyfeller

Thanks, @BagToad, I can't wait for update notices to be restored! Hopefully we hear back about any situations where forks depend on this and we instruct them appropriately. Should basically be a new linker flag in the build process, not the end of the world.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.73.0` -> `v2.74.1` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.74.1`](https://github.com/cli/cli/releases/tag/v2.74.1): GitHub CLI 2.74.1 [Compare Source](cli/cli@v2.74.0...v2.74.1) #### What's Changed - Document support for `@copilot` in `gh [pr|issue] edit --add-assignee` and `--remove-assignee` by [@timrogers](https://github.com/timrogers) in cli/cli#11056 - Fix pr edit when URL is provided by [@williammartin](https://github.com/williammartin) in cli/cli#11057 - Fix const in MR finder tests by [@babakks](https://github.com/babakks) in cli/cli#11091 **Full Changelog**: cli/cli@v2.74.0...v2.74.1 ### [`v2.74.0`](https://github.com/cli/cli/releases/tag/v2.74.0): GitHub CLI 2.74.0 [Compare Source](cli/cli@v2.73.0...v2.74.0) #### Security A security vulnerability has been identified in a core `gh` dependency, `go-gh`, where an attacker-controlled GitHub Enterprise Server could result in executing arbitrary commands on a user's machine by replacing HTTP URLs provided by GitHub with local file paths for browsing. This issue is addressed in this `gh` release by updating `go-gh` to a fixed version. For more information, see GHSA-g9f5-x53j-h563 #### What's changed ##### ✨ Features - Add `preview prompter` command by [@BagToad](https://github.com/BagToad) in cli/cli#10745 - \[gh run watch] Support `--compact` flag by [@iamazeem](https://github.com/iamazeem) in cli/cli#10629 - Fix brew update notifications by [@BagToad](https://github.com/BagToad) in cli/cli#11024 ##### 🐛 Fixes - Revert "\[gh config] Escape pipe symbol in Long desc for website manual" by [@BagToad](https://github.com/BagToad) in cli/cli#11004 - Fix formatting in allowed values for `gh config --help` by [@BagToad](https://github.com/BagToad) in cli/cli#11003 - fix: `gh gist edit` panic when no file in a gist by [@phanen](https://github.com/phanen) in cli/cli#10627 - Add retry logic when fetching TUF content in `gh attestation` commands by [@malancas](https://github.com/malancas) in cli/cli#10943 ##### 📚 Docs & Chores - Update README.md by [@irhdab](https://github.com/irhdab) in cli/cli#11022 - Add tests for `RenderJobs` and `RenderJobsCompact` by [@babakks](https://github.com/babakks) in cli/cli#11013 - Add example usage of `--head` option to `pr list` docs by [@babakks](https://github.com/babakks) in cli/cli#10979 - Mention `pr create` will print the created MR's URL by [@babakks](https://github.com/babakks) in cli/cli#10980 - Add Digest to ReleaseAsset struct by [@bdehamer](https://github.com/bdehamer) in cli/cli#11030 #####

Dependencies - Bump `go-gh` to v2.12.1 by [@BagToad](https://github.com/BagToad) in cli/cli#11043 - chore(deps): bump github.com/gabriel-vasile/mimetype from 1.4.8 to 1.4.9 by [@dependabot](https://github.com/dependabot) in cli/cli#10825 - Update sigstore-go dependency to v1.0.0 by [@malancas](https://github.com/malancas) in cli/cli#11028 - chore(deps): bump github.com/sigstore/protobuf-specs from 0.4.1 to 0.4.2 by [@dependabot](https://github.com/dependabot) in cli/cli#10999 - chore(deps): bump github.com/yuin/goldmark from 1.7.8 to 1.7.12 by [@dependabot](https://github.com/dependabot) in cli/cli#11032 #### New Contributors - [@irhdab](https://github.com/irhdab) made their first contribution in cli/cli#11022 - [@phanen](https://github.com/phanen) made their first contribution in cli/cli#10627 **Full Changelog**: cli/cli@v2.73.0...v2.74.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

feat(update): add updateable build tag for update notifications

22504bf

BagToad temporarily deployed to cli-automation May 26, 2025 15:45 — with GitHub Actions Inactive

BagToad marked this pull request as ready for review May 26, 2025 15:54

Copilot AI review requested due to automatic review settings May 26, 2025 15:54

BagToad requested a review from a team as a code owner May 26, 2025 15:54

BagToad requested a review from andyfeller May 26, 2025 15:54

BagToad temporarily deployed to cli-automation May 26, 2025 15:54 — with GitHub Actions Inactive

cliAutomation assigned BagToad May 26, 2025

BagToad commented May 26, 2025

View reviewed changes

BagToad requested a review from williammartin May 26, 2025 15:55

Copilot AI reviewed May 26, 2025

View reviewed changes

script/build.go Outdated Show resolved Hide resolved

internal/ghcmd/update_enabled.go Outdated Show resolved Hide resolved

BagToad and others added 2 commits May 27, 2025 10:11

Refactor build tag logic

3e3b9ad

Co-authored-by: Copilot <[email protected]>

doc(update): improve comments on updaterEnabled

dd4095d

andyfeller approved these changes May 27, 2025

View reviewed changes

BagToad merged commit 98b844a into trunk May 27, 2025
16 checks passed

BagToad deleted the kw/cli-10242-fix-updater-notifications branch May 27, 2025 20:33

BagToad mentioned this pull request May 27, 2025

gh: use new updateable build tag Homebrew/homebrew-core#224900

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix brew update notifications #11024

Fix brew update notifications #11024

Uh oh!

BagToad commented May 26, 2025 •

edited

Loading

Uh oh!

BagToad May 26, 2025

Uh oh!

williammartin May 26, 2025

Uh oh!

BagToad May 26, 2025

Uh oh!

BagToad May 26, 2025

Uh oh!

williammartin May 27, 2025

Uh oh!

williammartin May 27, 2025 •

edited

Loading

Uh oh!

BagToad May 27, 2025

Uh oh!

andyfeller May 27, 2025

Uh oh!

williammartin May 27, 2025

Uh oh!

andyfeller May 27, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

andyfeller left a comment

Uh oh!

Uh oh!

Uh oh!

Fix brew update notifications #11024

Fix brew update notifications #11024

Uh oh!

Conversation

BagToad commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Demo

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

williammartin May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andyfeller May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

andyfeller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BagToad commented May 26, 2025 •

edited

Loading

williammartin May 27, 2025 •

edited

Loading

andyfeller May 27, 2025 •

edited

Loading