Skip to content
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

mkShellMinimal: Create an ultra minimal nix-shell #132617

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fzakaria
Copy link
Contributor

@fzakaria fzakaria commented Aug 4, 2021

Motivation for this change

This shell is inspired some investigation I did
and a follow-up example that demonstrated starting a nix-shell with a minimal builder.sh.

The goal is to provide the facility to create an ultra-lightweight nix-shell that only has bash as the direct dependency. Users can set environment variables and add packages but the feature-set is purposely minimal.

The following Nix expression works and is pointing to the latest commit of this PR

let
  nixpkgs = import (builtins.fetchTarball {
    name = "nixpkgs-fzakaria-fork-2021-08-03";
    url =
      "https://github.com/fzakaria/nixpkgs/archive/refs/heads/faridzakaria/pure-mkShell.tar.gz";
    sha256 = "1h7aqlivq2hax7k7a612mffpfkawn1mfd3jfszzpx19i2fxwmqp2";
  }) { };
in
with nixpkgs;
mkShellMinimal {
  name = "my-test-shell";
  packages = [ ripgrep ];
  FOO = bar;
}

This minimal nix-shell has effectively a 0 (1.4 KiB) size closure.

❯ nix path-info -rSsh $(nix-build -I nixpkgs=/home/fmzakari/code/github.com/NixOS/nixpkgs test-shell-local.nix)
these derivations will be built:
  /nix/store/xaqxyk1y3zjk0qwpxkm0n3n6c05bj6fn-setup.drv
  /nix/store/rxrbwvk7rjvknil7ns1ki00n4vfqcs8h-my-test-shell.drv
building '/nix/store/xaqxyk1y3zjk0qwpxkm0n3n6c05bj6fn-setup.drv'...
building '/nix/store/rxrbwvk7rjvknil7ns1ki00n4vfqcs8h-my-test-shell.drv'...

This derivation is not meant to be built, unless you want to capture the dependency closure.

/nix/store/8ka1hnlf06z3h2rpd00b4d9w5yxh0n39-setup        	 376.0 	 376.0 
/nix/store/nprykggfqhdkn4r5lxxknjvlqc4qm1yl-builder.sh   	 280.0 	 280.0 
/nix/store/xd8d72ccrxhaz3sxlmiqjnn1z0zwfhm8-my-test-shell	 744.0 	   1.4K
Things TODO
  • I am still missing the necessary hook to clear $PATH when nix-shell is started pure. This exercise has actually shown how tied the nix-shell and stdenv concepts are.
  • Add support for inputDerivation similar to what @infinisil added for mkDerivation
  • calculate the new closure-size (should be 0?) for the minimal shell.
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Relase 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.

@fzakaria fzakaria force-pushed the faridzakaria/pure-mkShell branch 2 times, most recently from e251480 to 42ce8dd Compare August 4, 2021 04:37
@Mic92 Mic92 requested a review from zimbatm August 4, 2021 04:38
@fzakaria fzakaria force-pushed the faridzakaria/pure-mkShell branch 2 times, most recently from 8c9ce8e to 9001dc0 Compare August 4, 2021 04:43
@Mic92
Copy link
Member

Mic92 commented Aug 4, 2021

There is a section for special builders in nixpkgs doc nixpkgs/doc/builders/special/mkshell.section.md
This one could be added as well.

Copy link
Contributor

@tomberek tomberek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the goal? If to be minimal is there a path to use busybox instead of bash?

pkgs/build-support/mkshell/minimal.nix Outdated Show resolved Hide resolved
pkgs/build-support/mkshell/minimal.nix Outdated Show resolved Hide resolved
@fzakaria
Copy link
Contributor Author

fzakaria commented Aug 4, 2021

What is the goal? If to be minimal is there a path to use busybox instead of bash?

