runInMkShell: init

[lucasew]

Description of changes

This is based on code from dockerTools.streamNixShellImage in the hope that the same primitives could be generalized to other runtimes such as bundled executables, appimages, OCI containers and Singularity images.

I am not an expert in this domain tho but I had to adapt dockerTools.streamNixShellImage to use in production, also with guibou's nixGL so the GPUs get recognized.

As of dockerTools.streamNixShellImage, mkShellWrapper is based on mkShell but instead of generating an image it generates a script, like a writeShellScript that one can use directly in the entrypoint then pass the command to run inside the environment as parameters. All the nix-shell goodness should hopefully work here.

BTW drv can be either a derivation generated by mkShell or a attrset with mkShell parameters.

I am testing with the following snippet and can confirm it works:

lucasew@riverwood ~/WORKSPACE/OPENSOURCE-contrib/nixpkgs 0$ nix repl
nix-repl> :lf .
Added 17 variables.                                    

nix-repl> :b legacyPackages.x86_64-linux.mkShellWrapper { drv = { buildInputs = [ legacyPackages.x86_64-linux.python3Packages.numpy ];};}
warning: Nix search path entry '/nix/var/nix/profiles/per-user/root/channels' does not exist, ignoring

This derivation produced the following outputs:
  out -> /nix/store/5s379y8vgl8ymx5a4dcx60z9xj6lb2v4-nix-shell-wrapper

