fix(install): announce fallback apt-get update, make its test hermetic - #106
Merged
Merged
Conversation
…metic desktop_install_enabled() silently ran `sudo apt-get update` when APT package lists were unpopulated, before the script ever explained itself or asked permission - inconsistent with every other sudo call in this script. Log an info() line first. tests/install_script_test.go's "falls back to CLI only" case only shimmed apt-cache, so on a machine whose real /var/lib/apt/lists happens to be empty, running `go test` would trigger a genuine `sudo apt-get update` (sudo enforces its own secure_path, so shimming apt-get instead would not reliably intercept it). Shim sudo itself to keep the test hermetic regardless of the test machine's real APT state.
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
Fixes two small issues found while reviewing the CLI-only installer support merged in #103 (
db0b0d7onmaster):desktop_install_enabled()(install.sh) silently ransudo apt-get updatewhen APT package lists were unpopulated, before ever explaining itself — inconsistent with every other sudo call in the script. Now logs aninfo()line first.tests/install_script_test.go's "falls back to CLI only when unavailable" case only shimmedapt-cache, notsudo— on a machine whose real/var/lib/apt/listshappens to be empty,go testcould trigger a genuinesudo apt-get update.sudoenforces its ownsecure_path, so a PATH-onlyapt-getshim would not reliably intercept it; shimmingsudoitself does. Independently verified (outside the Go test, by sourcinginstall.shdirectly withapt_lists_populatedforced to fail) that the shim now correctly intercepts the call and the newinfo()message prints.No behavior change to the installer's actual CLI-only-vs-Desktop decision logic — both fixes are additive (a log line, a test shim).
Validation
All pass. Also manually confirmed the fix end-to-end by sourcing
install.shwithapt_lists_populatedoverridden to force the fallback branch, and a loggingsudoshim inPATH— confirmed the newinfo()message prints and the shim (not a realsudo) is invoked.Linked issue
Closes #105