Shebang support with flakes #5189

tomberek · 2021-08-28T20:40:36Z

Enables shebang usage of nix shell. All arguments with #! nix get
added to the nix invocation. This implementation does NOT set any
additional arguments other than placing the script path itself as the
first argument such that the interpreter can utilize it.

Example below:

    #!/usr/bin/env nix
    #! nix shell --quiet
    #! nix nixpkgs#bash
    #! nix nixpkgs#shellcheck
    #! nix nixpkgs#hello
    #! nix --ignore-environment --command bash
    # shellcheck shell=bash
    set -eu
    shellcheck "$0" || exit 1
    function main {
        hello
        echo 0:"$0" 1:"$1" 2:"$2"
    }
    "$@"

Overall it should make sense to any users of nix-shell.

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

agreed on idea
agreed on implementation strategy
tests, as appropriate
- functional tests - tests/**.sh
- unit tests - src/*/tests
- integration tests
documentation in the manual
code and comments are self-explanatory
commit message explains why the change was made
new feature or bug fix: updated release notes

roberth · 2021-08-29T22:26:07Z

I really like this. I didn't think we could use nix because portable /usr/bin/env doesn't support extra arguments, but you've proven me wrong 🎉.

My only objection as far as the design is concerned, is that this isn't about shells at all, but on closer reading, this isn't specific to the dubious nix shell command at all. In fact, this seems to solve #801.

thufschmitt

A couple of comments, but I really like the overall idea.

automatically inject "shell" argument?

I have a mixed feeling on that one: On the one hand, the only other command that (I think) could make sense in a shebang is nix develop, and that’s really an edge-case. On the other hand, having a too different behaviour when Nix is invoked as a shebang would probably be quite confusing (“why is ./myScript working but nix myScript returning some nonsense?”)

whitelist/blacklist certain options

I don’t think that would really be useful. At least from a security pov, you’re running an arbitrary script anyways, so restricting the interpreter wouldn’t change a lot.

support a bootstrap mechanism detecting if nix is installed and fetching otherwise (eg nix —store or nix-portable)?

How could that work? If you don’t have Nix installed, you can’t run the shebang at all afaik

src/libutil/args.cc

thufschmitt · 2021-08-31T08:12:49Z

src/libutil/args.cc