lucasew@riverwood ~/WORKSPACE/OPENSOURCE-contrib/nixpkgs 0$ /nix/store/5s379y8vgl8ymx5a4dcx60z9xj6lb2v4-nix-shell-wrapper python
Python 3.10.12 (main, Jun  6 2023, 22:43:10) [GCC 12.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import numpy as np
>>> dir(np)
['ALLOW_THREADS', 'BUFSIZE', 'CLIP', 'DataSource', 'ERR_CALL', 'ERR_DEFAULT', 'ERR_IGNORE', 'ERR_LOG', 'ERR_PRINT', 'ERR_RAISE', 'ERR_WARN', 'FLOATING_POINT_SUPPORT', 'FPE_DIVIDEBYZERO', 'FPE_INVALID', 'FPE_OVERFLOW', 'FPE_UNDERFLOW', 'False_', 'Inf', 'Infinity', 'MAXDIMS', 'MAY_SHARE_BOUNDS', 'MAY_SHARE_EXACT', 'NAN', 'NINF', 'NZERO', 'NaN', 'PINF', 'PZERO', 'RAISE', 'RankWarning', 'SHIFT_DIVIDEBYZERO', 'SHIFT_INVALID', 'SHIFT_OVERFLOW', 'SHIFT_UNDERFLOW', 'ScalarType', 'True_', 'UFUNC_BUFSIZE_DEFAULT', 'UFUNC_PYVALS_NAME', 'WRAP', '_CopyMode', '_NoValue', '_UFUNC_API', '__NUMPY_SETUP__', '__all__', '__builtins__', '__cached__', '__config__', '__deprecated_attrs__', '__dir__', '__doc__', '__expired_functions__', '__file__', '__former_attrs__', '__future_scalars__', '__getattr__', '__git_version__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__version__', '_add_newdoc_ufunc', '_builtins', '_distributor_init', '_financial_names', '_get_promotion_state', '_globals', '_int_extended_msg', '_mat', '_no_nep50_warning', '_pyinstaller_hooks_dir', '

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.11 Release Notes (or backporting 23.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[otavio]

Very nice. I found a minor issue in the env attribution. Please take a look.

[Artturin]

Needs tests

[thiagokokada]

Also documentation.

[drupol]

Nice addition, if I understand correctly, this is an abstraction of https://github.com/NixOS/nixpkgs/blob/master/pkgs/shells/fish/wrapper.nix but for any shell? Right?

[lucasew]

Needs tests

Where should I put them?

Nice addition, if I understand correctly, this is an abstraction of https://github.com/NixOS/nixpkgs/blob/master/pkgs/shells/fish/wrapper.nix but for any shell? Right?

IDK exactly. The idea here is to generate a wrapper script that runs the passed command inside a nix-shell like environment.

[Artturin]

Needs tests

Where should I put them?

In the same dir, check how the fetchers in pkgs/test/default.nix do it,
and check what they do in all-packages.nix to add the tests to the functions

[lucasew]

Needs tests

Where should I put them?

In the same dir, check how the fetchers in pkgs/test/default.nix do it, and check what they do in all-packages.nix to add the tests to the functions

Got it. Thank you.

Next step: documentation.

[lucasew]

Added docs, rendered locally and it seems everyting is fine.

EDIT 1: Docs are much better to work with now, congratulations to the ones involved!

[nixos-discourse]

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/2677

[Artturin]

I think there should be a better name for this, when I first saw this PR I thought it was a function to wrap shell's like bash and zsh.

Maybe runInMkShell

There's runInLinuxImage and runInLinuxVM in vmTools

[lucasew]

Well. I suggested that actually because the name is like mkShell. I also added the documentation for it right after mkShell because of SEO, discoverability.

I already did this kind of SEO many times like when I added a section in Hugo docs and opened issues while I was troubleshooting stuff and closed them right after with the solution I found so that would get better SEO and reach more people.

[lucasew]

changed to runInMkShell

[Artturin]

Would be better to just have users pass a drv always, and rename the argument to shellDrv.

[lucasew]

This is the reference behaviour but as most of the usecases are just passing packages in or passing the output of mkShell I think that is useful to support these usecases.

The drv name I got from the function that it's based on (dockerTools.streamNixShellImage).

[roberth]

This idea fits the direction Nix+Nixpkgs should take; see comment.

I hope you can refactor it to share code for with the dockerTools function.
If done later, it might feel like runInMkShell is hard to change.

[roberth]

I would like this to be the other way around. Currently Nix has suboptimal, yet ossified logic to implement "shells" because it couples with Nixpkgs stdenv. By flipping this around and making this function the default nix shell implementation, we could solve that architectural problem and let the behavior of nix-shell/nix develop evolve in a future proof way by pinning/locking Nixpkgs.
This would happen through an indirection such as a pkg.devShell attribute, e.g.

• POC: devShell interface #206728

This idea was received quite positively by the Nix team (even though we don't have an approved issue for it - I think we discussed it before the label).

It's fine to describe it this way until nix uses the devShell (or similarly named) attribute, so no change required here.

[roberth]

Not quite happy about the name.

• runIn is a prefix that was also used for runInLinuxVM, but this is not a derivation wrapper that runs the wrapped builder as part of its builder.
• MkShell would suggest that it's specific to that function, but it's more general.
• mk used to be for functions that are "data constructors"; functions that return their arguments in a specific, checked form.

Maybe not the best suggestion, but here's one:

Suggested change

	# pkgs.runInMkShell {#sec-pkgs-runInMkShell}
	# pkgs.buildShell {#sec-pkgs-buildShell}

[lucasew]

What about buildShellWrapper?

[lucasew]

What about buildShellWrapper?

[roberth]

Best to describe in terms of what it does. Something about a wrapper script that provides an environment similar to what Nix and stdenv provide.

[lucasew]

I explained a bit about the context of it's creation there.

[roberth]

It would be good to generate a script in $out/bin, "writeScriptBin style". That way the same function can produce for instance a file with just the environment variables for use in different shells (zsh, fish, ...), and potentially other files.
Splitting the executable and the bash library may also help with clarity.

Example files:

• $out/bin/shell
• $out/shell.sh
• $out/shell.env

.env could be done later. It's fine to limit this to bash usage for the time being.

I'd suggest not to call it $out/bin/build-shell etc, because that would suggest that the shell has to be closely align with some build, which it does not. These files form an interface that's more broadly applicable, including proper development shells that aren't necessarily about any specific build.

[lucasew]

Isn't $stdenv/setup already coupled with Bash?

BTW the generated script already checks if it's being sourced and don't run the entrypoint if yes.

[roberth]

checks if it's being sourced

Then you could symlink the .sh file to the executable.

My main point is that a directory can serve more purposes. For instance, shell.env could be the backend for nix print-dev-env, to support other shells, IDE extensions, ...
That's a bit more work to extract those variables, which is why I'm suggesting to do it later, but leave an "empty spot" for it in the output by making it a directory.

[roberth]

writeScriptBin mostly accomplishes that.

I don't want Nix to impose a requirement that the script must also be source-able or anything, which is why I lean towards having separate files for these purposes.
For instance we'll probably want to do something like setPersonality at some point, which seems like it will need a little C wrapper, which would not be natural to also support with the source-able executable pattern.

[lucasew]

How can I export the environment from inside the shell?

It kind of works using env but there are variables that span to multiple lines.

[lucasew]

I was thinking about using python to dump os.environ as json

[lucasew]

I was experimenting on only dumping /proc/self/environ.

It splits each variable definition with an \00 but bash is not behaving nicely when using that IFS feature in the run command.

[lucasew]

[lucasew]

got it

[lucasew]

That still didn't get multi line values

But this works nice: https://github.com/lucasew/playground/blob/master/shell/dump-env-from-proc

[lucasew]

I added another dockerTools function because in my experience that function is kind of broken for the usecase I was using it.

I wanted a behaviour kind of like happens with the official rclone container. After the image name you pass directly the arguments and by default you pass the command that would run inside that environment. That way the wrapper script would always run then use the rest of the arguments as the command itself to run.

Very simple to use and I don't need to break a function that is already being used somewhere.