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

Use closureInfo for building system tarballs and Docker container #49855

Merged
merged 4 commits into from Nov 26, 2018

Conversation

dingxiangfei2009
Copy link
Contributor

Motivation for this change

Since nix 2.0 is available, we can populate the nix store in the tarball with proper hashes. With these changes, building docker image is updated.

Note that previously the docker image does not contain the complete filesystem layout. Now the full filesystem is copied over.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@joachifm
Copy link
Contributor

I think this PR at least partially overlaps with #49414

@nlewo
Copy link
Member

nlewo commented Nov 12, 2018

I don't think it overlaps with #49414 since the PR #49414 simplifies the dockerTools.buildImageWithNix function which is not used in the context of this PR.

@joachifm
Copy link
Contributor

Ah, I just saw docker image and closureinfo were mentioned both places :) So this is good to go in your opinion, then?

@@ -14,8 +13,6 @@ stripSlash() {
if test "${res:0:1}" = /; then res=${res:1}; fi
}

touch pathlist
Copy link
Member

Choose a reason for hiding this comment

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

Do you know why this was there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Original nixos/lib/make-system-tarball.sh does not have any further reference to pathlist. However, I have a theory that this script could have been written in tandem with make-iso9660-image.sh, since the function names and variables are very similar.

@nlewo
Copy link
Member

nlewo commented Nov 13, 2018

@dingxiangfei2009 It seems there are some changes that are not related to the commit message (the Docker FS layout for instance). This makes your commit harder to review.

  • How could I test this changes?
  • I also wonder how you use this Docker profile:/
  • Did you check the cd-dvd build is still working?

@dingxiangfei2009
Copy link
Contributor Author

@nlewo
Here is how I tested this.
default.nix:

let
  nixos = import <nixpkgs/nixos> {
    configuration = ./configuration.nix;
    system = "x86_64-linux";
  };
in
nixos.config.system.build.tarball

configuration.nix:

{ pkgs, config, lib, ... }:
{
  imports = [
    <nixpkgs/nixos/modules/virtualisation/docker-image.nix>
    <nixpkgs/nixos/modules/installer/cd-dvd/channel.nix>
  ];

  documentation.doc.enable = false;

  environment.systemPackages = with pkgs; [
    bashInteractive
    cacert
    nix
  ];
}

Then running nix-build against default.nix gives you a tarball.
To load into docker, run docker import result/tarball/nixos-system-*.tar.xz nixos-docker.
If you do docker run --privileged -it nixos-docker /init, you will see systemd starting up.
Then you can log into this container through docker exec -it <container-name> /run/current-system/sw/bin/bash.

CD/DVD builds are still working. In fact, currently only the Docker profile uses make-system-tarball. 😮

@Mic92
Copy link
Member

Mic92 commented Nov 23, 2018

Have you also booted into the resulting dvd? Maybe one our tests covers that.

@Mic92
Copy link
Member

Mic92 commented Nov 23, 2018

You example is useful. I added it to the module itself.

@dingxiangfei2009
Copy link
Contributor Author

@Mic92 Yes. I built a x86_64 image. It boots in the virtual machine and installs NixOS. So the CD/DVD installers are still working.

@Mic92 Mic92 merged commit dd32831 into NixOS:master Nov 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants