Skip to content

Fix: return core update checker var to main pkg - #10492

Closed
BagToad wants to merge 1 commit into
trunkfrom
kw/cli-10242-fix-core-github-cli-update-checker-disabled
Closed

BagToad wants to merge 1 commit into
trunkfrom
kw/cli-10242-fix-core-github-cli-update-checker-disabled

Conversation

@BagToad

@BagToad BagToad commented Feb 23, 2025 •

Copy link
Copy Markdown
Member

Fixes #10242

@andyfeller

Copy link
Copy Markdown
Contributor

@BagToad : Thanks for opening up this draft! ✨

How I I typically test these types of changes

  1. Start with the go build command that make uses and include the additional static linking done in brew formula:

    In this case, we want to build an "older version" so we can see the core CLI update behavior.

    cli/cli ‹kw/cli-10242-fix-core-github-cli-update-checker-disabled›$ rm ~/.local/state/gh/state.yml
    
    cli/cli ‹kw/cli-10242-fix-core-github-cli-update-checker-disabled›$ go build -trimpath -ldflags "-X github.com/cli/cli/v2/internal/build.Date=2025-03-06 -X github.com/cli/cli/v2/internal/build.Version=v2.67.20250306 -X main.updaterEnabled=cli/cli" -o bin/gh ./cmd/gh
  2. Execute a command that gives gh long enough for the update checker to look for new releases, for example running an extension

    cli/cli ‹kw/cli-10242-fix-core-github-cli-update-checker-disabled›$ bin/gh copilot suggest
    
    Welcome to GitHub Copilot in the CLI!
    version 1.0.6 (2025-01-06)
    
    I'm powered by AI, so surprises and mistakes are possible. Make sure to verify any generated code or suggestions, and share feedback so that we can learn and improve. For more information, see https://gh.io/gh-copilot-transparency
    
    ? What kind of command can I help you with?
    > generic shell command
    
    ? What would you like the shell command to do? 
    > print hello world
    
    Suggestion:                                                                                                                                                                                                                             
                                                                                                                                                                                                                                           
      echo "Hello, World!"                                                                                                                                                                                                                  
    
    ? Select an option
    > Exit

Results of manual testing

Note

We need to follow up with the gh-copilot maintainers to change its update notice behavior to avoid the extra notice.

Here is the textual version:

> Exit


A new release of gh-copilot is available: 1.0.6 → 1.1.0
https://github.com/github/gh-copilot/releases/tag/v1.1.0



A new release of copilot is available: 1.0.6 → 1.1.0
To upgrade, run: gh extension upgrade copilot
https://github.com/github/gh-copilot



A new release of gh is available: 2.67.20250306 → 2.68.1
https://github.com/cli/cli/releases/tag/v2.68.1

Here is the visual version (with long text from extension cut off)

Screenshot of manually testing gh update notice behavior

@BagToad
BagToad force-pushed the kw/cli-10242-fix-core-github-cli-update-checker-disabled branch from 27f9a37 to cce46b9 Compare April 16, 2025 21:16
@BagToad
BagToad marked this pull request as ready for review April 16, 2025 21:20
@BagToad
BagToad requested a review from a team as a code owner April 16, 2025 21:20
@BagToad
BagToad requested a review from babakks April 16, 2025 21:20
@BagToad
BagToad temporarily deployed to cli-automation April 16, 2025 21:20 — with GitHub Actions Inactive

@williammartin williammartin left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the acceptance tests which require func() ~int as the Main entry point.

➜  GH_ACCEPTANCE_HOST=github.com GH_ACCEPTANCE_ORG=gh-acceptance-testing go test -count=1 -tags=acceptance -run ^TestIssues$ ./acceptance -v
# github.com/cli/cli/v2/acceptance_test [github.com/cli/cli/v2/acceptance.test]
acceptance/acceptance_test.go:21:13: not enough arguments in call to ghcmd.Main
        have ()
        want (string)

It would be ok to introduce a new func AcceptanceMain in the acceptance test package which calls this ghcmd.Main(false). That said, I think it's a bit strange to special case this behaviour into main but I can't specifically think of a reason not to do it, so I say we just move forward with this and see where we get to.

farhadsarani

This comment was marked as spam.

@williammartin

Copy link
Copy Markdown
Member

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.

@BagToad

BagToad commented Apr 22, 2025

Copy link
Copy Markdown
Member Author

@williammartin I think that's compelling. I'll close this PR for now, and bring the discussion back to the issue, since I'll likely open a fresh PR for that implementation.

@BagToad BagToad closed this Apr 22, 2025

This branch was previously deployed

1 inactive deployment
cli-automation — cce46b90 Deployed Apr 16, 2025 by BagToad
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

5 participants