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

dockerTools: fix build #109420

Merged
1 commit merged into from Jan 15, 2021
Merged

dockerTools: fix build #109420

1 commit merged into from Jan 15, 2021

Conversation

ghost
Copy link

@ghost ghost commented Jan 15, 2021

Motivation for this change
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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ghost ghost requested a review from roberth as a code owner January 15, 2021 01:54
@ghost
Copy link
Author

ghost commented Jan 15, 2021

Also CC @mikroskeem who created the Docker 20.10 PR

@mikroskeem
Copy link
Member

LGTM

@ghost ghost merged commit 0d724ba into NixOS:master Jan 15, 2021
@quetz
Copy link

quetz commented Jan 15, 2021

This breaks dockerTools.tarsum

nix-build -A pkgs.dockerTools.tarsum
error: attribute 'moby' missing, at /Users/qz/src/nixpkgs/pkgs/build-support/docker/default.nix:123:14
(use '--show-trace' to show detailed location information)

@ghost
Copy link
Author

ghost commented Jan 15, 2021

This breaks dockerTools.tarsum

I don't think so, I can build dockerTools.tarsum on master (and it's exactly what this is supposed to fix).
Did you cherry-pick this on top of nixos-unstable or something? Because this needs #108960 (which was already on master when this was merged).

@quetz
Copy link

quetz commented Jan 15, 2021

I don't think so, I can build dockerTools.tarsum on master (and it's exactly what this is supposed to fix).
Did you cherry-pick this on top of nixos-unstable or something? Because this needs #108960 (which was already on master when this was merged).

Tried it on 8e4690c. What commit are you on?

@ghost
Copy link
Author

ghost commented Jan 15, 2021

The same:

[pbb@regolith:~/proj/nixpkgs]$ git checkout nixos/master
HEAD is now at 8e4690c0ca9 Merge pull request #109011 from NixOS/haskell-updates

[pbb@regolith:~/proj/nixpkgs]$ nix-build -A dockerTools.tarsum
/nix/store/af35q3bmxqx4i8416m5rnzbark13vbi8-tarsum

[pbb@regolith:~/proj/nixpkgs]$ nix-build -A dockerTools
/nix/store/yfz0j967i56253zbcmkk6qjzk2gqq132-bin-sh
/nix/store/6vyz7dcdczp2l5xhc11iv1h0s1imb5si-fake-nss
/nix/store/af35q3bmxqx4i8416m5rnzbark13vbi8-tarsum

[pbb@regolith:~/proj/nixpkgs]$

@quetz
Copy link

quetz commented Jan 15, 2021

Weird. Are your search results for moby look similar to mine?

~/src/nixpkgs(master ✔) ag moby
pkgs/development/tools/buildkit/default.nix
7:  goPackagePath = "github.com/moby/buildkit";
11:    owner = "moby";
21:    homepage = "https://github.com/moby/buildkit";

pkgs/top-level/aliases.nix
364:  moby = throw "moby has been removed, merged into linuxkit in 2018.  Use linuxkit instead.";

