feat(self): replace GitHub release install with go install#92
Conversation
Maintainers aren't regularly creating GitHub releases, so self-updates via release assets are unreliable. Switch to `go install` from the main branch, which always reflects the latest source. Add a new GoBin base type that runs `go install`, copies the resulting binary from GOBIN into the backplane directory structure, and uses the VCS commit hash embedded in the binary as the version identifier. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: geowa4 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughAdds an exported Go-based installer type Changes
Sequence DiagramsequenceDiagram
participant User
participant SelfTool as Self Tool
participant GoBin as GoBin.Install
participant GoCmd as go executable
participant FS as Filesystem
participant BuildInfo as debug/buildinfo
User->>SelfTool: invoke Install()
SelfTool->>GoBin: delegate Install()
GoBin->>GoCmd: lookup 'go' executable
GoCmd-->>GoBin: path to go
GoBin->>GoCmd: run 'go install Module@Branch'
GoCmd->>FS: compile & write binary to GOBIN/GOPATH/bin
FS-->>GoCmd: binary created
GoBin->>GoCmd: run 'go env GOBIN' / 'go env GOPATH'
GoCmd-->>GoBin: output directory
GoBin->>FS: verify binary exists
FS-->>GoBin: binary confirmed
GoBin->>BuildInfo: read debug/buildinfo from binary
BuildInfo-->>GoBin: module path and version
GoBin->>FS: create versioned dir, copy binary, set exec perms
GoBin->>FS: remove old 'latest' and create new symlink to versioned binary
FS-->>GoBin: filesystem ops complete
GoBin-->>SelfTool: Install complete
SelfTool-->>User: success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 7 minutes and 34 seconds. Comment |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/tools/base/gobin.go`:
- Around line 14-24: Default.InstalledVersion() is unreliable for GoBin because
GoBin.Install writes into a revision-specific directory; implement
GoBin.InstalledVersion() to return the actual installed vcs.revision instead of
inheriting Default behaviour by locating the installed binary directory used by
Install (use the same path logic as GoBin.Install), read the revision metadata
file that Install writes (or parse the revision from the directory name), and
return that revision string (ensure signature InstalledVersion() (string, error)
on type GoBin).
- Around line 82-100: The resolveGOBIN function currently treats the entire
GOPATH string as one path (variable gopath) and does filepath.Join(gopath,
"bin"), which breaks when GOPATH contains multiple entries; update resolveGOBIN
to call filepath.SplitList(gopath), select the first non-empty entry
(firstGOPATH := split[0] or iterate to find first non-empty), validate it is not
empty, and return filepath.Join(firstGOPATH, "bin") instead of joining the full
GOPATH string; keep existing error handling for exec.Command and use the same
variable names (gobin, gopath) so the change is minimal and localized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fc9b6608-ee3e-47a7-8419-a46ce94c23d8
📒 Files selected for processing (2)
pkg/tools/base/gobin.gopkg/tools/self/self.go
| type GoBin struct { | ||
| Default | ||
| Module string | ||
| Branch string | ||
| } | ||
|
|
||
| func (g *GoBin) LatestVersion() (string, error) { | ||
| return g.Branch, nil | ||
| } | ||
|
|
||
| func (g *GoBin) Install() error { |
There was a problem hiding this comment.
Implement InstalledVersion() for GoBin.
GoBin now installs into a revision-specific directory, but it still inherits Default.InstalledVersion(), which scrapes a hash out of the binary bytes. That won't reliably return the vcs.revision you extract during install, so list installed and upgrade decisions can be wrong.
🔧 Proposed fix
type GoBin struct {
Default
Module string
Branch string
}
func (g *GoBin) LatestVersion() (string, error) {
return g.Branch, nil
}
+
+func (g *GoBin) InstalledVersion() (string, error) {
+ return extractVCSRevision(g.SymlinkPath())
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type GoBin struct { | |
| Default | |
| Module string | |
| Branch string | |
| } | |
| func (g *GoBin) LatestVersion() (string, error) { | |
| return g.Branch, nil | |
| } | |
| func (g *GoBin) Install() error { | |
| type GoBin struct { | |
| Default | |
| Module string | |
| Branch string | |
| } | |
| func (g *GoBin) LatestVersion() (string, error) { | |
| return g.Branch, nil | |
| } | |
| func (g *GoBin) InstalledVersion() (string, error) { | |
| return extractVCSRevision(g.SymlinkPath()) | |
| } | |
| func (g *GoBin) Install() error { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/tools/base/gobin.go` around lines 14 - 24, Default.InstalledVersion() is
unreliable for GoBin because GoBin.Install writes into a revision-specific
directory; implement GoBin.InstalledVersion() to return the actual installed
vcs.revision instead of inheriting Default behaviour by locating the installed
binary directory used by Install (use the same path logic as GoBin.Install),
read the revision metadata file that Install writes (or parse the revision from
the directory name), and return that revision string (ensure signature
InstalledVersion() (string, error) on type GoBin).
There was a problem hiding this comment.
Default.InstalledVersion() works correctly here — it extracts the version from the directory name in the symlink path (splits on path separator and takes element [1]). Since GoBin.Install() uses the module pseudo-version as the directory name, the inherited method returns exactly that pseudo-version. Verified with backplane-tools list installed showing v1.3.1-0.20260423145906-289db43d04d1.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the Default.InstalledVersion() implementation
rg -n "InstalledVersion" --type=go -A 10
Length of output: 3423
@geowa4, you're right — I misread the implementation. Default.InstalledVersion() resolves the latest/ symlink and extracts the version from the directory name in the resolved path, not from binary bytes. Since GoBin.Install() names the version directory after the module pseudo-version, the inherited method returns exactly that value. My review comment was a false positive. Thanks for the clarification and for verifying it end-to-end!
📝 Learnings were identified and not saved (knowledge base disabled). Enable
…builds Binaries built via `go install module@branch` don't have VCS build info embedded since they're built from module cache, not a git checkout. Use the module pseudo-version from build info instead, which includes the commit hash. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
pkg/tools/base/gobin.go (1)
92-100:⚠️ Potential issue | 🟡 MinorHandle multi-entry
GOPATHbefore appending/bin.On Line 100,
filepath.Join(gopath, "bin")treats a path list as a single directory. IfGOPATHhas multiple entries, binary discovery can fail aftergo install.🛠️ Proposed fix
func resolveGOBIN(goBin string) (string, error) { out, err := exec.Command(goBin, "env", "GOBIN").Output() if err != nil { return "", fmt.Errorf("failed to run 'go env GOBIN': %w", err) } gobin := strings.TrimSpace(string(out)) if gobin != "" { return gobin, nil } out, err = exec.Command(goBin, "env", "GOPATH").Output() if err != nil { return "", fmt.Errorf("failed to run 'go env GOPATH': %w", err) } gopath := strings.TrimSpace(string(out)) - if gopath == "" { + entries := filepath.SplitList(gopath) + first := "" + for _, e := range entries { + if e != "" { + first = e + break + } + } + if first == "" { return "", fmt.Errorf("both GOBIN and GOPATH are empty; cannot determine where 'go install' placed the binary") } - return filepath.Join(gopath, "bin"), nil + return filepath.Join(first, "bin"), nil }#!/bin/bash # Verify resolveGOBIN still uses whole GOPATH instead of first entry. sed -n '82,105p' pkg/tools/base/gobin.go rg -n 'filepath\.SplitList|filepath\.Join\(gopath,\s*"bin"\)' pkg/tools/base/gobin.go🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/tools/base/gobin.go` around lines 92 - 100, The code treats a possibly multi-entry GOPATH as a single directory by doing filepath.Join(gopath, "bin"); update the function that computes the GOBIN fallback (the block that calls exec.Command(goBin, "env", "GOPATH").Output() and uses variable gopath) to split gopath with filepath.SplitList, pick the first non-empty entry (or return an error if none), then return filepath.Join(firstEntry, "bin") so the binary path uses the first GOPATH element rather than the whole path list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/tools/base/gobin.go`:
- Around line 92-100: The code treats a possibly multi-entry GOPATH as a single
directory by doing filepath.Join(gopath, "bin"); update the function that
computes the GOBIN fallback (the block that calls exec.Command(goBin, "env",
"GOPATH").Output() and uses variable gopath) to split gopath with
filepath.SplitList, pick the first non-empty entry (or return an error if none),
then return filepath.Join(firstEntry, "bin") so the binary path uses the first
GOPATH element rather than the whole path list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9e6c55bc-342d-48f6-a194-f3e4a746c8a8
📒 Files selected for processing (1)
pkg/tools/base/gobin.go
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
Satisfies the perfsprint linter rule. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use filepath.SplitList to split GOPATH and take the first entry, since go install places binaries in the first GOPATH's bin directory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
pkg/tools/base/gobin.go (1)
93-101:⚠️ Potential issue | 🟡 MinorSplit
GOPATHlist entries before derivingbinpath.At Line 101, joining the raw
GOPATHstring with"bin"breaks whenGOPATHhas multiple entries. Usefilepath.SplitList(gopath)and choose the first non-empty entry.🛠️ Proposed fix
out, err = exec.Command(goBin, "env", "GOPATH").Output() if err != nil { return "", fmt.Errorf("failed to run 'go env GOPATH': %w", err) } gopath := strings.TrimSpace(string(out)) -if gopath == "" { +entries := filepath.SplitList(gopath) +first := "" +for _, e := range entries { + if strings.TrimSpace(e) != "" { + first = e + break + } +} +if first == "" { return "", errors.New("both GOBIN and GOPATH are empty; cannot determine where 'go install' placed the binary") } -return filepath.Join(gopath, "bin"), nil +return filepath.Join(first, "bin"), nil#!/bin/bash # Verify current implementation still joins full GOPATH and does not split list entries. rg -n -C2 'go env GOPATH|filepath\.Join\(gopath,\s*"bin"\)|filepath\.SplitList\(gopath\)' pkg/tools/base/gobin.go🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/tools/base/gobin.go` around lines 93 - 101, The code currently uses the raw gopath string and returns filepath.Join(gopath, "bin"), which fails when GOPATH contains multiple entries; instead call filepath.SplitList(gopath), iterate the returned list to pick the first non-empty entry (e.g., first entry from split), and then return filepath.Join(firstEntry, "bin"); update the logic around the gopath variable and the final return accordingly (keep the existing error handling when gopath is empty).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/tools/base/gobin.go`:
- Around line 93-101: The code currently uses the raw gopath string and returns
filepath.Join(gopath, "bin"), which fails when GOPATH contains multiple entries;
instead call filepath.SplitList(gopath), iterate the returned list to pick the
first non-empty entry (e.g., first entry from split), and then return
filepath.Join(firstEntry, "bin"); update the logic around the gopath variable
and the final return accordingly (keep the existing error handling when gopath
is empty).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6a03c5f8-68ae-4017-adc2-d0689748b131
📒 Files selected for processing (1)
pkg/tools/base/gobin.go
|
@geowa4: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #92 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 34 35 +1
Lines 1594 1607 +13
=====================================
- Misses 1594 1607 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/lgtm |
3 participants
Summary
go install github.com/openshift/backplane-tools@mainbase.GoBinbase type that handles the full install flow:go install, GOBIN resolution, commit hash extraction from binary build info, copy to backplane directory, and symlinkpkg/tools/self/self.go) is now a minimal constructor embeddingbase.GoBinMotivation: Maintainers aren't regularly creating GitHub releases, making self-updates via release assets unreliable.
go installfrom main always reflects the latest source.How it works:
Install()runsgo install ...@main, copies the binary from GOBIN into~/.local/bin/backplane/backplane-tools/<commit-hash>/, and symlinks tolatest/vcs.revision) viadebug/buildinfoLatestVersion()returns"main"(never matches a commit hash), so upgrades always re-rungo installTest plan
go vet ./...andgo build ./...passbackplane-tools install backplane-toolsinstalls viago installand creates the expected directory structurebackplane-tools list installedshows the commit hash as the installed versionbackplane-tools upgrade backplane-toolsalways re-installs from maingoon PATH, a clear error is shown🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor