-
Notifications
You must be signed in to change notification settings - Fork 81
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
Variation of #214 (Rule to generate docker images with nix store paths) #351
Open
dmadisetti
wants to merge
5
commits into
tweag:master
Choose a base branch
from
cemel-jhu:docker
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
b7e7f84
FF + Wrap pathnames as strings for Nix compatibility
dmadisetti 65332a8
Escape path for tests + attributes
dmadisetti 09e2d1b
Handle cases for longer attribute paths
dmadisetti 42e7670
Missing edge case for attribute path
dmadisetti edac673
Sanitize path with ~
dmadisetti File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package(default_visibility = ["//visibility:public"]) | ||
|
||
load("@bazel_skylib//:bzl_library.bzl", "bzl_library") | ||
|
||
exports_files([ | ||
"docker/stream.sh", | ||
]) | ||
|
||
bzl_library( | ||
name = "docker", | ||
srcs = [ | ||
"docker.bzl", | ||
], | ||
visibility = ["//visibility:public"], | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
""" | ||
# Docker containerization Bazel Nixpkgs rules | ||
|
||
To run bazel artifacts across systems and platforms, nixpkgs_rules exposed a | ||
docker hook. e.g. | ||
|
||
```starlark | ||
nixpkgs_docker_image( | ||
name = "nix_deps_image", | ||
srcs = [ | ||
"@cc_toolchain_nixpkgs_info////bazel-support", | ||
"@nixpkgs_python_toolchain_python3//bazel-support", | ||
"@nixpkgs_sh_posix_config//bazel-support", | ||
"@rules_haskell_ghc_nixpkgs//bazel-support", | ||
"@nixpkgs_valgrind//bazel-support", | ||
], | ||
bazel = "@nixpkgs_bazel//bazel-support", | ||
repositories = {"nixpkgs": "@nixpkgs"}, | ||
) | ||
``` | ||
|
||
here, nixpkgs rules dependencies are bundled into a docker container for use and | ||
deployment. | ||
|
||
""" | ||
|
||
def _nixpkgs_docker_image_impl(repository_ctx): | ||
repositories = repository_ctx.attr.repositories | ||
|
||
nix_build_bin = repository_ctx.which("nix-build") | ||
repository_ctx.symlink(nix_build_bin, "nix-build") | ||
|
||
srcs = repository_ctx.attr.srcs | ||
bazel = repository_ctx.attr.bazel | ||
|
||
# HACK! On bazel from nixpkgs, shebangs get mangled in things like | ||
# @bazel_tools//tools/cpp:linux_cc_wrapper.sh.tpl, so that nix store | ||
# paths end up referenced there. | ||
# One needs to ensure those paths are available on the docker image | ||
# as well, we can do that by including bazel | ||
srcs = srcs + [bazel] if bazel else srcs | ||
|
||
contents = [] | ||
for src in srcs: | ||
path_to_default_nix = repository_ctx.path(src.relative("default.nix")) | ||
package = "bazel-support/%s" % src.workspace_name | ||
repository_ctx.symlink(path_to_default_nix.dirname, package) | ||
contents.append("(import ./%s {})" % package) | ||
|
||
repository_ctx.template( | ||
"default.nix", | ||
Label("@io_tweag_rules_nixpkgs//containers:docker/default.nix.tpl"), | ||
substitutions = { | ||
"%{name}": repr(repository_ctx.name), | ||
"%{contents}": "\n ".join(contents), | ||
}, | ||
executable = False, | ||
) | ||
|
||
args = list(repository_ctx.attr.nixopts) | ||
for repo_label, repo_name in repositories.items(): | ||
absolute_repo = repository_ctx.path(repo_label).dirname | ||
|
||
# Excessive quoting due to nix limitations for ~ in file path | ||
# (see NixOS/nix#7742). | ||
args.extend([ | ||
'"-I"', | ||
"\"%s=\\\"%s\\\"\"" % (repo_name, absolute_repo), | ||
]) | ||
|
||
repository_ctx.template( | ||
"BUILD", | ||
Label("@io_tweag_rules_nixpkgs//containers:docker/BUILD.bazel.tpl"), | ||
substitutions = { | ||
"%{args_comma_sep}": ",\n ".join(args), | ||
"%{args_space_sep}": " ".join(args), | ||
}, | ||
executable = False, | ||
) | ||
|
||
_nixpkgs_docker_image = repository_rule( | ||
implementation = _nixpkgs_docker_image_impl, | ||
attrs = { | ||
"nixopts": attr.string_list(), | ||
"repositories": attr.label_keyed_string_dict(), | ||
"srcs": attr.label_list( | ||
doc = 'List of nixpkgs_package to include in the image. E.g. ["@hello//nixpkg"]', | ||
), | ||
"bazel": attr.label( | ||
doc = """If using bazel from nixpkgs, this a nixpackage_package | ||
based on exactly the same bazel derivation. This is to ensure any paths | ||
for mangled '/usr/env bash' introduced by nix exist in the store. | ||
Example: '<nixpkgs>.bazel_4'. | ||
""", | ||
), | ||
}, | ||
) | ||
|
||
def nixpkgs_docker_image(name, **kwargs): | ||
repositories = kwargs.get("repositories") | ||
if repositories: | ||
inversed_repositories = {value: key for key, value in repositories.items()} | ||
kwargs["repositories"] = inversed_repositories | ||
_nixpkgs_docker_image(name = name, **kwargs) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
sh_binary( | ||
name = "stream", | ||
srcs = ["@io_tweag_rules_nixpkgs//containers:docker/stream.sh"], | ||
data = [ | ||
":default.nix", | ||
":nix-build", | ||
], | ||
env = {"NIX_BUILD": "$(location :nix-build)"}, | ||
args = [ | ||
'"./$(location :default.nix)"', | ||
%{args_comma_sep}, | ||
], | ||
) | ||
|
||
genrule( | ||
name = "image", | ||
srcs = [ | ||
":default.nix", | ||
], | ||
outs = ["image.tgz"], | ||
exec_tools = [":nix-build"], | ||
cmd = """ | ||
$(location :nix-build) %{args_space_sep} \ | ||
--arg stream false \ | ||
--out-link $@ \ | ||
"./$(location :default.nix)" | ||
""", | ||
local = True, | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
{ stream ? true, tag ? null, nixpkgs ? import <nixpkgs> {} }: | ||
let | ||
dockerImage = if stream | ||
then nixpkgs.dockerTools.streamLayeredImage | ||
else nixpkgs.dockerTools.buildLayeredImage; | ||
|
||
name = %{name}; | ||
|
||
contents = [ | ||
%{contents} | ||
]; | ||
|
||
manifest = nixpkgs.writeTextFile | ||
{ name = "${name}-MANIFEST"; | ||
text = nixpkgs.lib.strings.concatMapStrings (pkg: "${pkg}\n") contents; | ||
destination = "/MANIFEST"; | ||
}; | ||
|
||
usr_bin_env = nixpkgs.runCommand "usr-bin-env" {} '' | ||
mkdir -p "$out/usr/bin/" | ||
ln -s "${nixpkgs.coreutils}/bin/env" "$out/usr/bin/" | ||
''; | ||
in | ||
dockerImage { | ||
inherit name tag; | ||
|
||
contents = [ | ||
# Contents get copied to the top-level of the image, so we jus putt | ||
# a short manifest file here and get all the store paths as dependencies | ||
manifest | ||
|
||
# Ensure "/usr/bin/env bash" works correctly | ||
nixpkgs.bash | ||
nixpkgs.coreutils | ||
usr_bin_env | ||
|
||
# avoid "commitBuffer: invalid argument (invalid character)" running tests | ||
nixpkgs.glibcLocales | ||
]; | ||
|
||
extraCommands = "mkdir -m 0777 tmp"; | ||
|
||
config = { | ||
Cmd = [ "${nixpkgs.bashInteractive}/bin/bash" ]; | ||
}; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
set -euo pipefail | ||
|
||
run() { | ||
${NIX_BUILD} --arg stream true "$@" | ||
} | ||
|
||
$(run "$@") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
{ %{args_with_defaults} }: | ||
let | ||
value_or_function = %{def}; | ||
value = | ||
if builtins.isFunction value_or_function then | ||
let | ||
formalArgs = builtins.functionArgs value_or_function; | ||
actualArgs = builtins.intersectAttrs formalArgs { inherit %{args}; }; | ||
in | ||
value_or_function actualArgs | ||
else value_or_function; | ||
in | ||
value%{maybe_attr} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One problem is the unquoted
~
as discussed above. The linked workaround is the correct fix.Another issue with bzlmod mode will be that this
containers
package doesn't fit into any of the new rules_nixpkgs Bazel modules (see #181, in particular #182 and #322). As laid out there rules_nixpkgs has been split into multiple sub-modules such asrules_nixpkgs_core
undercore/
orrules_nixpkgs_cc
undertoolchains/cc
.The motivation for this split is to avoid rules_nixpkgs turning into a Bazel module that transitively depends on almost all Bazel modules out there.
For the specific rule added here, it doesn't actually introduce any new Bazel module dependencies. The fact that it generates a Docker image might suggest that it should belong into a dedicated
rules_nixpkgs_containers
orrules_nixpkgs_docker
Bazel module. But, since it only depends on functionality inrules_nixpkgs_core
and Nix itself, it can actually be part ofrules_nixpkgs_core
.@benradf any thoughts on this? My inclination would be to go with the simpler route of just placing this into
core/
intead of introducing all the overhead of a new dedicatedrules_nixpkgs_containers
module when it's not really necessary.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, there's no compelling reason for a new module right now, so putting it in
rules_nixpkgs_core
makes most sense. It can always be moved later if additional module dependencies are required.