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

docker: add clientOnly / docker-client #124561

Merged
merged 1 commit into from
May 27, 2021
Merged

docker: add clientOnly / docker-client #124561

merged 1 commit into from
May 27, 2021

Conversation

zowoq
Copy link
Contributor

@zowoq zowoq commented May 27, 2021

Currently the docker client is only available on non-linux platforms as docker,
this makes the client available on linux and other platforms as docker-client.

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/)
  • Added a release notes entry if the change is major or breaking
  • Fits CONTRIBUTING.md.

@r-rmcgibbo
Copy link

r-rmcgibbo commented May 27, 2021

Result of nixpkgs-review pr 124561 at 3e550bcc run on aarch64-linux 1

1 package built successfully:
  • docker-client
4 suggestions:
  • warning: missing-phase-hooks

    buildPhase should probably contain runHook preBuild.

    Near pkgs/development/go-packages/generic/default.nix:138:5:

        |
    138 |     buildPhase = args.buildPhase or ''
        |     ^
    
  • warning: name-and-version

    Did you mean to pass pname instead of name to mkDerivation?

    Near pkgs/applications/virtualization/docker/default.nix:127:5:

        |
    127 |     name = "docker-${version}";
        |     ^
    

    Near pkgs/applications/virtualization/docker/default.nix:125:12:

        |
    125 |     inherit version rev;
        |            ^
    
  • warning: unused-argument

    Unused argument: fetchpatch.
    Near pkgs/applications/virtualization/docker/default.nix:13:34:

       |
    13 |       , stdenv, fetchFromGitHub, fetchpatch, buildGoPackage
       |                                  ^
    
  • warning: missing-phase-hooks

    installPhase should probably contain runHook preInstall and runHook postInstall.

    Near pkgs/development/go-packages/generic/default.nix:216:5:

        |
    216 |     installPhase = args.installPhase or ''
        |     ^
    

Result of nixpkgs-review pr 124561 at 3e550bcc run on x86_64-linux 1

1 package built successfully:
  • docker-client
4 suggestions:
  • warning: name-and-version

    Did you mean to pass pname instead of name to mkDerivation?

    Near pkgs/applications/virtualization/docker/default.nix:127:5:

        |
    127 |     name = "docker-${version}";
        |     ^
    

    Near pkgs/applications/virtualization/docker/default.nix:125:12:

        |
    125 |     inherit version rev;
        |            ^
    
  • warning: unused-argument

    Unused argument: fetchpatch.
    Near pkgs/applications/virtualization/docker/default.nix:13:34:

       |
    13 |       , stdenv, fetchFromGitHub, fetchpatch, buildGoPackage
       |                                  ^
    
  • warning: missing-phase-hooks

    installPhase should probably contain runHook preInstall and runHook postInstall.

    Near pkgs/development/go-packages/generic/default.nix:216:5:

        |
    216 |     installPhase = args.installPhase or ''
        |     ^
    
  • warning: missing-phase-hooks

    buildPhase should probably contain runHook preBuild.

    Near pkgs/development/go-packages/generic/default.nix:138:5:

        |
    138 |     buildPhase = args.buildPhase or ''
        |     ^
    

Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

passthru.tests is wrong when clientOnly is true.
For testing, we should add an option to the docker module not to add cfg.package to the systemPackages. We can then parameterize the docker test and run it with docker-client in systemPackages.

pkgs/applications/virtualization/docker/default.nix Outdated Show resolved Hide resolved
@zowoq
Copy link
Contributor Author

zowoq commented May 27, 2021

For testing, we should add an option to the docker module not to add cfg.package to the systemPackages. We can then parameterize the docker test and run it with docker-client in systemPackages.

I may get to this at some point but as we'll testing the client via the podman test I think that will suffice for now.

Currently the docker client is only available on non-linux platforms as `docker`,
this makes the client available on linux and other platforms as `docker-client`.
@zowoq zowoq merged commit 7233acd into NixOS:master May 27, 2021
@zowoq zowoq deleted the docker-client branch May 27, 2021 22:09
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.

3 participants