From 9916c8168f5bfef6f305ee42b1edaf41f75672e7 Mon Sep 17 00:00:00 2001 From: Mathieu Boespflug Date: Tue, 13 Nov 2018 14:38:06 +0100 Subject: [PATCH 1/3] Refactor: Replace _execute_error() with _execute_or_fail(). This refactor has two simple goals: * Simplify control flow: callers no longer need to `if-then-else` themselves, which becomes cumbersome when you have a sequence of calls to `repository_ctx.execute()`. * Improve the output on failure (by listing the failing command). --- nixpkgs/nixpkgs.bzl | 66 ++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/nixpkgs/nixpkgs.bzl b/nixpkgs/nixpkgs.bzl index 413cce85..e6d09901 100644 --- a/nixpkgs/nixpkgs.bzl +++ b/nixpkgs/nixpkgs.bzl @@ -97,14 +97,17 @@ def _nixpkgs_package_impl(repository_ctx): # as we can do. timeout = 1073741824 - res = repository_ctx.execute(nix_build, quiet = False, timeout = timeout, - environment=dict(NIX_PATH=nix_path)) - if res.return_code == 0: - output_path = res.stdout.splitlines()[-1] - else: - _execute_error(res, "Cannot build Nix attribute `{}`" - .format(repository_ctx.attr.attribute_path)) - + exec_result = _execute_or_fail( + repository_ctx, + nix_build, + failure_message = "Cannot build Nix attribute '{}'.".format( + repository_ctx.attr.attribute_path + ), + quiet = False, + timeout = timeout, + environment=dict(NIX_PATH=nix_path), + ) + output_path = exec_result.stdout.splitlines()[-1] # Build a forest of symlinks (like new_local_package() does) to the # Nix store. _symlink_children(repository_ctx, output_path) @@ -142,6 +145,26 @@ def nixpkgs_package(*args, **kwargs): else: _nixpkgs_package(*args, **kwargs) +def _execute_or_fail(repository_ctx, arguments, failure_message = "", *args, **kwargs): + """Call repository_ctx.execute() and fail if non-zero return code.""" + result = repository_ctx.execute(arguments, *args, **kwargs) + if result.return_code: + outputs = dict( + failure_message = failure_message, + arguments = arguments, + return_code = result.return_code, + stderr = result.stderr, + ) + fail(""" +{failure_message} +Command: {arguments} +Return code: {return_code} +Error output: +{stderr} +""").format(**outputs) + return result + + def _symlink_children(repository_ctx, target_dir): """Create a symlink to all children of `target_dir` in the current build directory.""" @@ -154,13 +177,10 @@ def _symlink_children(repository_ctx, target_dir): # filenames can contain \n "-print0", ] - find_res = repository_ctx.execute(find_args) - if find_res.return_code == 0: - for target in find_res.stdout.rstrip("\0").split("\0"): - basename = target.rpartition("/")[-1] - repository_ctx.symlink(target, basename) - else: - _execute_error(find_res) + exec_result = _execute_or_fail(repository_ctx, find_args) + for target in exec_result.stdout.rstrip("\0").split("\0"): + basename = target.rpartition("/")[-1] + repository_ctx.symlink(target, basename) def _executable_path(repository_ctx, exe_name, extra_msg=""): @@ -170,19 +190,3 @@ def _executable_path(repository_ctx, exe_name, extra_msg=""): fail("Could not find the `{}` executable in PATH.{}\n" .format(exe_name, " " + extra_msg if extra_msg else "")) return path - - -def _execute_error(exec_result, msg): - """Print a nice error message for a failed `execute`.""" - fail(""" -execute() error: {msg} -status code: {code} -stdout: -{stdout} -stderr: -{stderr} -""".format( - msg=msg, - code=exec_result.return_code, - stdout=exec_result.stdout, - stderr=exec_result.stderr)) From f1975f68326e2c3681b0198752f0f8f664c74923 Mon Sep 17 00:00:00 2001 From: mrkkrp Date: Thu, 7 Jun 2018 22:31:55 +0700 Subject: [PATCH 2/3] Allow to re-configure cc toolchain using Nix --- nixpkgs/nixpkgs.bzl | 69 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/nixpkgs/nixpkgs.bzl b/nixpkgs/nixpkgs.bzl index e6d09901..95709476 100644 --- a/nixpkgs/nixpkgs.bzl +++ b/nixpkgs/nixpkgs.bzl @@ -1,3 +1,5 @@ +load("@bazel_tools//tools/cpp:cc_configure.bzl", "cc_autoconf_impl") + """Rules for importing Nixpkgs packages.""" def _nixpkgs_git_repository_impl(repository_ctx): @@ -190,3 +192,70 @@ def _executable_path(repository_ctx, exe_name, extra_msg=""): fail("Could not find the `{}` executable in PATH.{}\n" .format(exe_name, " " + extra_msg if extra_msg else "")) return path + + +def _cc_configure_custom(ctx): + overriden_tools = { + "gcc": ctx.path(ctx.attr.gcc), + "ld": ctx.path(ctx.attr.ld), + } + return cc_autoconf_impl(ctx, overriden_tools) + + +cc_configure_custom = repository_rule( + implementation = _cc_configure_custom, + attrs = { + "gcc": attr.label( + executable=True, + cfg="host", + allow_single_file=True, + doc="`gcc` to use in cc toolchain", + ), + "ld": attr.label( + executable=True, + cfg="host", + allow_single_file=True, + doc="`ld` to use in cc toolchain", + ), + }, + local = True, + environ = [ + "ABI_LIBC_VERSION", + "ABI_VERSION", + "BAZEL_COMPILER", + "BAZEL_HOST_SYSTEM", + "BAZEL_LINKOPTS", + "BAZEL_PYTHON", + "BAZEL_SH", + "BAZEL_TARGET_CPU", + "BAZEL_TARGET_LIBC", + "BAZEL_TARGET_SYSTEM", + "BAZEL_USE_CPP_ONLY_TOOLCHAIN", + "BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN", + "BAZEL_USE_LLVM_NATIVE_COVERAGE", + "BAZEL_VC", + "BAZEL_VS", + "CC", + "CC_CONFIGURE_DEBUG", + "CC_TOOLCHAIN_NAME", + "CPLUS_INCLUDE_PATH", + "CUDA_COMPUTE_CAPABILITIES", + "CUDA_PATH", + "GCOV", + "HOMEBREW_RUBY_PATH", + "NO_WHOLE_ARCHIVE_OPTION", + "SYSTEMROOT", + "USE_DYNAMIC_CRT", + "USE_MSVC_WRAPPER", + "VS90COMNTOOLS", + "VS100COMNTOOLS", + "VS110COMNTOOLS", + "VS120COMNTOOLS", + "VS140COMNTOOLS", + ], +) +"""Overwrite cc toolchain by supplying custom `gcc` and `ld` (e.g. from +Nix). This allows to fix mismatch of `gcc` versions between what is used by +packages that come from Nix (e.g. `ghc`) and what Bazel detects +automatically (i.e. system-level `gcc`). +""" From 200bff4dd681ad532d2788b921cb46ca09415953 Mon Sep 17 00:00:00 2001 From: Mathieu Boespflug Date: Sun, 11 Nov 2018 00:53:43 +0100 Subject: [PATCH 3/3] Implement nixpkgs_cc_configure This macro calls cc_autoconf_impl like before, but ahead of then also defines a nixpkgs_package with all the tools we want for the CC toolchain. --- README.md | 55 +++++++++++++++++ WORKSPACE | 9 ++- nixpkgs/BUILD.pkg | 2 +- nixpkgs/nixpkgs.bzl | 147 ++++++++++++++++++++++---------------------- tests/BUILD | 6 ++ tests/cc-test.cc | 1 + 6 files changed, 143 insertions(+), 77 deletions(-) create mode 100644 tests/cc-test.cc diff --git a/README.md b/README.md index 746c4b95..4f72dd1a 100644 --- a/README.md +++ b/README.md @@ -226,6 +226,61 @@ filegroup( +### nixpkgs_cc_configure + +Tells Bazel to use compilers and linkers from Nixpkgs for the CC +toolchain. By default, Bazel autodetects a toolchain on the current +`PATH`. Overriding this autodetection makes builds more hermetic and +is considered a best practice. + +Example: + +```bzl +nixpkgs_cc_configure(repository = "@nixpkgs//:default.nix") +``` + + + + + + + + + + + + + + + + + + + + + + + + + +
Attributes
nix_file_content +

