What about calling this contents? It seems to me it behaves like the contents attribute of our other docker tools.

Why not just adding extraPackages in the contents list?

Hm it's not the same thing: contents are added to the / path, but extraPackages are things that are added to PATH (and other env vars) later by stdenv's setup-hook. So by using nativeBuildInputs all kinds of dependencies should work, though now I'm debating whether that's even needed. Maybe it's needed for man support or so though.

IIUC, this is to allow a user to add extra tooling in its container. I think, in other images, this use case is covered by the contents attribute, even if it is less convenient than what you are doing (because enviroment variables are not set). In order to have a coherent dockerTools API, I would prefer to expose the dockerTools.buildLayeredImage contents and env attributes: if the user needs extra tooling, it would be still possible to add packages with contents and set some environment variables with env. WDYT?

Thinking about this again, I feel like extraPackages should be removed, because people can just use .overrideAttrs on the derivation they pass in instead.

Similarly, I don't think an env is needed, because this can also be achieved by setting that in the derivation in multiple ways already:

{
  VAR = "VAL";
  shellHook = ''
    export VAR=VAL
  '';
}

I think the idea of passing through contents is a good one though, I'll change it as such

This is different from the bash image, which has this entrypoint https://github.com/tianon/docker-bash/blob/master/5.1/docker-entrypoint.sh

When I try this in a normal session I get

$ bash --login echo foo
/run/current-system/sw/bin/echo: /run/current-system/sw/bin/echo: cannot execute binary file

Which seems to be a cryptic way of telling us that it's refusing to interpret an ELF executable as a script.

Passing a script does work

$ bash --login <(echo "echo hello") asdf
hello

So I think it should be like the official bash entrypoint.

The Cmd should probably be ["bash"].

We'll need a test for this. You can add cases to nixos/tests/docker-tools.nix, based on a new example you can add to pkgs/build-support/docker/examples.nix.

Not sure I get the motivation. nix-shell is bash-based, but it's not an image for the purpose of executing bash. In this case, we need some way to source stdenv and execute shellHook when it starts, which I'm now doing by passing --login and writing the shell init to /etc/profile. Currently this allows you to either do docker run -it <image>, then run genericBuild, or you can also do

docker run -it <image> -c genericBuild

And this works, which I don't think would be the case with your suggestion.

I agree we seem to have different use cases in mind, but we can probably support both.

• use the nix-shell image to build something
• use the nix-shell image as a developer environment

For building, we should invoke the builder with args and not wrongly assume bashInteractive.
For interactive use, I do think we should emulate the official bash image behavior.

Can't we support both?

I now changed it to have no entrypoint at all, however there's bash, all of coreutils and a special buildDerivation binary in the PATH. In addition, the Cmd is set to [ "bash" ]. I managed to get around the --login requirement by using ~/.bashrc instead. This means by default you'll get a nix-shell environment, but you can also do any of

$ docker run -it <image> # nix-shell
$ docker run -it <image> bash # nix-shell
$ docker run -it <image> buildDerivation # build the derivation directly
$ docker run -it <image> bash -c 'buildDerivation && ls $out/bin' # builds derivation and lists the binaries it produced

So we're basically emulating the nix sandbox env setup here. Structured attrs and passAsFile are missing, but that's probably ok for a start.

Yeah, didn't want to bother with structuredAttrs, but I guess I can issue a warning when it's enabled

An assertion now triggers when __structuredAttrs is set. The current code can't handle __structuredAttrs at all really.

What about passAsFile though?

Yeah that didn't work before, but I implemented support for it now :)

extraInit seems like a rather arbitrary term that doesn't have a well-understood purpose. Can't we remove it and rely on the user extending shellHook beforehand, or setting an appropriate Cmd?

cd /home breaks the WorkingDir config item.

Yeah I'm removing extraInit in the next iteration. Can also be done with just shellHook.

WorkingDir can't be configured in the current version anyways

With docker you can always configure any config attribute afterwards.

Now using WorkingDir directly to specify this, instead of cd ing

Are these paths writable in the store?
We should have a test case for this as well.

It would be nice to have symlinks to these paths in a fixed location for derived images to use.

They are indeed writable. Perhaps I could create /home/user/outputs/* again, symlinking into the store

Now created symlinks from /home/user/outputs/* into the /nix/store paths

I want to override some settings in this config. I also want to use streamLayeredImage. So really it makes sense to just return the attrset directly and let the user decide where the image is actually realized.

You could use (buildLayeredImage args).stream to solve that part of the problem.
I feel the increasing number of interfaces for building docker images is starting to become a problem, so I'm not excited about having a function that returns arguments for another function. I'd like to experiment with a more powerful interface during Ocean Sprint, so I'll know more in a month.

This should probably be easily overridable. I want to override it in my use case as I don't want to only be able to run containers interactively. Using shellHook to run a task means it only works when .bashrc is loaded which means it must be interactive.

Is that file not yet converted to markdown?