pkgs/applications/virtualization/docker/default.nix
15:      , mobyRev, mobySha256
63:    moby = buildGoPackage ((optionalAttrs (stdenv.isLinux)) rec {
64:      name = "moby-${version}";
69:        owner = "moby";
70:        repo = "moby";
71:        rev = mobyRev;
72:        sha256 = mobySha256;
124:    inherit docker-runc docker-containerd docker-proxy docker-tini moby;
182:      ln -s ${moby}/bin/dockerd $out/bin/dockerd
214:  # https://github.com/moby/moby/tree/${version}/hack/dockerfile/install/*
219:    mobyRev = "v${version}";
220:    mobySha256 = "0c2zycpnwj4kh8m8xckv1raj3fx07q9bfaj46rr85jihm4p2dp5w";

pkgs/build-support/docker/default.nix
123:    ln -sT ${docker.moby.src}/pkg/tarsum src/github.com/docker/docker/pkg/tarsum

pkgs/build-support/docker/stream_layered_image.py
29:[1]: https://github.com/moby/moby/blob/master/image/spec/v1.2.md
30:[2]: https://github.com/moby/moby/blob/4fb59c20a4fb54f944fe170d0ff1d00eb4a24d6f/image/spec/v1.2.md#image-json-field-descriptions

doc/builders/images/dockertools.xml
8:  <varname>pkgs.dockerTools</varname> is a set of functions for creating and manipulating Docker images according to the <link xlink:href="https://github.com/moby/moby/blob/master/image/spec/v1.2.md#docker-image-specification-v120"> Docker Image Specification v1.2.0 </link>. Docker itself is not used to perform any of the operations done by these functions.
97:     <varname>config</varname> is used to specify the configuration of the containers that will be started off the built image in Docker. The available options are listed in the <link xlink:href="https://github.com/moby/moby/blob/master/image/spec/v1.2.md#image-json-field-descriptions"> Docker Image Specification v1.2.0 </link>.
215:      Run-time configuration of the container. A full list of the options are available at in the <link xlink:href="https://github.com/moby/moby/blob/master/image/spec/v1.2.md#image-json-field-descriptions"> Docker Image Specification v1.2.0 </link>.

nixos/tests/common/acme/server/ca.key.pem
9:Wl0xRmwil2YYPuL5MeJgJPktMObY1IfpBCw3tz3w2M3fiuCMf0d2dMGtO1xLiUnH

@ghost
Copy link
Author

ghost commented Jan 15, 2021

Yes:

[pbb@regolith:~/proj/nixpkgs]$ git reset

[pbb@regolith:~/proj/nixpkgs]$ rg moby
pkgs/top-level/aliases.nix
364:  moby = throw "moby has been removed, merged into linuxkit in 2018.  Use linuxkit instead.";

pkgs/development/tools/buildkit/default.nix
7:  goPackagePath = "github.com/moby/buildkit";
11:    owner = "moby";
21:    homepage = "https://github.com/moby/buildkit";

pkgs/build-support/docker/stream_layered_image.py
29:[1]: https://github.com/moby/moby/blob/master/image/spec/v1.2.md
30:[2]: https://github.com/moby/moby/blob/4fb59c20a4fb54f944fe170d0ff1d00eb4a24d6f/image/spec/v1.2.md#image-json-field-descriptions

pkgs/build-support/docker/default.nix
123:    ln -sT ${docker.moby.src}/pkg/tarsum src/github.com/docker/docker/pkg/tarsum

doc/builders/images/dockertools.xml
8:  <varname>pkgs.dockerTools</varname> is a set of functions for creating and manipulating Docker images according to the <link xlink:href="https://github.com/moby/moby/blob/master/image/spec/v1.2.md#docker-image-specification-v120"> Docker Image Specification v1.2.0 </link>. Docker itself is not used to perform any of the operations done by these functions.
97:     <varname>config</varname> is used to specify the configuration of the containers that will be started off the built image in Docker. The available options are listed in the <link xlink:href="https://github.com/moby/moby/blob/master/image/spec/v1.2.md#image-json-field-descriptions"> Docker Image Specification v1.2.0 </link>.
215:      Run-time configuration of the container. A full list of the options are available at in the <link xlink:href="https://github.com/moby/moby/blob/master/image/spec/v1.2.md#image-json-field-descriptions"> Docker Image Specification v1.2.0 </link>.

pkgs/applications/virtualization/docker/default.nix
15:      , mobyRev, mobySha256
63:    moby = buildGoPackage ((optionalAttrs (stdenv.isLinux)) rec {
64:      name = "moby-${version}";
69:        owner = "moby";
70:        repo = "moby";
71:        rev = mobyRev;
72:        sha256 = mobySha256;
124:    inherit docker-runc docker-containerd docker-proxy docker-tini moby;
182:      ln -s ${moby}/bin/dockerd $out/bin/dockerd
214:  # https://github.com/moby/moby/tree/${version}/hack/dockerfile/install/*
219:    mobyRev = "v${version}";
220:    mobySha256 = "0c2zycpnwj4kh8m8xckv1raj3fx07q9bfaj46rr85jihm4p2dp5w";

[pbb@regolith:~/proj/nixpkgs]$

But I believe you are not actually building dockerTools.tarsum from the nixpkgs tree you are searching. What command did you use?

@quetz
Copy link

quetz commented Jan 15, 2021 •
edited 

~/src/nixpkgs(master ✔) env | grep NIX_PATH
NIX_PATH=nixpkgs=/Users/qz/src/nixpkgs

~/src/nixpkgs(master ✔) nix-build -A pkgs.dockerTools.tarsum
error: attribute 'moby' missing, at /Users/qz/src/nixpkgs/pkgs/build-support/docker/default.nix:123:14
(use '--show-trace' to show detailed location information)

Looks like it is referencing to default.nix at my checked out nixpkgs repo. Any ideas how to debug that further?
Just looking at source I can't figure out where's that moby attribute supposed to come from. What code sets it on docker derivation?

@ghost
Copy link
Author

ghost commented Jan 15, 2021 •
edited by ghost

docker.moby is set in pkgs/applications/virtualization/docker/default.nix:63.

Are you sure there are no local modifications? You can check with git reset.

Oh, wait, what platform are you on? moby only exists for Linux appearantly.

@quetz
Copy link

quetz commented Jan 15, 2021

Darwin here. And looks like moby is set only for linux at

nixpkgs/pkgs/applications/virtualization/docker/default.nix

Line 63 in 8e4690c

moby = buildGoPackage ((optionalAttrs (stdenv.isLinux)) rec {

@mikroskeem
Copy link
Member

mikroskeem commented Jan 15, 2021 •
edited

Makes sense why this is broken... but building Docker daemon / Moby does not really make sense on non-Linux, and build process has changed significantly. Could probably expose Moby source in different way maybe?

@quetz
Copy link

quetz commented Jan 15, 2021 •
edited

I just replaced stdenv.isLinux there with true and tarsum started building just fine.

Exposing moby sources as moby-src and referring to it also works (and should not break anything).

@ozanmakes ozanmakes mentioned this pull request Mar 4, 2021
Merged
10 tasks
ozanmakes pushed a commit to ozanmakes/nixpkgs that referenced this pull request Mar 4, 2021
dockerTools: fix build on Darwin
78f322f 
Fixes NixOS#110665

Introduced by NixOS#109420
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roberth roberth Awaiting requested review from roberth roberth is a code owner

@thiagokokada thiagokokada Awaiting requested review from thiagokokada

@NeQuissimus NeQuissimus Awaiting requested review from NeQuissimus

@vdemeester vdemeester Awaiting requested review from vdemeester

Assignees
No one assigned
Labels
8.has: documentation 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mikroskeem @quetz