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

Jump to bottom

Docker-related cross-compilation fixes #68900

Closed
wants to merge 4 commits into from
Closed

Docker-related cross-compilation fixes #68900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e0527ce
cni: add aarch64-linux as platform
nspin Sep 10, 2019
6176cf7
tini: fix cross-compilation
nspin Jan 25, 2019
12c5361
containerd: fix cross-compilation
nspin Sep 10, 2019
c59e9a0
docker: fix cross-compilation
nspin Sep 10, 2019
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
2 changes: 1 addition & 1 deletion pkgs/applications/networking/cluster/cni/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ buildGoPackage rec {
license = licenses.asl20;
homepage = https://github.com/containernetworking/cni;
maintainers = with maintainers; [ offline vdemeester ];
platforms = [ "x86_64-linux" ];
platforms = [ "x86_64-linux" "aarch64-linux" ];
};
}
3 changes: 2 additions & 1 deletion pkgs/applications/virtualization/containerd/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ buildGoPackage rec {

hardeningDisable = [ "fortify" ];

buildInputs = [ btrfs-progs go-md2man utillinux ];
nativeBuildInputs = [ go-md2man utillinux ];
buildInputs = [ btrfs-progs ];
buildFlags = "VERSION=v${version}";

BUILDTAGS = []
Expand Down
25 changes: 19 additions & 6 deletions pkgs/applications/virtualization/docker/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{ stdenv, lib, fetchFromGitHub, makeWrapper, removeReferencesTo, pkgconfig
{ stdenv, lib, fetchFromGitHub, fetchpatch, makeWrapper, removeReferencesTo, pkgconfig
, go-md2man, go, containerd, runc, docker-proxy, tini, libtool
, sqlite, iproute, lvm2, systemd
, btrfs-progs, iptables, e2fsprogs, xz, utillinux, xfsprogs
Expand Down Expand Up @@ -82,12 +82,22 @@ rec {
sha256 = sha256;
};

patches = [
# Replace hard-coded cross-compiler with $CC
(fetchpatch {
url = https://github.com/docker/docker-ce/commit/2fdfb4404ab811cb00227a3de111437b829e55cf.patch;
sha256 = "1af20bzakhpfhaixc29qnl9iml9255xdinxdnaqp4an0n1xa686a";
})
];

# Optimizations break compilation of libseccomp c bindings
hardeningDisable = [ "fortify" ];

nativeBuildInputs = [ pkgconfig ];
inherit (go.nativeDrv or go) GOOS GOARCH;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same issue with .nativeDrv here.


nativeBuildInputs = [ pkgconfig go-md2man go libtool removeReferencesTo ];
buildInputs = [
makeWrapper removeReferencesTo go-md2man go libtool
makeWrapper
] ++ optionals (stdenv.isLinux) [
sqlite lvm2 btrfs-progs systemd libseccomp
];
Expand Down Expand Up @@ -120,7 +130,7 @@ rec {
'';

# systemd 230 no longer has libsystemd-journal as a separate entity from libsystemd
patchPhase = ''
postPatch = ''
substituteInPlace ./components/cli/scripts/build/.variables --replace "set -eu" ""
'' + optionalString (stdenv.isLinux) ''
patchShebangs .
Expand Down Expand Up @@ -159,12 +169,15 @@ rec {
install -Dm644 ./components/cli/contrib/completion/zsh/_docker $out/share/zsh/site-functions/_docker

# Include contributed man pages (cli)
cd ./components/cli

'' + lib.optionalString (stdenv.hostPlatform == stdenv.buildPlatform) ''
# Generate man pages from cobra commands
echo "Generate man pages from cobra"
cd ./components/cli
mkdir -p ./man/man1
go build -o ./gen-manpages github.com/docker/cli/man
./gen-manpages --root . --target ./man/man1
'' + ''

# Generate legacy pages from markdown
echo "Generate legacy manpages"
Expand All @@ -183,7 +196,7 @@ rec {
'';

preFixup = ''
find $out -type f -exec remove-references-to -t ${go} -t ${stdenv.cc.cc} '{}' +
find $out -type f -exec remove-references-to -t ${go.nativeDrv or go} -t ${stdenv.cc.cc} '{}' +
Copy link
Contributor Author

@nspin nspin Sep 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

In the case of cross-compilation, go refers to the host derivation, but we need the build derivation. .nativeDrv is only present when we're cross-compiling (this is the result of an optimization in splice.nix).

There are many similar cases in nixpkgs where a native dependency is meant to be interpolated into a string. For example, grep for remove-references-to -t ${go}. As in the case above, this does not work when cross-compiling.

There are no explicit references to .nativeDrv like this in nixpkgs. Such explicit references don't feel like the right solution here, especially considering the fact that the .nativeDrv attribute is unstable, and will someday be renamed to pkgBuildHost. Should we add library functions to wrap explicit access?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Related: #49526 (comment) and #68895

Copy link
Member

Choose a reason for hiding this comment

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

I think buildPackages.go would be better here? /cc @Ericson2314

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Especially for removing references, the go interpolated into that string must match the one used as a nativeBuildInput. So, buildPackages.go would have to replace each instance of go in that file. The drawback of this approach is that go becomes harder to override for this package.

Copy link
Member

Choose a reason for hiding this comment

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

Yes that is a the drawback, but splicing is evil in using things like builtins.tryEval and still falling over itself when the packages are too different. It's better to use buildInputs.go, I'm afraid. If you really want, you could do goForBuild = buildPackages.go in the override list or something and use that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that this is the best solution in sight right now.

'' + optionalString (stdenv.isLinux) ''
find $out -type f -exec remove-references-to -t ${stdenv.glibc.dev} '{}' +
'';
Expand Down
3 changes: 2 additions & 1 deletion pkgs/applications/virtualization/tini/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ stdenv.mkDerivation rec {
"-DPR_GET_CHILD_SUBREAPER=37"
];

buildInputs = [ cmake glibc glibc.static ];
nativeBuildInputs = [ cmake ];
buildInputs = [ glibc glibc.static ];

meta = with stdenv.lib; {
description = "A tiny but valid init for containers";
Expand Down