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.

Sign up for GitHub

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

Support fetching docker images from V2 registries #32248

Merged
merged 10 commits into from
Mar 4, 2018
Merged

Support fetching docker images from V2 registries #32248

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2586568
docker: init fetchdocker nix code for docker2nix
ixmatus Dec 2, 2017
c1eb962
fetchdocker: Integration test exercising hocker and fetchdocker
ixmatus Dec 2, 2017
6c9de3f
hocker: Don't check package and wrap the binaries with PATH to nix
ixmatus Dec 2, 2017
a6cdf0a
Merge branch 'master' into parnell/fetchdocker
ixmatus Dec 2, 2017
e4ec980
Merge remote-tracking branch 'upstream/master' into parnell/fetchdocker
ixmatus Jan 10, 2018
bd9869d
Merge branch 'master' into parnell/fetchdocker
ixmatus Jan 24, 2018
c2a5f68
Merge branch 'master' into parnell/fetchdocker
ixmatus Jan 30, 2018
6f95cb1
Merge branch 'master' into parnell/fetchdocker
ixmatus Feb 11, 2018
0a603ee
Merge remote-tracking branch 'upstream/master' into parnell/fetchdocker
ixmatus Feb 13, 2018
58895aa
Merge branch 'parnell/fetchdocker' of github.com:awakesecurity/nixpkg…
ixmatus Feb 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions nixos/tests/hocker-fetchdocker/machine.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
{ config, pkgs, ... }:
{ nixpkgs.config.packageOverrides = pkgs': {
hello-world-container = pkgs'.callPackage ./hello-world-container.nix { };
haskellPackages = pkgs'.haskellPackages.override {
overrides = new: old: {
hocker = pkgs'.haskell.lib.dontCheck old.hocker;
};
};
};

virtualisation.docker = {
Expand Down
34 changes: 34 additions & 0 deletions pkgs/development/haskell-modules/configuration-common.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,4 +984,38 @@ self: super: {
};
}).override { language-c = self.language-c_0_7_0; };

hocker =
overrideCabal
# Not checking because it's failing on a test that needs a data
# file not included in its source distribution; this will be
# removed when that is fixed
( dontCheck super.hocker )
( oldDerivation: {
testToolDepends =
(oldDerivation.testToolDepends or []) ++[ pkgs.nix ];
Copy link
Member

Choose a reason for hiding this comment

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

I guess the package should declare its dependency on Nix, i.e. by depending on http://hackage.haskell.org/package/nix-paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only utility from the hocker package that depends on Nix (other than the tests) is docker2nix; I think that's clear enough for now. I agree that a build-time dependency on Nix is better than a run-time dependency, so thank you for pointing me at nix-paths. Here are my reasons why I don't think we should gate this PR on that, though:

  • I don't entirely understand how nix-paths works; it looks like it has magic constants in it but I don't understand where those come from; a minimal README.md for the project would help a lot I think (note that if you explain it all to me, I'm also happy to submit a PR with a README.md contribution)
  • The utility that we need for docker2nix is nix-hash, which is not provided by nix-paths. I can submit a PR to fix that but I don't think we should gate this PR on that and I also want to understand how nix-paths works first before contributing more functionality

Copy link
Member

Choose a reason for hiding this comment

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

http://hackage.haskell.org/package/nix-paths-1.0.1 adds support for nix-hash. The paths are determined are configure-time by Cabal. Look at Setup.lhs for the implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for adding support for nix-hash @peti. I'll probably get to that soon. In the meantime can I get a review of the other parts of this PR?

I don't think using nix-paths is a blocker for this PR but if it is, could you say so?

Copy link
Member

Choose a reason for hiding this comment

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

I would not call the hocker-related overrides a blocker. That's too strong a term, IMHO. What I can say though is that I won't merge the PR as-is. Other Nixpkgs maintainers may very well feel differently about it.

Copy link
Contributor Author

@ixmatus ixmatus Dec 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Okay, thanks for clarifying!

I'd like to clarify my understanding of nix-paths (would you accept a PR adding a README.md with anything I learn? That may help newcomers and provide a reference to point people at).

I see now that Setup.lhs discovers the absolute path to, say, nix-hash using lookupProgram which is great because if nix-hash is not available, it becomes a compile-time error and not a run-time error (I think this is great, btw, I wish this were more easily discovered!) Doing this removes the need for the wrapProgram invocations but it does require that nix-paths is included in the buildInputs of hocker's derivation (as a build-time and run-time dependency). Do I have this right?

Additionally, I think the generated derivation for the nix-paths hackage package only includes the Nix package in the nativeBuildInputs; shouldn't it be in the propagatedBuildInputs so that the same Nix used as a build-time dependency is included in the run-time closure of any program that depends on nix-paths?

Also, the generated derivation for the nix-paths hackage package restricts the hydra build to Linux: hydraPlatforms = [ "i686-linux" "x86_64-linux" ]; what's the reason for this? hocker is usable on Darwin systems, will the hydraPlatforms line prevent hocker from being integrated on a Darwin worker?

buildDepends =
(oldDerivation.buildDepends or []) ++ [ pkgs.makeWrapper ];

postInstall =
(oldDerivation.postInstall or "") + ''
# Globbing for hocker-* fails with: Builder called die:
# makeWrapper doesn't understand the arg /nix/store/rsic1v6y6v63q6lkmpn3xmn7cnzx8irk-hocker-1.0.2/bin/hocker-image
wrapProgram $out/bin/hocker-image \
--suffix PATH : ${pkgs.nix}/bin

wrapProgram $out/bin/hocker-layer \
--suffix PATH : ${pkgs.nix}/bin

wrapProgram $out/bin/hocker-config \
--suffix PATH : ${pkgs.nix}/bin

wrapProgram $out/bin/hocker-manifest \
--suffix PATH : ${pkgs.nix}/bin

wrapProgram $out/bin/docker2nix \
--suffix PATH : ${pkgs.nix}/bin
'';
}
);

}