+    if (runEnv && cmdline.size() > 0) {
+        auto script = *cmdline.begin();
+        try {
+            auto lines = tokenizeString<Strings>(readFile(script), "\n");


I don’t know how much of an issue that is in practice, but this code reads the whole file, even when it bails right after because it doesn’t start by a shebang. Maybe you could use an FdSource or similar instead to only read as much as is needed

Something like this? tomberek@cf02a4f

Could not see how to use FdSource for this use case.

Is that approach somewhat sane in a C++ world?

Looks alright.
The #! check is a bit complicated, but at least it won't suffer when the first "line" is very long (e.g. not a text file). When it looks like a shebang, you'll still want to ignore the whole line though.

I like tomberek@cf02a4f.
It would also be nice to have a way to have an explicit final nix shebang lines, so that we can stop scanning. This would allow for huge files to start with a nix shebang, which is useful for installers. Perhaps a line with #! nix-end-of-arguments or something?

Another issue I’ve sometime hit is the desire to have a bit of shell parsing of the command line, or access to a SOURCE_SCRIPT var. (it’s an XY problem situation, I’m still trying to support perlPackages, etc)

src/libutil/args.cc

roberth · 2021-08-31T08:54:52Z

To support nix develop, you could override the various basePath/baseDir variables to point to the script's location. That way its functioning does not depend on the caller's working directory, which could be anywhere.

tomberek · 2021-08-31T15:31:22Z

Another issue that has come up as I'm writing the docs (stealing most of it from nix-shell), is that we don't run the setup or shell hooks to modify PYTHONPATH or PERL5LIB and so forth. Not sure if we want to adopt exactly the legacy nix-shell behavior which led us down an convoluted path. Is there an easier way?

roberth · 2021-08-31T16:13:06Z

we don't run the setup or shell hooks

This is why we need to rename the current nix shell to something like nix env run.
If you want to run stdenv, currently your only nix-command option is nix develop.

I would like Nix to defer to packages, allowing it to take "installing to environment variables" seriously. Maybe it's overkill because for regular commands PATH+XDG_DATA_DIRS will generally do, but perhaps this idea can be used for more complicated stdenv-like environment composition. Assuming Nix can somehow learn how to compose variables. (Not everything is :-separated)

Previously my (and apparently Eelco's) assumption was that we don't need the stdenv/setup-based ad-hoc shells like you are asking for. What's your use case? For packaging we've assumed writing an expression is best.

Anyway that's all fairly orthogonal to the shebang implementation, as long as we don't

automatically inject "shell" argument

I hope I was able to provide some context.

roosemberth · 2021-09-02T10:30:26Z

   #!/usr/bin/env nix
   #! nix shell --quiet
   #! nix nixpkgs#bash
   #! nix nixpkgs#shellcheck

I find utterly confusing references to derivations are written as #! nix f#pkg instead of #! nix shell f#pkg or simply #! f#pkg.

I think we should also support a varadic derivation references until the end of the line (e.g. #! nix shell f#pkg1 f#pkg2) like nix shell already does.

roberth · 2021-09-02T10:45:10Z

The #! nix has the same role as #!nix-shell which seemed to have served a technical purpose for potentially mixing shebang aware interpreters. I wonder to what extend this is required. Perhaps it could be optional? For example, we could strip either #! or #!nix, whichever is a longer prefix. That way, we only need to confuse the reader when it's necessary for technical reasons.

Note that #! nix with the space would then be parsed as a #! prefix and a nix argument instead of #! nix as a prefix. I think this is an improvement, because #!nix reads a bit more like a single keyword.

Example:

#!/usr/bin/env nix
#! shell --quiet
#! nixpkgs#bash
#! nixpkgs#shellcheck

And in case we're invoking an interpreter that can and must ignore #!nix lines, but not #! lines.

#!/usr/bin/env nix
#!nix shell --quiet
#!nix nixpkgs#bash
#!nix nixpkgs#shellcheck
#! coolLang foo bar baz use imagination

roosemberth · 2021-09-02T12:49:54Z

I agree that `#!nix` would be much more idiomatic. I think making both `#!` and `#! nix` valid will just create FUD for no reason. Do you have examples of shebang-aware interpreters that'd play nice with nix shebangs as described in this document? I've never seen them in the wild (doesn't mean they don't exist), but the idea of supporting bot cases does seem a bit feature-creep to me.

On 2 September 2021 12:45:22 CEST, Robert Hensing ***@***.***> wrote: The `#! nix` has the same role as `#!nix-shell` which seemed to have served a technical purpose for potentially mixing shebang aware interpreters. I wonder to what extend this is required. Perhaps it could be optional? For example, we could strip either `#!` or `#!nix`, whichever is a longer prefix. That way, we only need to confuse the reader when it's necessary for technical reasons. Note that `#! nix` with the space would then be parsed as a `#!` prefix and a `nix` argument instead of `#! nix` as a prefix. I think this is an improvement, because `#!nix` reads a bit more like a single keyword. -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #5189 (comment)

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

tomberek · 2021-09-02T14:10:35Z

The proposed approach is variadic and can be put all on one line. Sometimes these lines end up very long and having them split up can be helpful.

As to the prefix, this was meant to copy the way nix-shell does it( most of the code was simply copied or adopted over).

There are several shebang-aware interpreters, eg perl. I’m sure there are more.

The biggest issue is being unable to load the stdenv in the “right way”. I suspect that is actually what people will want. The examples of legacy nix-shell show bringing in python and Perl libraries and that will only work if we run shellHooks or at least do more than just modify PATH.

ymeister · 2021-09-08T18:12:01Z

Forgive me if I am wrong, but is it really necessary considering env -S option:

-S, --split-string=S
              process and split S into separate arguments; used to pass multiple arguments on shebang lines

That way your example becomes:

#!/usr/bin/env -S nix shell --quiet nixpkgs#bash nixpkgs#shellcheck nixpkgs#hello --ignore-environment --command bash
# shellcheck shell=bash
set -eu
shellcheck "$0" || exit 1
function main {
    hello
    echo 0:"$0" 1:"$1" 2:"$2"
}
"$@"

Aside from multi-line support, I don't see any problem here.

roberth · 2021-09-08T18:50:55Z

@ymeister that's a good suggestion, but it is not universally supported on the other unixes. It also doesn't allow spaces in arguments for use with --expr.

tomberek · 2021-09-08T19:09:09Z

Forgive me if I am wrong, but is it really necessary considering env -S option

The original nix-shell shebang mechanism did not rely on that feature - i presume for various compatibility reasons. If one wishes to use it for their own scripts, they are welcome to do so. It should work with this PR (if not, I'd consider that a bug).

My greater concern is that examples such as (https://github.com/NixOS/nix/blob/7ab3bc32b139e251ff8d9abf25d6cadf3c0c6945/doc/manual/src/command-ref/nix-shell.md#use-as-a--interpreter) won't work with nix shell as it is currently implemented. Thus this is involved with the whole nix-shell/nix shell/nix develop/nix run/nix env do-a-thing design and debate. My summary of the situation is that there are a few use cases:

Please put some packages A, B, C into my PATH. - usually nix-shell -p somepkg now nix shell
I want to develop on package A. - usually nix-shell -A somepkg now nix develop
Please make language-specific libraries available: eg: PYTHONPATH, PERL5LIB... - usually nix-shell -p or nix-shell shell.nix
Just run a program, don't change any env vars - nix run

To make (1)(3) possible nix-shell would create a derivation behind the scenes on your behalf, composing it together and additionally invoking stdenv environment setup and shell hooks. This is why nix shell nixpkgs#python3 nixpkgs#python3Packages.requests does not work as expected. The shebang example from this PR hits this issue. The only way to express "please run the shellHooks" is to create your own flake devShell, but ad-hoc CLI mechanisms don't seem to allow it.

Another approach

Can we add "multiple installables" support to nix develop? Then nix develop is always the one that brings in stdenv/shellHooks.

This would create three distinct tools:

nix run - only runs an executable, no changes to env vars, multiple installables makes no sense
nix shell - only adds things to PATH, multiple installables means to add them all to the PATH
nix develop - add things to PATH and run shell hooks. multiple installables means to do a "buildEnv" composition.

ymeister · 2021-09-09T09:36:07Z

@roberth that's a good notice. Albeit the wide adoption of env -S among other unixes, it doesn't seem to be POSIX-compliant, so that's a bummer. On another note, I haven't experienced any problem using it with --expr:

#! /usr/bin/env -S nix shell --impure --expr "with import <nixpkgs> {}; python.withPackages (pkgs: with pkgs; [ prettytable ])" -c python

import prettytable

# Print a simple table.
t = prettytable.PrettyTable(["N", "N^2"])
for n in range(1, 10): t.add_row([n, n * n])
print t

(with import <nixpkgs> {} requires nixpkgs flake path in $NIX_PATH. That can be achieved either by pinning it system-wide (NixOS), or by exporting NIX_PATH="nixpkgs=$(nix flake metadata nixpkgs --json | jq -r .path)")

Above code also showcases an example of a workaround of the issue discussed by @tomberek. Although I agree that it is just a workaround and is nowhere near a proper solution like one proposed by @tomberek.

roberth · 2021-09-09T09:50:55Z

I may have been wrong about -S with quotes. I only read the man page, not the info page (:confused:) which goes into more detail, confirming your observations for at least GNU env and FreeBSD.
Instead of <nixpkgs> you should be able to reference a local flake when the base path is passed correctly. Also note that nix shell --expr is very different from nix-shell --expr (without and with stdenv respectively; also use case (1) vs (2))

nixos-discourse · 2022-01-11T23:57:35Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-shell-with-shebang/17088/2

Artturin · 2022-03-16T00:31:32Z

Please make sure these 2 keep working, Thanks.

#!/usr/bin/env nix-shell
//! ```cargo
//! [dependencies]
//! time = "0.1.25"
//! ```
/*
#!nix-shell -i rust-script -p rustc -p rust-script -p cargo
*/
fn main() {
    for argument in std::env::args().skip(1) {
        println!("{}", argument);
    };
    println!("{}", std::env::var("HOME").expect(""));
    println!("{}", time::now().rfc822z());
}
// vim: ft=rust

#!/usr/bin/env nix-shell
#![allow()] /*
#!nix-shell -i bash -p rustc
rsfile="$(readlink -f $0)"
binfile="/tmp/$(basename "$rsfile").bin"
rustc "$rsfile" -o "$binfile" --edition=2021 && exec "$binfile" $@ || exit $?
*/
fn main() {
    for argument in std::env::args().skip(1) {
        println!("{}", argument);
    };
    println!("{}", std::env::var("HOME").expect(""));
}
// vim: ft=rust

chvp · 2022-05-02T08:07:45Z

For anyone stumbling onto this in the future, you can avoid having to set NIX_PATH by using the following (as roberth hinted):

#!/usr/bin/env -S nix shell --impure --expr "(import (builtins.getFlake \"nixpkgs\") {}).python3.withPackages (ps: [ ps.requests ])" --command python

roberth · 2023-05-01T14:56:38Z

src/libutil/args.cc

+    auto isNixCommand = std::regex_search(programName, std::regex("nix$"));
+    if (isNixCommand && cmdline.size() > 0) {


Perhaps a nice declaration would be

void Args::parseCmdline(const Strings & _cmdline, bool allowShebang = false);

This solves the architectural concern and a default is nicer than an overload I think.

roberth · 2023-05-01T15:03:38Z

src/libutil/args.cc

@@ -69,6 +76,36 @@ void Args::parseCmdline(const Strings & _cmdline)
    }

    bool argsSeen = false;
+
+    // Heuristic to see if we're invoked as a shebang script, namely,


This requires quite a lot of refactoring in nix-build.cc, which seems disproportionate. I'm not convinced that the resulting abstraction would be useful, and coupling to the very stable nix-build.cc may well be counterproductive.

That said, factoring out some static functions / private methods will probably make the code easier to follow.

roberth · 2023-05-01T15:04:53Z

src/libutil/args.cc

+                    std::smatch match;
+                    if (std::regex_match(line, match, std::regex("^#!\\s*nix\\s(.*)$")))
+                        for (const auto & word : shellwords(match[1].str()))
+                            cmdline.push_back(word);


The current implementation seems to strike a good balance. We may learn more in the experimental phase.
Resolving this thread.

src/libutil/args.cc

roberth · 2023-05-01T15:38:51Z

src/libutil/util.cc

+            case '"':
+                cur.append(begin, it);
+                begin = it + 1;
+                st = st == sBegin ? sQuote : sBegin;
+                break;
+            case '\\':
+                /* perl shellwords mostly just treats the next char as part of the string with no special processing */
+                cur.append(begin, it);
+                begin = ++it;
+                break;


Sorry to comment on moved code, but I think shellwords isn't quite appropriate for this use case. I think we should fork it to be more predictable and future proof.
By restricting the syntax, we can be more compatible with the shell (in the shebang -> shell direction only of course; wouldn't want to reimplement a shell - in the limit). Enforcing this is perhaps convenient but most of all helps with understanding. Furthermore each syntax we forbid is trivially forward compatible.
No need to go overboard with this, so what I've tried to narrow it down to are the following changes based on shellwords:

In the unquoted state, reject single quotes. We don't need them and putting them in the parsed string would confuse readers.

Similarly, we should reject/reserve at least these chars in the unquoted state: \, $, `, <, >; perhaps also *.

Fail for lines that have an unterminated quoted string.

~~Use single quotes, because we won't be doing variable substitution (certainly for now)~~ reject unescaped $ in double-quoted strings?

The backtick, <, and unterminated quoted string cases are particularly interesting, because they could serve as extension points for multiline strings, which I think would be very useful, but whose implementation could be scoped out of this PR.
OT?: I think double backticks would make a nice analog for nixlang's double single quote strings, except they'd be entirely verbatim.

These changes aren't compatible with nix-shell, so this would be new code and the move can be reverted.

@roberth This is still pending.

I think trying to conform with existing syntaxes and conventions is a far harder problem than coming up with a useful syntax that works well for our use case.
New proposal:

Use double backtick quotes as the only support type of quotation

Disallow any shell-like syntax outside the quoted strings, and be eager to reject syntax in general

This way we keep the code fairly simple, leaving reserved syntax for possible later extension, leaving our options open, while solving "our own use case" well: postponing good inline expression support seems like a mistake the more I think about it.

I'll have a go at implementing this.

roberth · 2023-05-01T15:42:30Z

src/nix/shell.md

+```bash
+#! /usr/bin/env nix
+#! nix shell --impure --expr
+#! nix "with (import (builtins.getFlake ''nixpkgs'') {}); terraform.withPlugins (plugins: [ plugins.openstack ])"


Suggested change

#! nix "with (import (builtins.getFlake ''nixpkgs'') {}); terraform.withPlugins (plugins: [ plugins.openstack ])"

#! nix 'with (import (builtins.getFlake "nixpkgs") {}); terraform.withPlugins (plugins: [ plugins.openstack ])'

This will then change a bit.

fricklerhandwerk · 2023-05-02T09:03:30Z

Reviewed in Nix team meeting 2023-05-01:

status unclear
@roberth: stuck on syntax?
@edolstra: not a replacement for nix-shell -p yet
@roberth: that's ok, separate scope
@roberth: can technically be done with nix develop --expr '...'
@tomberek: what about the syntax? Currently scans all comment-like lines
@tomberek will rebase, @roberth still assigned for review

nixos-discourse · 2023-05-02T09:12:00Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-05-01-nix-team-meeting-minutes-51/27803/1

Enables shebang usage of nix shell. All arguments with `#! nix` get added to the nix invocation. This implementation does NOT set any additional arguments other than placing the script path itself as the first argument such that the interpreter can utilize it. Example below: ``` #!/usr/bin/env nix #! nix shell --quiet #! nix nixpkgs#bash #! nix nixpkgs#shellcheck #! nix nixpkgs#hello #! nix --ignore-environment --command bash # shellcheck shell=bash set -eu shellcheck "$0" || exit 1 function main { hello echo 0:"$0" 1:"$1" 2:"$2" } "$@" ``` fix: include programName usage

fix: release notes

roberth · 2023-05-12T18:47:38Z

Oops, I didn't reset the ~~upstream~~ pushRemote. Made a separate PR with my additions. I'll revert this in a sec.
EDIT: github doesn't seem to tell me the old hash, or at least I'm not sure. I'll leave it at these rebased commits.

roberth · 2023-05-12T18:53:07Z

I've changed the quoting and interpretation of relative paths in #8327. wdyt?

nixos-discourse · 2023-05-16T21:34:45Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/flox-nix-community-update-2023-04/28233/1

tomberek · 2023-05-17T00:30:24Z

I've changed the quoting and interpretation of relative paths in #8327. wdyt?

Looks reasonable, but more contentious. I'm okay with either approach.

musjj · 2023-08-10T15:18:23Z

Would it be desirable to have support for brace expansion? So you can do something like this:

#!/usr/bin/env nix
#!nix shell --command bash
#!nix nixpkgs#{bash,jq,fd}

tomberek · 2023-11-23T03:10:28Z

v2 was merged, please try it out and provide feedback

tomberek force-pushed the shebang_flakes branch from e8231c5 to 8b6fcce Compare August 28, 2021 20:41

tomberek marked this pull request as draft August 28, 2021 20:44

tomberek mentioned this pull request Aug 31, 2021

pkgsMerge: automatic buildEnv's with chained packages NixOS/nixpkgs#136263

Closed

4 tasks

thufschmitt reviewed Aug 31, 2021

View reviewed changes

tomberek force-pushed the shebang_flakes branch from 8b6fcce to 31e9285 Compare August 31, 2021 15:37

tomberek changed the title ~~Shellbang support with flakes~~ Shellbang support with flakes (blocked on nix run/shell/develop semantics) Nov 11, 2021

fricklerhandwerk added the UX The way in which users interact with Nix. Higher level than UI. label Sep 9, 2022

thufschmitt mentioned this pull request Sep 19, 2022

Rename #! nix-shell to #! nix #7061

Closed

fricklerhandwerk added the feature Feature request or proposal label Sep 19, 2022

fricklerhandwerk mentioned this pull request Sep 19, 2022

Rename 'nix shell' #4715

Open

Artturin mentioned this pull request Oct 31, 2022

switch to nushell and drop gitpod nix-community/nixpkgs-wayland#376

Merged

tomberek force-pushed the shebang_flakes branch from 31e9285 to 86140ca Compare November 3, 2022 03:01

roberth assigned roberth and unassigned edolstra Jan 30, 2023

tomberek force-pushed the shebang_flakes branch from f8f41e7 to 4694b70 Compare May 1, 2023 14:25

github-actions bot added documentation new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority labels May 1, 2023

tomberek requested a review from roberth May 1, 2023 14:47

roberth reviewed May 1, 2023

View reviewed changes

tomberek and others added 8 commits May 12, 2023 20:03

src/libutil/util.hh: Formatting

59feecf

fix: test to ensure arguments are passed

7518ce6

doc: shebang release notes, docs, tests

6af0aff

fix: release notes

Read file incrementally

d7792eb

feat: break out of shebang processing for non-comments

750a4c5

doc: remove reference to nix-shell

da45757

fix: refactor parseCmdline interface

7aea62f

roberth force-pushed the shebang_flakes branch 2 times, most recently from f50bdfe to f8b54bf Compare May 12, 2023 18:39

roberth mentioned this pull request May 12, 2023

Shebang flakes v2 #8327

Merged

10 tasks

roberth force-pushed the shebang_flakes branch from f8b54bf to 829dd9f Compare May 12, 2023 18:46

roberth force-pushed the shebang_flakes branch from 829dd9f to 7aea62f Compare May 12, 2023 18:49

tomberek closed this Nov 23, 2023

		auto isNixCommand = std::regex_search(programName, std::regex("nix$"));
		if (isNixCommand && cmdline.size() > 0) {

	#! nix "with (import (builtins.getFlake ''nixpkgs'') {}); terraform.withPlugins (plugins: [ plugins.openstack ])"
	#! nix 'with (import (builtins.getFlake "nixpkgs") {}); terraform.withPlugins (plugins: [ plugins.openstack ])'

Shebang support with flakes #5189

Shebang support with flakes #5189

Conversation

tomberek commented Aug 28, 2021 • edited by roberth Loading

Checklist for maintainers

roberth commented Aug 29, 2021 • edited Loading

thufschmitt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roberth commented Aug 31, 2021

tomberek commented Aug 31, 2021

roberth commented Aug 31, 2021

roosemberth commented Sep 2, 2021 • edited Loading

roberth commented Sep 2, 2021 • edited Loading

roosemberth commented Sep 2, 2021 via email

tomberek commented Sep 2, 2021 • edited Loading

ymeister commented Sep 8, 2021

roberth commented Sep 8, 2021

tomberek commented Sep 8, 2021

Another approach

ymeister commented Sep 9, 2021

roberth commented Sep 9, 2021

nixos-discourse commented Jan 11, 2022

Artturin commented Mar 16, 2022

chvp commented May 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fricklerhandwerk commented May 2, 2023

nixos-discourse commented May 2, 2023

roberth commented May 12, 2023 • edited Loading

roberth commented May 12, 2023

nixos-discourse commented May 16, 2023

tomberek commented May 17, 2023

musjj commented Aug 10, 2023

tomberek commented Nov 23, 2023

tomberek commented Aug 28, 2021 •

edited by roberth

Loading

roberth commented Aug 29, 2021 •

edited

Loading

roosemberth commented Sep 2, 2021 •

edited

Loading

roberth commented Sep 2, 2021 •

edited

Loading

tomberek commented Sep 2, 2021 •

edited

Loading

roberth commented May 12, 2023 •

edited

Loading