String; optional

+

An expression for a Nix environment derivation.

+
repository +

Label; optional

+

A repository label identifying which Nixpkgs to use.

+
repositories +

String-keyed label dict; optional

+

A dictionary mapping `NIX_PATH` entries to repository labels.

+

Setting it to + 

repositories = { "myrepo" : "//:myrepo" }
+ for example would replace all instances + of <myrepo> in the called nix code by the + path to the target "//:myrepo". See the + relevant + section in the nix manual for more information.

+

Specify one of `path` or `repositories`.

+
## Migration diff --git a/WORKSPACE b/WORKSPACE index 4a19e8f2..00373245 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -1,6 +1,11 @@ workspace(name = "io_tweag_rules_nixpkgs") -load("//nixpkgs:nixpkgs.bzl", "nixpkgs_git_repository", "nixpkgs_package") +load( + "//nixpkgs:nixpkgs.bzl", + "nixpkgs_cc_configure", + "nixpkgs_git_repository", + "nixpkgs_package" +) # For tests @@ -74,3 +79,5 @@ filegroup( ) """, ) + +nixpkgs_cc_configure(repository = "@remote_nixpkgs//:default.nix") diff --git a/nixpkgs/BUILD.pkg b/nixpkgs/BUILD.pkg index 288cf998..badfdcc2 100644 --- a/nixpkgs/BUILD.pkg +++ b/nixpkgs/BUILD.pkg @@ -1,4 +1,4 @@ -package(default_visibility = [ "//visibility:public" ]) +package(default_visibility = ["//visibility:public"]) filegroup( name = "bin", diff --git a/nixpkgs/nixpkgs.bzl b/nixpkgs/nixpkgs.bzl index 95709476..1991b4b7 100644 --- a/nixpkgs/nixpkgs.bzl +++ b/nixpkgs/nixpkgs.bzl @@ -112,7 +112,9 @@ def _nixpkgs_package_impl(repository_ctx): output_path = exec_result.stdout.splitlines()[-1] # Build a forest of symlinks (like new_local_package() does) to the # Nix store. - _symlink_children(repository_ctx, output_path) + for target in _find_children(repository_ctx, output_path): + basename = target.rpartition("/")[-1] + repository_ctx.symlink(target, basename) _nixpkgs_package = repository_rule( @@ -130,6 +132,7 @@ _nixpkgs_package = repository_rule( local = True, ) + def nixpkgs_package(*args, **kwargs): # Because of https://github.com/bazelbuild/bazel/issues/5356 we can't # directly pass a dict from strings to labels to the rule (which we'd like @@ -147,6 +150,70 @@ def nixpkgs_package(*args, **kwargs): else: _nixpkgs_package(*args, **kwargs) + +def _nixpkgs_cc_autoconf_impl(repository_ctx): + # Calling repository_ctx.path() on anything but a regular file + # fails. So the roundabout way to do the same thing is to find + # a regular file we know is in the workspace (i.e. the WORKSPACE + # file itself) and then use dirname to get the path of the workspace + # root. + workspace_file_path = repository_ctx.path( + Label("@nixpkgs_cc_toolchain//:WORKSPACE") + ) + workspace_root = _execute_or_fail( + repository_ctx, + ["dirname", workspace_file_path], + ).stdout.rstrip() + + # Make a list of all available tools in the Nix derivation. Override + # the Bazel autoconfiguration with the tools we found. + bin_contents = _find_children(repository_ctx, workspace_root + "/bin") + overriden_tools = { + tool: repository_ctx.path(Label("@nixpkgs_cc_toolchain//:bin/" + tool)) + for entry in bin_contents + for tool in [entry.rpartition("/")[-1]] # Compute basename + } + cc_autoconf_impl(repository_ctx, overriden_tools = overriden_tools) + +_nixpkgs_cc_autoconf = repository_rule( + implementation = _nixpkgs_cc_autoconf_impl +) + + +def nixpkgs_cc_configure( + repository = None, + repositories = None, + nix_file_content = """ + with import {}; buildEnv { + name = "bazel-cc-toolchain"; + paths = [ gcc binutils ]; + } + """): + """Use a CC toolchain from Nixpkgs. + + By default, Bazel auto-configures a CC toolchain from commands (e.g. + `gcc`) available in the environment. To make builds more hermetic, use + this rule to specific explicitly which commands the toolchain should + use. + """ + if repository and repositories or not repository and not repositories: + fail("Specify one of repository or repositories (but not both).") + elif repository: + repositories = {"nixpkgs": repository} + + nixpkgs_package( + name = "nixpkgs_cc_toolchain", + repositories = repositories, + nix_file_content = nix_file_content, + build_file_content = """exports_files(glob(["bin/*"]))""", + ) + # Following lines should match + # https://github.com/bazelbuild/bazel/blob/master/tools/cpp/cc_configure.bzl#L93. + _nixpkgs_cc_autoconf(name = "local_config_cc") + native.bind(name = "cc_toolchain", actual = "@local_config_cc//:toolchain") + native.register_toolchains("@local_config_cc//:all") + + def _execute_or_fail(repository_ctx, arguments, failure_message = "", *args, **kwargs): """Call repository_ctx.execute() and fail if non-zero return code.""" result = repository_ctx.execute(arguments, *args, **kwargs) @@ -167,11 +234,10 @@ Error output: return result -def _symlink_children(repository_ctx, target_dir): - """Create a symlink to all children of `target_dir` in the current - build directory.""" +def _find_children(repository_ctx, target_dir): find_args = [ _executable_path(repository_ctx, "find"), + "-L", target_dir, "-maxdepth", "1", # otherwise the directory is printed as well @@ -180,10 +246,8 @@ def _symlink_children(repository_ctx, target_dir): "-print0", ] exec_result = _execute_or_fail(repository_ctx, find_args) - for target in exec_result.stdout.rstrip("\0").split("\0"): - basename = target.rpartition("/")[-1] - repository_ctx.symlink(target, basename) - + return exec_result.stdout.rstrip("\0").split("\0") + def _executable_path(repository_ctx, exe_name, extra_msg=""): """Try to find the executable, fail with an error.""" @@ -192,70 +256,3 @@ def _executable_path(repository_ctx, exe_name, extra_msg=""): fail("Could not find the `{}` executable in PATH.{}\n" .format(exe_name, " " + extra_msg if extra_msg else "")) return path - - -def _cc_configure_custom(ctx): - overriden_tools = { - "gcc": ctx.path(ctx.attr.gcc), - "ld": ctx.path(ctx.attr.ld), - } - return cc_autoconf_impl(ctx, overriden_tools) - - -cc_configure_custom = repository_rule( - implementation = _cc_configure_custom, - attrs = { - "gcc": attr.label( - executable=True, - cfg="host", - allow_single_file=True, - doc="`gcc` to use in cc toolchain", - ), - "ld": attr.label( - executable=True, - cfg="host", - allow_single_file=True, - doc="`ld` to use in cc toolchain", - ), - }, - local = True, - environ = [ - "ABI_LIBC_VERSION", - "ABI_VERSION", - "BAZEL_COMPILER", - "BAZEL_HOST_SYSTEM", - "BAZEL_LINKOPTS", - "BAZEL_PYTHON", - "BAZEL_SH", - "BAZEL_TARGET_CPU", - "BAZEL_TARGET_LIBC", - "BAZEL_TARGET_SYSTEM", - "BAZEL_USE_CPP_ONLY_TOOLCHAIN", - "BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN", - "BAZEL_USE_LLVM_NATIVE_COVERAGE", - "BAZEL_VC", - "BAZEL_VS", - "CC", - "CC_CONFIGURE_DEBUG", - "CC_TOOLCHAIN_NAME", - "CPLUS_INCLUDE_PATH", - "CUDA_COMPUTE_CAPABILITIES", - "CUDA_PATH", - "GCOV", - "HOMEBREW_RUBY_PATH", - "NO_WHOLE_ARCHIVE_OPTION", - "SYSTEMROOT", - "USE_DYNAMIC_CRT", - "USE_MSVC_WRAPPER", - "VS90COMNTOOLS", - "VS100COMNTOOLS", - "VS110COMNTOOLS", - "VS120COMNTOOLS", - "VS140COMNTOOLS", - ], -) -"""Overwrite cc toolchain by supplying custom `gcc` and `ld` (e.g. from -Nix). This allows to fix mismatch of `gcc` versions between what is used by -packages that come from Nix (e.g. `ghc`) and what Bazel detects -automatically (i.e. system-level `gcc`). -""" diff --git a/tests/BUILD b/tests/BUILD index ba338fb8..866f8fbc 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -47,3 +47,9 @@ package(default_testonly = 1) timeout = "short", ) ] + +# Test nixpkgs_cc_configure() by building some CC code. +cc_binary( + name = "cc-test", + srcs = ["cc-test.cc"], +) diff --git a/tests/cc-test.cc b/tests/cc-test.cc new file mode 100644 index 00000000..76e81970 --- /dev/null +++ b/tests/cc-test.cc @@ -0,0 +1 @@ +int main() { return 0; }