fix(update-check): detect nix and linuxbrew installs on linux - #344
fix(update-check): detect nix and linuxbrew installs on linux#344BenyD wants to merge 2 commits into
Conversation
The linux branch of getUpdateInstructions never inspected execPath outside the npm check, falling through to getLinuxPackageManager(), which only probes which package managers exist on the machine. A nix install on Debian or Ubuntu was told to run 'apt-get install infisical' for a binary in the immutable nix store. Homebrew-on-Linux hit the same gap, since /home/linuxbrew/.linuxbrew/ does not match the /homebrew/ substring the darwin branch checks. Mirrors the existing darwin behaviour: nix returns no instruction rather than a wrong one. Adds unit cases and integration steps to the integration-linux job. Partially addresses Infisical/infisical#226
|
✅ CLA satisfied. All contributors have signed the current CLA. The |
|
| Filename | Overview |
|---|---|
| packages/util/check-for-update.go | Adds Nix and Linuxbrew path detection; the Linuxbrew match is broader than necessary and can misclassify unrelated paths. |
| packages/util/check-for-update_test.go | Adds focused unit cases for canonical Nix and Linuxbrew executable paths. |
| .github/workflows/test-update-instructions.yml | Adds Linux integration checks proving Nix suppression and Linuxbrew guidance on an apt-enabled runner. |
Reviews (1): Last reviewed commit: "fix(update-check): detect nix and linuxb..." | Re-trigger Greptile
Review feedback: a bare "linuxbrew" substring also classified unrelated paths such as /opt/linuxbrew-tools as Homebrew installs. Uses "linuxbrew/" rather than a fully bounded "/linuxbrew/" so both real layouts still match: the multi-user default (/home/linuxbrew/.linuxbrew) and the single-user prefix (~/.linuxbrew), where the component is preceded by a dot rather than a slash. Adds a single-user case and a negative case for /opt/linuxbrew-tools, plus a notExpected field on the test table to express the negative assertion.
|
Gentle bump on this one. It is still mergeable with no conflicts and the CLA is signed. Happy to rebase onto current main, split the nix and linuxbrew cases into separate commits, or reword the update instructions if any of that would make review easier. Just let me know what you would prefer. |
Description 📣
On Linux,
getUpdateInstructionsnever inspects the executable path outside the npm check. It falls through togetLinuxPackageManager(), which only probes which package managers happen to exist on the machine, so a Nix install on Debian or Ubuntu is told to runapt-get install infisicalfor a binary living in the immutable Nix store.Homebrew-on-Linux hits the same gap, since
/home/linuxbrew/.linuxbrew/does not match the/homebrew/substring the darwin branch checks for.This mirrors the existing darwin handling: Nix returns no instruction rather than a wrong one, matching the
darwin nix returns emptycase already in the tests. Linuxbrew returns the brew command.Fixes the Linux side of #226. The macOS path in that report already returns no instruction for Nix installs, via the existing
darwin nix returns emptyhandling.Type ✨
Tests 🛠️
Added two cases to
TestGetUpdateInstructions, and two steps to theintegration-linuxjob following the existing/tmp/homebrewand npm patterns.Worth noting for review: the
linux nix returns emptyunit case only asserts anything on a machine that actually has a package manager installed. On a dev Mac it passes with or without this change, becausegetLinuxPackageManager()returns empty there anyway. On the ubuntu runner, where apt-get is present, it fails without the fix. That is why the integration step matters here, and I left a comment above it explaining the same thing.go test ./packages/util/ -run TestGetUpdateInstructions -vVerified the new tests are meaningful by stashing the change in
check-for-update.goand re-running:linux_linuxbrewfails as expected.