-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
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
Extend writeShellScript and friends #123608
Conversation
runtime = { | ||
preamble = "#!${runtimeShell}"; | ||
checkPhase = '' | ||
${stdenv.shell} -n $out |
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.
I believe you'll want runtimeShell
here as well:
${stdenv.shell} -n $out | |
${runtimeShell} -n $out |
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.
yup! That's what I get for writing this in 5 minutes ha.
${stdenv.shell} -n $out | ||
''; | ||
}; | ||
bash = { |
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.
I wonder if not this could be removed, since runtimeShell
variable is set to bash already.
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.
It's a bit superfluous, but I liked the distinction between the nixpkgs-wide runtime shell, whatever that may be, and bash. That way if I choose bash, I know it's bash, rather than assuming runtimeShell == bash
. I know there was talk awhile ago of switching runtimeShell to dash for performance.
''; | ||
}; | ||
zsh = { | ||
preamble = "#!${lib.getBin zsh}/bin/zsh"; |
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.
Could do
preamble = "#!${lib.getBin zsh}/bin/zsh"; | |
preamble = "#!${lib.getBin zsh}/${zsh.shellPath}"; |
and same with check path below. Indeed could generalize this by creating a function that takes the shell package as an argument.
Hmm, better yet. Maybe it would be possible to put the write*
functions directly as a passthrough in the derivation? It would be pretty cool if could do something like
pkgs.zsh.writeProgram "foo" {} ''
echo hello world
''
or
pkgs.fish.writeProgram "foo" {} ''
echo hello world
''
or even
pkgs.ghc.writeProgram "foo" {} ''
main = putStrLn "hello world"
''
I kind of like that since it feels more modular, we wouldn't have to put so many new functions in trivial-builders. It would also be possible to more naturally use a specific package (e.g., with overrides) in the creation of the program.
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.
Note, my idea with the {}
is that it would allow whatever customization that may be necessary, e.g.,
pkgs.zsh.writeProgram "foo" { executable = false; } ''
echo hello world
''
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.
Created a quick and dirty proof of concept in #124080.
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.
I like it! It definitely makes more sense once you start thinking of writePythonScript
, writeJuliaScript
, etc.
I've seen more chatter lately of cleaning up nixpkgs in general (e.g. #107539), so maybe this could part of that discussion?
I'd love if there was a standardized interface for where these kinds of functions should live. I kind of like the idea of there being a canonical langPlatform
(e.g. rustPlatform
) for every language where all things related to lang
could live.
I marked this as stale due to inactivity. → More info |
@colinxs are you still interested in pursuing this? |
Tentatively closing. Happy to reopen if you're still interested. |
Motivation for this change
From this discussion over at home-manager it came up that it would be useful to extend
writeShellScript
to provide both executable and non-executable variants (writeShellScript
andwriteShellText
, resp.), and to support additional shells (e.g.writeZSHScript
andwriteZSHText
).The motivation for this change is taken from home-manager where a large number of modules generate shell text (e.g. .bashrc) or scripts. Making sure these files are correct is critical to avoiding a corrupted system (the other day I had to jump into the terminal and debug a broken .profile). These files may or may not be executable and run in a variety different shells. The same issue arises for NixOS modules. We discussed adding these functions in home-manager, but thought nixpkgs would be a better home.
This PR is just a sketch as I wasn't sure how the bootstrapping worked in pkgs/top-level/stage.nix and because there's a significant amount of duplication in its current form.
I figured I'd solicit feedback before cleaning things up.
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)