-
-
Notifications
You must be signed in to change notification settings - Fork 15.5k
fish: make python an optional dependency #86316
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
Conversation
@ofborg eval Also, let's get an eval going... |
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.
Diff LGTM. Overall in favor of this change. fish_config
with Python disabled fails, as expected.
EDIT: Closure goes from 190.3M
to 125.5M
with Python disabled, according to nix path-info -Sh ./result
.
[2 built, 0.0 MiB DL]
https://github.com/NixOS/nixpkgs/pull/86316
1 package built:
fish
@ofborg build fish
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
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.
Diff LGTM, I'd just like to see some rationale in the commit message (see line comment).
Python is still used for tests, but does not become part of the closure. In addition, nowadays fish only ever uses Python via __fish_anypython, so using sed or propagating python isn't necessary anymore.
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.
Sorry this was left alone for so long. The good news is: now I can merge it myself :)
Diff LGTM, just testing it out locally and then I'll merge. Thanks!
Thanks again :) |
Motivation for this change
Fish needs Python only for generating completions from man pages and the web config editor, so people may want to avoid depending on it. Even without depending on Python, the functions can still be called by adding Python to
$PATH
(using e.g.nix-shell
). The package still depends on Python by default.Notes
These days fish only uses the
__fish_anypthon
function to find an interpreter to use, so using sed or propagating python isn't necessary anymore.cc @adisbladis
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)