-
Notifications
You must be signed in to change notification settings - Fork 6.9k
fix(nix): make shell completion installation resilient to failures #9283
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
fix(nix): make shell completion installation resilient to failures #9283
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes Nix build failures when shell completion generation fails or produces empty output by making the completion installation process resilient to errors.
Changes:
- Added error handling (
|| true) to completion command execution - Captures completion output in variables before attempting installation
- Conditionally installs completions only when output is non-empty
- Separates bash and zsh completion installation into independent conditional blocks
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Any idea on why the completions aren’t being generated? Is it a specific platform issue? It’d be nice to see a root cause rather than a workaround |
|
Looking at actions runs I don’t see this as a problem, the only instances I see of the completions not being generated are when the cli literally crashes at runtime… you can think of being able to generate completions as a build check, if it fails then the build output is probably broken in some way and should be investigated |
It's not a workaround, it's a proper detection that completion are installed only if they are properly generated. Basically, nix packages with shell completions already install completion related to a specific shell only if the shell is actually part of the packages installed on NixOS (at least the ones properly done). There's tons of use cases of flake outside NixOS, such as using home manager on other Linux distros or macOS just to generate reproducible home environment. And in that environment, you have absolutely no guarantee that build packages are actually installed from the nixpkgs: home manager is mainly used to generate reproducible configurations while relying on the OS binaries using package overload to dummy. For example, macOS users using nix-darwin + home manager do not install zsh or git from nixpkgs but generate their respective dotfiles from home manager flake. That error is happening on any OS compatible with that use case with bash not installed (dash based distros or recent macOS, not yet in CI). |
Generating completions is a base functionality of opencode, if it fails, that's an indicator of something being seriously wrong with the resultant binary
that's not how it works, installShellCompletion simply puts a file at the appropriate shell specific location in the $out directory of the build.
sure... and? I dont see how this is relevant
again, that's not how it works, installShellCompletion runs at build time (where stdenvNoCC,mkDerivation literally means "run some commands in bash") to create completion files and put then in the derivation output, nothing to do with how the package is used/installed. |
And fail miserably if there's no input, it's how it works. If you do not understand why functionA output feeded to functionB input known to fail if empty needs to be checked BEFORE to handle failures encountered in the real word with for examples https://github.com/jerome-benoit/dotfiles home-manager configuration run on macOS or Linux distros with nix installed or any OS supporting nix but not offering the environment needed to generate the shell completion for a shell with a piece of code but will still work without it ... do you think people using the nix package care more about having an up to date opencode nix package without completion on a shell they do not use or not? It's like saying: do not add a NULL pointer check before its dereference code path because the pointer is not supposed to be NULL ... It's called a defensive programming pattern and it does not hinder to actually check the root cause of the NULL pointer source. |
I think you have a fundamental misunderstanding of nix. If the package is being built, "the environment needed to generate the shell completions" is always available - that's literally the guarantee nix gives you. Just because you may be building opencode on "dash based distros or recent macOS" doesnt mean nix isnt going to run bash. And if you're grabbing the prebuilt binary on a host without that capability, hooray, the builder built the shell completion files already and includes them for free!
yes, that's the point - if opencode cant even generate completions then it is not a successful build
it's not defensive programming, its discarding an error in favor of needing to later discover that a component is broken. see how literally every other nixpkgs package generates/installs completions: https://sourcegraph.com/search?q=context:global+lang:nix+installShellCompletion+repo:%5Egithub%5C.com/NixOS/nixpkgs%24+&patternType=keyword&sm=0 1137 packages can't all be doing it wrong |
It's a theoretical claim that does not stand in the real world vs. cross platform and here you have a counter example: a nix package that build a totally working binary on a target platform even if a build step that should work is not working as intended for unknown reason so far. And it's getting quite ridiculous, a simple and common defensive programming pattern per definition to ensure people on macOS latest can build and run opencode using nix-darwin generating unconstructive comments: the issue is 100% reproducible on macOS latest. And the code needed to track it and not fail is a best practice that is used in all the code allowing that unfruitful discussion to happen and not fail, it's the unix legacy. |
…llation Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
bf51803 to
2c9c5e0
Compare
Fixes #9285
Summary
Fixes Nix build failure when shell completion generation fails or produces empty output.
Changes
|| trueto prevent build failures if bash or zsh are not installed-ntest)echoinstead of direct command substitutionProblem
The build was failing with:
This occurred when:
opencode completioncommand failed or returned empty output during the Nix build processSolution
The completion installation is now optional and gracefully handles failures, allowing the build to succeed even if completions cannot be generated. Warning messages are displayed when completion generation fails.