Core GitHub CLI update checker disabled due to relocation of package build variable

Describe the bug

The core update checking logic has not reported updates to users since v2.59.0 release because the updaterEnabled package variable was relocated. This package variable is set in our Homebrew formula, however the main import path no longer applies, requiring setting the new, fully qualified Go package instead:

    with_env(
      "GH_VERSION" => gh_version,
      "GO_LDFLAGS" => "-s -w -X main.updaterEnabled=cli/cli",
    ) do
      system "make", "bin/gh", "manpages"
    end
    bin.install "bin/gh"

whereas now this would be:

    with_env(
      "GH_VERSION" => gh_version,
      "GO_LDFLAGS" => "-s -w -X github.com/cli/cli/v2/internal/ghcmd.updaterEnabled=cli/cli",
    ) do
      system "make", "bin/gh", "manpages"
    end
    bin.install "bin/gh"

The main package appears to be special from what I can gather in the depths of Go cmd/link source code, however this behavior isn't well documented.

Relates #9745

Steps to reproduce the behavior

Clone Homebrew formula repository
```
gh repo clone homebrew/homebrew-core
```

Install 2.58.0 version via Homebrew

brew remove gh
git -C homebrew-core checkout 06ce7bd256e44f9382de1b02e2a12662be994d61
brew install ./homebrew-core/Formula/g/gh.rb
gh version

confirming:

gh version 2.58.0 (2024-10-01)
https://github.com/cli/cli/releases/tag/v2.58.0

Clean up older state file and run gh command to generate update notice

rm ~/.local/state/gh/state.yml
gh pr list --state all --limit 1000 --repo cli/cli

confirming:

A new release of gh is available: 2.58.0 → 2.65.0
To upgrade, run: brew upgrade gh
https://github.com/cli/cli/releases/tag/v2.65.0

Install 2.59.0 version via Homebrew

brew remove gh
git -C homebrew-core checkout 628e97421ed16b9502409891c38175167c066b6a
brew install ./homebrew-core/Formula/g/gh.rb
gh version

confirming:

gh version 2.59.0 (2024-10-15)
https://github.com/cli/cli/releases/tag/v2.59.0

Clean up older state file and run gh command but no update notice

rm ~/.local/state/gh/state.yml
gh pr list --state all --limit 1000 --repo cli/cli

Expected vs actual behavior

brew based builds should notify the user on stderr if a new release is present.

Logs

Paste the activity from your command line. Redact if needed.

Do we want tight coupling of the brew formula with our internal package structure?

@cli/code-reviewers : At first, I thought this was just a simple PR on the formula when I realized we might not want to deal with the tight coupling. So I wanted to field others feedback whether we just relocate the updaterEnabled package variable back into the main package.

Appreciate insight and feedback! ❤

So I wanted to field others feedback whether we just relocate the updaterEnabled package variable back into the main package.

I don't have a strong opinion, but It makes more sense to me for us to handle this on our end by relocating the updaterEnabled variable back tomain.

Acceptance Criteria

Acceptance criteria to satisfy triage requirements

brew based builds notify the user on stderr if a new release is present.

@andyfeller

@andyfeller I made a draft PR for this so I could see what this might look like if we were to handle this on our end by moving the variable back to the main package: #10492

Note: I haven't tested this.

Even though it doesn't look or feel like a great solution, I think I am still personally in favour doing this on our end rather than adjusting how Brew and any others build gh. Also, this was originally broken because of the acceptance testing changes we made; given how we're still developing an understanding of how acceptance tests are to be used, I'm not sure we want to bank on the variable being in the ghcmd package command forever. It might make more sense to commit to having this in our main package, avoiding future churn for Brew and others. 🤷

What do you think?

@williammartin

@williammartin said:

Actually what do you think about this?
//go:build !homebrew
package updater

var Enabled = false
//go:build homebrew
package updater

var Enabled = true
Then update the homebrew build to go build --tags homebrew

This would uncouple them from the specific path. We could generalise this to --tags updatable, though I'm not sure if this is used for anything other than homebrew.

@andyfeller @babakks can you leave your thoughts on this approach too? Before I put in the work to get that setup, I want us to agree on the approach 😀

I'm also concerned about how we coordinate a change like that with Brew 🤔 But maybe that's not a problem.

From IPM:

Wondering whether/how we should externally notify about this fix

Navigation Menu

Repository navigation

Core GitHub CLI update checker disabled due to relocation of package build variable #10242

Describe the bug

Steps to reproduce the behavior

Expected vs actual behavior

Logs

Acceptance Criteria

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Core GitHub CLI update checker disabled due to relocation of package build variable #10242

Description

Describe the bug

Steps to reproduce the behavior

Expected vs actual behavior

Logs

Activity

andyfeller commented on Jan 14, 2025

BagToad commented on Jan 16, 2025

BagToad commented on Jan 16, 2025

Acceptance Criteria

BagToad commented on Feb 23, 2025

BagToad commented on Apr 22, 2025

RyanHecht commented on Apr 23, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions