Shebang flakes v2 #8327

roberth · 2023-05-12T18:44:54Z

Motivation

This adds shebang support as implemented by @tomberek with the changes/additions

Double backtick quoting only. This keeps things simple, while also delivering a powerful syntax for self contained scripts (one file) with custom expressions.
Relative paths are relative to the script. Otherwise, they'd be relative to the working directory, which is would be unpredictable and therefore less useful.
It reserves syntax in the shebang lines; in other words, some symbols demand quotes. This has two benefits
- The syntax looks unambiguous. Quoted symbols look like literal symbols, and they are.
- The syntax can be extended later, without breaking scripts that used to work.

Remaining todo:

last commit makes flake flags relative to script dir. tests?
should some commands add an implicit --? All commands or just run/shell/develop?

Context

Based on

Shebang support with flakes #5189

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 - tests/nixos/*
documentation in the manual
documentation in the internal API docs
code and comments are self-explanatory
commit message explains why the change was made
new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

fricklerhandwerk

From a superficial perspective of a potential user and manual reader, I mainly have two questions:

Why is that "weird" escaping needed? If it's technically unavoidable it sure needs the reason documented. Otherwise it appears arbitrary and cumbersome.
Why is it nix shell? What does that have to do with shells at all?

I understand that historically stuff was just tacked onto nix-shell being a kitchen sink, but this is a different situation. Could we do something like #! nix shebang ... --interpreter <command> which could be shortened with -i?

Or even #! nix interpret <command> ... where the command is positional?

fricklerhandwerk · 2023-05-13T18:46:08Z

doc/manual/src/release-notes/rl-next.md

+* The experimental nix command is now a `#!-interpreter` by appending the
+  contents of any `#! nix` lines and the script's location to a single call.


Suggested change

* The experimental nix command is now a `#!-interpreter` by appending the

contents of any `#! nix` lines and the script's location to a single call.

* The experimental `nix` command is now a `#!-interpreter` by concatenating the

contents of any `#! nix` lines and the script's location into a single call to ???.

Is that what you mean? What gets called? This isn't clear.

doc/manual/src/release-notes/rl-next.md

src/nix/shell.md

fricklerhandwerk · 2023-05-13T18:56:34Z

src/nix/shell.md

+> You must use double backticks (```` `` ````) when passing a simple Nix expression
+> in a nix shell shebang.
+
+Finally, using the merging of multiple nix shell shebangs the following


How is this merging something? I don't understand how it relates to the example. It looks exactly like the other ones.

multiple lines of "#!nix" are concatenated after that prefix is removed. This allows breaking up a long line into multiple lines.

fricklerhandwerk · 2023-05-13T18:58:09Z

src/nix/shell.md

+import Network.Curl.Download
+import Text.HTML.TagSoup
+import Data.Either
+import Data.ByteString.Char8 (unpack)
+
+-- Fetch nixos.org and print all hrefs.
+main = do
+  resp <- openURI "https://nixos.org/"
+  let tags = filter (isTagOpenName "a") $ parseTags $ unpack $ fromRight undefined resp
+  let tags' = map (fromAttrib "href") tags
+  mapM_ putStrLn $ filter (/= "") tags'


That's pretty much arbitrary stuff just to show the shebang mechanism. The actual scripts should aim to be one liners. Everything else will be noise to 90% of the readers.

history: I tried to copy the previous examples and modify as little as possible. I am not opposed to a simpler script if that would communicate the concept better.

src/nix/shell.md

nixos-discourse · 2023-05-31T02:08:53Z

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

https://discourse.nixos.org/t/2023-05-26-nix-team-meeting-minutes-58/28572/1

tomberek

Remaining items are release notes and docs updates from @fricklerhandwerk

doc/manual/src/release-notes/rl-next.md

tomberek · 2023-08-18T11:15:42Z

I started trying to incorporate the suggestions. (hercules-ci@5d33afe)

tests/flakes/flakes.sh

nixos-discourse · 2023-10-21T05:51:51Z

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

https://discourse.nixos.org/t/nix3-equivilents-to-nix-shell-i-python-p-hello/34417/2

src/libutil/args.cc

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 EDIT: For posterity I've changed shellwords to shellwords2 in order not to interfere with other changes during a rebase. shellwords2 is removed in a later commit. -- roberth

fix: release notes

This will allow a different base directory to be used, matching a shebang script location instead of the working directory.

Being restrictive about syntax leaves opportunity to improve the syntax and functionality later.

These usages of the working directory are perhaps unlikely to interact with shebangs, but the code is more consistent this way, and we're less likely to miss usages that do interact.

Ericson2314 · 2023-11-07T22:51:46Z

FYI perhaps we never made this policy, but this was merged (a) without a merge commit and (b) with commits that perhaps don't individually pass tests (my guess from the messages). The combination of those two things makes bisecting later harder. (Merge commit helps with more complicated history because --first-parent allows skipping the PR history.)

tomberek · 2023-11-08T01:02:29Z

FYI perhaps we never made this policy, but this was merged (a) without a merge commit and (b) with commits that perhaps don't individually pass tests (my guess from the messages). The combination of those two things makes bisecting later harder. (Merge commit helps with more complicated history because --first-parent allows skipping the PR history.)

Worth a revert and re-merge?

roberth · 2023-11-10T10:36:26Z

Three PRs have been merged since, and would have to be remerged, plus you get the general nastiness of force-pushing to master. I'd say leave as is, but pay attention to how you're merging PRs.
Assuming you're using the green button, that thing is stateful. It remembers which way you merged the last one, so just always leave it at "merge" and never micro optimize by using rebase for single commit PRs.
Squashing should be done with the CLI, preferably by the author.

nixos-discourse · 2023-12-19T10:07:18Z

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/4

roberth requested review from fricklerhandwerk and edolstra as code owners May 12, 2023 18:44

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 12, 2023

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

roberth mentioned this pull request May 12, 2023

Shebang support with flakes #5189

Closed

17 tasks

fricklerhandwerk reviewed May 13, 2023

View reviewed changes

thufschmitt assigned thufschmitt and tomberek and unassigned thufschmitt May 26, 2023

thufschmitt added the idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. label May 26, 2023

tomberek requested changes Jun 2, 2023

View reviewed changes

tomberek reviewed Aug 18, 2023

View reviewed changes

doc/manual/src/release-notes/rl-next.md Outdated Show resolved Hide resolved

tomberek reviewed Sep 28, 2023

View reviewed changes

tests/flakes/flakes.sh Outdated Show resolved Hide resolved

github-actions bot removed the with-tests Issues related to testing. PRs with tests have some priority label Oct 23, 2023

roberth force-pushed the shebang_flakes branch from 34a7333 to b8db017 Compare October 23, 2023 16:39

github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Oct 23, 2023

tomberek reviewed Nov 6, 2023

View reviewed changes

src/libutil/args.cc Show resolved Hide resolved

tomberek and others added 8 commits November 6, 2023 17:07

src/libutil/util.hh: Formatting

59b3909

fix: test to ensure arguments are passed

6cefab7

doc: shebang release notes, docs, tests

e2e46b6

fix: release notes

Read file incrementally

0574d80

feat: break out of shebang processing for non-comments

4a9a007

doc: remove reference to nix-shell

88e9281

fix: refactor parseCmdline interface

771ac96

libcmd: lookupFileArg(): add baseDir

e4970ac

This will allow a different base directory to be used, matching a shebang script location instead of the working directory.

roberth force-pushed the shebang_flakes branch from b8db017 to eb8d947 Compare November 6, 2023 16:22

roberth added 8 commits November 6, 2023 19:24

nix: Reserve shebang line syntax and only parse double backtick quotes

7db185e

Being restrictive about syntax leaves opportunity to improve the syntax and functionality later.

nix: Add command baseDir to parse --expr relative to shebang script

80e9bad

nix: Parse --file relative to shebang script

4344d4c

nix/installables.cc: Use getCommandBaseDir() where possible

9fcceae

These usages of the working directory are perhaps unlikely to interact with shebangs, but the code is more consistent this way, and we're less likely to miss usages that do interact.

Move shebang docs from rl-next to nix.md

e73466f

Fix nix shebang interaction with NixOS#8131 overhaul completions

d655946

parseShebangs: Make strings with backtick sequences representable

32f75a8

Test parseShebangContent round trip

8387c43

roberth force-pushed the shebang_flakes branch from 03329bb to 8387c43 Compare November 6, 2023 18:29

tomberek approved these changes Nov 7, 2023

View reviewed changes

tomberek merged commit ab69dc4 into NixOS:master Nov 7, 2023
11 checks passed

iFreilicht mentioned this pull request Nov 7, 2023

Link to shebang interpreter docs from release notes #9322

Merged

roberth mentioned this pull request Nov 16, 2023

Shebang flakes hercules-ci/nix#1

Closed

iFreilicht mentioned this pull request Jan 26, 2024

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Shebang flakes v2 #8327

Shebang flakes v2 #8327

roberth commented May 12, 2023 •

edited by tomberek

Loading

fricklerhandwerk left a comment •

edited

Loading

fricklerhandwerk May 13, 2023

fricklerhandwerk May 13, 2023

tomberek Jul 27, 2023

fricklerhandwerk May 13, 2023

tomberek Jul 27, 2023

nixos-discourse commented May 31, 2023

tomberek left a comment

tomberek commented Aug 18, 2023

nixos-discourse commented Oct 21, 2023

Ericson2314 commented Nov 7, 2023 •

edited

Loading

tomberek commented Nov 8, 2023

roberth commented Nov 10, 2023

nixos-discourse commented Dec 19, 2023

		* The experimental nix command is now a `#!-interpreter` by appending the
		contents of any `#! nix` lines and the script's location to a single call.

Shebang flakes v2 #8327

Shebang flakes v2 #8327

Conversation

roberth commented May 12, 2023 • edited by tomberek Loading

Motivation

Context

Checklist for maintainers

Priorities

fricklerhandwerk left a comment • edited Loading

Choose a reason for hiding this comment

fricklerhandwerk May 13, 2023

Choose a reason for hiding this comment

fricklerhandwerk May 13, 2023

Choose a reason for hiding this comment

tomberek Jul 27, 2023

Choose a reason for hiding this comment

fricklerhandwerk May 13, 2023

Choose a reason for hiding this comment

tomberek Jul 27, 2023

Choose a reason for hiding this comment

nixos-discourse commented May 31, 2023

tomberek left a comment

Choose a reason for hiding this comment

tomberek commented Aug 18, 2023

nixos-discourse commented Oct 21, 2023

Ericson2314 commented Nov 7, 2023 • edited Loading

tomberek commented Nov 8, 2023

roberth commented Nov 10, 2023

nixos-discourse commented Dec 19, 2023

roberth commented May 12, 2023 •

edited by tomberek

Loading

fricklerhandwerk left a comment •

edited

Loading

Ericson2314 commented Nov 7, 2023 •

edited

Loading