In practice I found we were using nix-shell to have a reproducible environment, but stdenv was bringing and doing a lot of stuff. I just wanted the ability to declare environment variables & a list of packages.
(Might need to expand slightly to setup-hooks for setting stuff like language paths like bundler or Java's classpath)

I want to showcase an ultra-minimal mkShell (in fact the real mkShell should probably be built on-top of this), where one lists only a set of packages.

The whole notion of buildInputs vs. propagatedBuildInputs is unnecessary is you are writing a shell. You are never planning to build the derivation.

@fzakaria fzakaria force-pushed the faridzakaria/pure-mkShell branch 2 times, most recently from 591435f to b6ac56e Compare August 4, 2021 05:06
@fzakaria
Copy link
Contributor Author

fzakaria commented Aug 4, 2021

There is a section for special builders in nixpkgs doc nixpkgs/doc/builders/special/mkshell.section.md
This one could be added as well.

I will work on documentation once I get a few more 👀 on this. I have a few TODOS I added to the description that I still need to solve (if you have hints please share).

@fzakaria fzakaria changed the title minimalMkShell: Create an ultra minimal nix-shell mkShellMinimal: Create an ultra minimal nix-shell Aug 4, 2021
@siraben
Copy link
Member

siraben commented Aug 4, 2021

@fzakaria thanks for working on this!

@tomberek a more minimal shell would be good for packages that don't need the full closure of stdenv, which has occurred for me while working on #123095. Maybe one based on busybox instead of bash would be even more minimal indeed. See #116274 for an stdenv based on uutils-coreutils.

pkgs/build-support/mkshell/minimal.nix Outdated Show resolved Hide resolved
pkgs/build-support/mkshell/minimal.nix Outdated Show resolved Hide resolved

for a in $packages; do
export PATH=$a/bin:$PATH
done
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll also want to set XDG_DATA_DIRS, because that's what bash uses to find completion scripts on demand. Stdenv sets it generically since #103501, which inspired devShell to do the same.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It comes at a very very little cost, but does enable shell completions.

If that's not the design choice you want to make, I wonder if it should even set PATH. Maybe someone only wants to set another environment variable, in which case PATH is redundant.

I guess it boils down to the question "when is it not a shell anymore?" which is obviously not clearly delineated, but needs an answer for this to be actually minimal.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, in what regard is it minimal? I would define it as the combination of the number of store paths and the total byte size. The latter is generally so disproportionally determined by dependencies that the size of the shellHook (or similar) does not matter, as long as it doesn't increase store path count.
I do consider store path count separately because it introduces latency with nix-copy-closure and most binary caches.

Copy link
Member

@Mic92 Mic92 Sep 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say minimal with regards to closure size but from that we could support features to make the whole thing practical usable i.e. set MANPATH.

pkgs/build-support/mkshell/minimal.nix Outdated Show resolved Hide resolved
pkgs/build-support/mkshell/minimal.nix Show resolved Hide resolved
Comment on lines 38 to 43
builder = writeScript "builder.sh" ''
#!${bash}/bin/bash
echo
echo "This derivation is not meant to be built, aborting";
echo
exit 1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@infinisil I actually don't see a particular reason why nix-shells can't be buildable.
I wonder if doing your export trick here to $out is enough + adding an error message.

@fzakaria
Copy link
Contributor Author

fzakaria commented Aug 4, 2021

@siraben since there's no dependency on coreutils (merely bash); I like that this in spirit allows the user to choose busybos or coreutils.

I think keeping the sole dependency to bash more in spirit to this minimalism otherwise the choice will no longer be possible?
There might be an opportunity to remove bash completely and solely rely on /bin/sh which must exist on the system ?

@roberth
Copy link
Member

roberth commented Aug 4, 2021

nix-shell will start the bash on the derivation's PATH, so you'd need some trickery to start a different shell. Such trickery isn't minimal and it's unexpected, so I'd leave it up to the user if they really feel like they need it.

Please don't use /bin/sh. It's only intended for programs that make calls to the C system function and such.

@fzakaria
Copy link
Contributor Author

fzakaria commented Aug 4, 2021

@roberth makes sense.
Looks like the size of bash vs. busybox is the same mostly due to the size of glibc's locales.
There's an opportunity to use a version of bash with an overridden glibc that only has UTF8 as the supported LOCALE.
(should dramatically reduce the minimal size)

@roberth
Copy link
Member

roberth commented Aug 4, 2021

The user should choose their own bash implementation if they don't want to use the impure PATH's bash, so optimizing it is another topic.

Did you mean to use /bin/sh in the builder? That's not so bad, because that's not used for the actual shell and the builder logic is, well, super simple.

@fzakaria
Copy link
Contributor Author

fzakaria commented Aug 4, 2021

@roberth i did mean it for the builder since the expectation is that it's run on a POSIX compliant machine and for the purpose of having a development environment.

@fzakaria
Copy link
Contributor Author

fzakaria commented Aug 4, 2021

Woot!
New closure size is merely 1.4KiB

❯ nix path-info -rSsh $(nix-build -I nixpkgs=/home/fmzakari/code/github.com/NixOS/nixpkgs test-shell-local.nix)
these derivations will be built:
  /nix/store/xaqxyk1y3zjk0qwpxkm0n3n6c05bj6fn-setup.drv
  /nix/store/rxrbwvk7rjvknil7ns1ki00n4vfqcs8h-my-test-shell.drv
building '/nix/store/xaqxyk1y3zjk0qwpxkm0n3n6c05bj6fn-setup.drv'...
building '/nix/store/rxrbwvk7rjvknil7ns1ki00n4vfqcs8h-my-test-shell.drv'...

This derivation is not meant to be built, unless you want to capture the dependency closure.

/nix/store/8ka1hnlf06z3h2rpd00b4d9w5yxh0n39-setup        	 376.0 	 376.0 
/nix/store/nprykggfqhdkn4r5lxxknjvlqc4qm1yl-builder.sh   	 280.0 	 280.0 
/nix/store/xd8d72ccrxhaz3sxlmiqjnn1z0zwfhm8-my-test-shell	 744.0 	   1.4K

@andir
Copy link
Member

andir commented Aug 4, 2021

Do you also plan to support inputsFrom (and potential other features that I am unaware of) from mkShell?

@fzakaria
Copy link
Contributor Author

@zimbatm any interest in contributing to this or merging the two into nixpkgs?
I think there's room here to make progress.

We can chat over Matrix or something. Cheers.

@fzakaria fzakaria requested a review from andir September 17, 2021 19:55
@fzakaria
Copy link
Contributor Author

Ping for others here.

@roberth
Copy link
Member

roberth commented Sep 18, 2021

Looks like you missed this review item https://github.com/NixOS/nixpkgs/pull/132617/files#r682575668

@colemickens
Copy link
Member

This is nice to see, I just recently copied a version of this into my repo after reading the related blog post. A thought, related to some open questions:

Could this either be (1) more flexible, allowing the user to opt-in to things like completions, or (2) be the basis of a number of functions, similar to some of the pkgs/trivial functions that often minimally build upon each other?

Along those lines, if this is added to nixpkgs, the API sort of immediately becomes somewhat frozen. Not sure if @zimbatm and @fzakaria chatted? Is there a ton of benefit to this being in nixpkgs vs outside? With flakes and an external repo, one would be free to alter the API, basically at any time.

@fzakaria
Copy link
Contributor Author

I'd love to collaborate to continue to make this composable.
I envision a way where you "stack" features onto the minimalShell setup for various features you want.
(Stdenv.mkDerivation would be just a prebuilt stack)

Didn't get a chance to chat more with @zimbatm more about it.

@erikarvstedt
Copy link
Member

I'd like to see this included in nixpkgs. mkShell is not fit to use as a general purpose env builder due to the number of irrelevant env vars. @zimbatm, would you join this effort?

Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
@asymmetric
Copy link
Contributor

@fzakaria are you still planning on working on this? I would be interested in seeing it land in nixpkgs.

@roberth
Copy link
Member

roberth commented Mar 7, 2022

I'm increasingly convinced that development shells should be started through nix run, giving complete control to expressions that can be pinned, rather than relying on fragile, complicated built-in behavior that is inherently hard to improve upon.

It's a technically and organizationally superior solution, but the hacky nix-shell solution has a dedicated subcommand and a decade of usage :'(

@asymmetric
Copy link
Contributor

@roberth do I understand correctly that your
concern is about how this is run, rather than how it’s implemented? If so, it should be orthogonal to this PR?

@roberth
Copy link
Member

roberth commented Mar 7, 2022

@roberth do I understand correctly that your concern is about how this is run, rather than how it’s implemented? If so, it should be orthogonal to this PR?

My concern is that this PR uses the wrong tool to achieve its goal, as nix-shell is too complicated to host a minimal shell. Note that nix-shell is a hack of all trades but master of none, as evidenced by the many nix issues that want to pull it either in the direction of nix run or the actual real build sandbox.
It is certainly not orthogonal, but I won't oppose incremental changes like this PR, as long as you're making the conscious choice to go for a messy local optimum rather than a principled solution.

@colemickens
Copy link
Member

I'm not sure how to make sense of these thoughts given that I use mkMinimalShell with nix develop (I don't really know the internal differences between nix-shell/nix run/nix develop. How does that compare to nix run?

@tomberek
Copy link
Contributor

tomberek commented Mar 7, 2022

I'm not sure how to make sense of these thoughts given that I use mkMinimalShell with nix develop (I don't really know the internal differences between nix-shell/nix run/nix develop. How does that compare to nix run?

Roughly:

  1. nix run: just run the installable using an absolute path.
  2. nix shell: set the PATH to the "bin" of the installable (and from /nix-support/propagated-user-env-packages) and start a sub-shell with "$SHELL" or "bash" using normal /etc/profile or bashrc or zshrc initialization.
  3. nix develop: set a ton of variables that would be present during a build. start a sub-shell using a bootstrap rcfile that calls the setupHook for the derivation you are using.
  4. nix-shell: various things

nix run can simulate the others as it can run some arbitrary script

@fzakaria
Copy link
Contributor Author

fzakaria commented Mar 8, 2022

The discussion and PR i would like to continue to push forward is breaking down mkShell in composable units.
I was trying to setup the barest (smallest) unit first to start and everything else should compose on top of it.

@roberth I don’t think anything in mkMinimalShell goes against what you are saying. It’s creating a shell to inherit all environment variables. I agree I would like to see less specified in nix Itself for defining this stuff.

Thoughts?

@roberth
Copy link
Member

roberth commented Mar 8, 2022

@roberth I don’t think anything in mkMinimalShell goes against what you are saying.

My only concern is that it may not achieve its end goal, but that doesn't mean that it doesn't have value. As I said, I won't oppose incremental changes like this PR.

barest (smallest) unit first to start and everything else should compose on top of it.

This is a good goal, but you'll be duplicating significant parts of "stdenv". We already have a NixOS way of configuring the environment and a stdenv/setup/mkDerivation way of configuring environments. Will this create a third way of doing things?

Perhaps a better angle for implementing this is to modularize stdenv. We have stdenvNoCC which, as the name suggests, does not include the C compiler. Similarly, you could define more minimal "env"s. I'd stay away from negations like "NoCC" and construct ones that are minimal with respect to some goal. Maybe you could have setupenv which relies on an impure environment, but runs setup.sh; shenv which builds on that but makes the shell pure; unixenv which adds a small number of programs used in scripts; busyboxenv, etc.
This feels like bootstrapping, but I don't know if these distinctions/layers are useful for that. As far as I'm concerned, these envs' dependencies should all be built with the usual stdenv, to keep things simple.

By using those minimal envs, you can modularize shells using the same functionality that is used for builds, such as the setup hooks, and you don't have to introduce a third way of doing things.

Copy link
Contributor

@asymmetric asymmetric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation suggestions.

Comment on lines +14 to +15
with nixpkgs;
mkShellMinimal {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this super-broad use of with from the doc? It doesn't seem to add anything over

Suggested change
with nixpkgs;
mkShellMinimal {
pkgs.mkShellMinimal {


```nix
let
nixpkgs = import <nixpkgs> { };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
nixpkgs = import <nixpkgs> { };
pkgs = import <nixpkgs> { };

Since you use pkgs.mkShellMinimal above (and is more idiomatic IMO)

@fzakaria
Copy link
Contributor Author

fzakaria commented Mar 8, 2022 via email

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 21, 2022
@infinisil infinisil added the significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. label Apr 19, 2023
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 19, 2023
@johnrichardrinehart
Copy link
Contributor

@asymmetric Are you still requesting changes? I'd like @fzakaria to be able to merge this, if possible.

@asymmetric
Copy link
Contributor

@johnrichardrinehart I don't think my review is the main thing blocking this (but @fzakaria please do let me know if that's the case). There are conflicts that must be resolved.

But on the topic of my review: I especially think we shouldn't advocate the "global" with in the manual. There were some issues explaining its problems but I can't find them now.

@roberth
Copy link
Member

roberth commented Sep 28, 2023

A truly minimal shell would not have the form of a derivation, however this requires changes in the Nix CLI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict 8.has: documentation 10.rebuild-darwin: 0 10.rebuild-linux: 0 significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet