fix(binary): revert EnsureBinary unknown-version guard (#152 regression)#156
Merged
Conversation
PR #152 changed EnsureBinary to treat an empty local.Version (from a failing VersionLocalF) as "needs download". The intent was to prevent a broken version probe (like argsh's 'version' subcommand bug) from silently skipping updates via the "" == "" match. However, many presets have version probes that fail for environmental reasons — tilt's 'tilt version' fails when 'getent' isn't in $PATH, for example. With the #152 change, tilt re-downloaded its 41MB binary on every 'b update' because the version probe returned empty. Revert to the original logic where "" == "" → skip. The argsh fix (calling --version instead of version, and parsing the right token) is sufficient for that case at the preset level — the framework doesn't need to change. The test is updated to verify the skip-when-unknown behaviour is correct and documents the trade-off: a genuinely outdated binary with a broken version probe won't auto-update until the probe is fixed or the user runs 'b update --force'. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reverts the EnsureBinary “unknown local version” guard introduced in #152, restoring the prior behavior where an empty VersionLocalF result can still satisfy the skip-check during b update, preventing repeated re-downloads when version probing is broken (e.g., tilt when getent is missing).
Changes:
- Revert
EnsureBinaryto allow the existing skip condition to short-circuit even whenlocal.Versionis empty. - Update/rename the regression test to assert “skip when version is unknown” rather than “force download when version is unknown”.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/binary/binary.go | Restores original update skip-check behavior by removing the local.Version != "" guard. |
| pkg/binary/binary_test.go | Replaces the prior regression test with one that verifies skipping downloads when the local version probe fails. |
…rkaround ssh-to-age gained a -version flag (prints bare version like "1.2.0"). Replace the old SHA256-hash-of-help-output workaround with a direct '-version' call. Prepend 'v' prefix so it matches GithubLatest's "v1.3.0" format for the skip comparison. Removes the hardcoded versions map and the crypto/sha256 import. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 tasks
fentas
added a commit
that referenced
this pull request
May 8, 2026
## Summary \`tilt version\` fails in b's clean exec environment because tilt's analytics init calls \`getent\` which isn't available. This left \`VersionLocalF\` unable to report the installed version. ## Fix Set \`TILT_DISABLE_ANALYTICS=1\` inside \`VersionLocalF\`, same pattern as: - k9s: \`K9S_LOGS_DIR=/tmp\` - argocd: \`HOME=/tmp\` - packer: \`HOME=/tmp\` ## Test plan - [x] \`go test ./...\` — all 39 packages pass - [x] With the EnsureBinary revert from #156, tilt's version check now succeeds → no more 41MB re-downloads on every \`b update\` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fentas
pushed a commit
that referenced
this pull request
May 8, 2026
🤖 I have created a release *beep* *boop* --- ## [4.17.1](v4.17.0...v4.17.1) (2026-05-08) ### Bug Fixes * **binary:** revert EnsureBinary unknown-version guard ([#152](#152) regression) ([#156](#156)) ([37428f4](37428f4)) * **preset:** disable tilt analytics during version check ([#158](#158)) ([f30b803](f30b803)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #152 changed `EnsureBinary` to treat empty `local.Version` (from a failing `VersionLocalF`) as "needs download". This caused tilt to re-download its 41MB binary on every `b update` because tilt's version probe fails when `getent` isn't in `$PATH`.
Root cause
`tilt version` → exits non-zero ("Fatal error initializing analytics: getent not found") → `VersionLocalF` returns `("", err)` → `local.Version = ""`.
With #152's guard: `local.Version != ""` = false → skip the skip-check → always download.
Without #152's guard (original logic): `"" == ""` → skip. Binary stays on disk.
Fix
Revert to the original `EnsureBinary` logic. The argsh fix in #152 was at the preset level (calling `--version` instead of `version`), so the framework-level guard isn't needed.
Trade-off
A genuinely outdated binary whose version probe is broken won't auto-update until:
This is acceptable — re-downloading 41MB on every update is worse.
Test plan
🤖 Generated with Claude Code