From bae01852d85a17845068091f1b9dc9d4f1b66e06 Mon Sep 17 00:00:00 2001 From: Andreas Herrmann Date: Fri, 7 Feb 2020 16:06:32 +0100 Subject: [PATCH 1/4] nixpkgs_python_configure: Use platform toolchain This changes the Nix configured Python runtime from an "in-build runtime" to a "platform runtime", see https://docs.bazel.build/versions/2.0.0/be/python.html#py_runtime.files. This is to avoid unecessary disk usage in the runfiles tree of targets depending on the python toolchain. In case of an "in-build runtime" Bazel sees all files that belong to the toolchain, including the many include and Python module files. This means that any target that has a runtime dependency on the Python toolchain, will have each file contained in the Python toolchain listed in its runfiles and runfiles manifest file. This adds around 4.6MiB to the manifest file and around 34MiB to the runfiles tree. This space is occupied repeatedly for each target depending on the nixpkgs python toolchain. E.g. on the rules_haskell repository: ``` $ du -ah bazel-bin/tests/run-tests@repl@repl.runfiles/{nixpkgs_python_toolchain_python3,MANIFEST} ... 4.7M bazel-bin/tests/run-tests@repl@repl.runfiles/MANIFEST 34M bazel-bin/tests/run-tests@repl@repl.runfiles/nixpkgs_python_toolchain_python3 ``` After this change: ``` $ du -ah bazel-bin/tests/run-tests@repl@repl.runfiles/{nixpkgs_python_toolchain_python3,MANIFEST} 40K bazel-bin/tests/run-tests@repl@repl.runfiles/MANIFEST ``` --- nixpkgs/nixpkgs.bzl | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/nixpkgs/nixpkgs.bzl b/nixpkgs/nixpkgs.bzl index 263bb37c1..ec0c52d32 100644 --- a/nixpkgs/nixpkgs.bzl +++ b/nixpkgs/nixpkgs.bzl @@ -357,14 +357,27 @@ _nixpkgs_python_toolchain = repository_rule( }, ) -_python_build_file_content = """ -py_runtime( - name = "runtime", - files = glob(["**"]), - interpreter = "{bin_path}", - python_version = "{version}", - visibility = ["//visibility:public"], -) +_python_nix_file_content = """ +with import {{ config = {{}}; overlays = []; }}; +runCommand "bazel-nixpkgs-python-toolchain" + {{ executable = false; + # Pointless to do this on a remote machine. + preferLocalBuild = true; + allowSubstitutes = false; + }} + '' + n=$out/BUILD.bazel + mkdir -p "$(dirname "$n")" + + cat >>$n < Date: Fri, 7 Feb 2020 16:40:37 +0100 Subject: [PATCH 2/4] Update the python toolchain test We can no longer verify the use of the toolchain python interpreter by checking whether it lies in the runfiles directory. Instead, we verify that it is the same interpreter as defined by the configured nixpkgs python toolchain. --- WORKSPACE | 6 ++++++ tests/BUILD.bazel | 8 ++++---- tests/python-test.nix | 36 ++++++++++++++++++++++++++++++++++++ tests/python-test.py | 19 ------------------- 4 files changed, 46 insertions(+), 23 deletions(-) create mode 100644 tests/python-test.nix delete mode 100644 tests/python-test.py diff --git a/WORKSPACE b/WORKSPACE index a2d575006..cef667e9b 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -138,6 +138,12 @@ nixpkgs_cc_configure(repository = "@remote_nixpkgs") nixpkgs_python_configure(repository = "@remote_nixpkgs") +nixpkgs_package( + name = "nixpkgs_python_configure_test", + nix_file = "//tests:python-test.nix", + repository = "@remote_nixpkgs", +) + load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( diff --git a/tests/BUILD.bazel b/tests/BUILD.bazel index 0ef7f438c..049537822 100644 --- a/tests/BUILD.bazel +++ b/tests/BUILD.bazel @@ -59,11 +59,11 @@ cc_binary( ) # Test nixpkgs_python_configure() by running some Python code. -py_test( +test_suite( name = "python-test", - srcs = ["python-test.py"], - python_version = "PY3", - srcs_version = "PY3", + tests = [ + "@nixpkgs_python_configure_test//:python-test", + ], ) # Test nixpkgs_sh_posix_configure() checking that Unix commands are in Nix store. diff --git a/tests/python-test.nix b/tests/python-test.nix new file mode 100644 index 000000000..77a5fffcf --- /dev/null +++ b/tests/python-test.nix @@ -0,0 +1,36 @@ +with import { config = {}; overlays = []; }; +runCommand "test-nixpkgs-python-toolchain" + { executable = false; } + '' + mkdir -p $out + + cat >$out/BUILD.bazel <<'EOF_BUILD' + py_test( + name = "python-test", + srcs = ["python-test.py"], + python_version = "PY3", + srcs_version = "PY3", + visibility = ["//visibility:public"], + ) + EOF_BUILD + + cat >$out/python-test.py <<'EOF_PYTHON' + import os + import sys + + _failure_message = """\ + Python interpreter is not provided by the toolchain. + Expected: '{expected}' + Actual: '{actual}'. + """ + + if __name__ == "__main__": + python_bin = "${python3}/bin/python" + if not sys.executable == python_bin: + print(_failure_message.format( + expected = python_bin, + actual = sys.executable, + ), file=sys.stderr) + sys.exit(1) + EOF_PYTHON + '' diff --git a/tests/python-test.py b/tests/python-test.py deleted file mode 100644 index 5cb28d610..000000000 --- a/tests/python-test.py +++ /dev/null @@ -1,19 +0,0 @@ -import os -import sys - -_failure_message = """\ -Python interpreter is not provided by the toolchain. -Expected: '{expected}' -Actual: '{actual}'. -""" - -if __name__ == "__main__": - runfiles_dir = os.environ["RUNFILES_DIR"] - python_bin = os.path.join( - runfiles_dir, "nixpkgs_python_toolchain_python3", "bin", "python") - if not sys.executable == python_bin: - print(_failure_message.format( - expected = python_bin, - actual = sys.executable, - ), file=sys.stderr) - sys.exit(1) From 8769ec2fad97548b66c628fcf736d68aa15e9283 Mon Sep 17 00:00:00 2001 From: Andreas Herrmann Date: Fri, 7 Feb 2020 16:47:15 +0100 Subject: [PATCH 3/4] Test the python2 toolchain as well --- WORKSPACE | 5 ++++- tests/BUILD.bazel | 3 ++- tests/python-test.nix | 20 ++++++++++++++++---- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index cef667e9b..87bf2058e 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -136,7 +136,10 @@ nixpkgs_package( nixpkgs_cc_configure(repository = "@remote_nixpkgs") -nixpkgs_python_configure(repository = "@remote_nixpkgs") +nixpkgs_python_configure( + python2_attribute_path = "python2", + repository = "@remote_nixpkgs", +) nixpkgs_package( name = "nixpkgs_python_configure_test", diff --git a/tests/BUILD.bazel b/tests/BUILD.bazel index 049537822..5cc1a5f61 100644 --- a/tests/BUILD.bazel +++ b/tests/BUILD.bazel @@ -62,7 +62,8 @@ cc_binary( test_suite( name = "python-test", tests = [ - "@nixpkgs_python_configure_test//:python-test", + "@nixpkgs_python_configure_test//:python2-test", + "@nixpkgs_python_configure_test//:python3-test", ], ) diff --git a/tests/python-test.nix b/tests/python-test.nix index 77a5fffcf..8e953a43f 100644 --- a/tests/python-test.nix +++ b/tests/python-test.nix @@ -6,7 +6,16 @@ runCommand "test-nixpkgs-python-toolchain" cat >$out/BUILD.bazel <<'EOF_BUILD' py_test( - name = "python-test", + name = "python2-test", + main = "python-test.py", + srcs = ["python-test.py"], + python_version = "PY2", + srcs_version = "PY2", + visibility = ["//visibility:public"], + ) + py_test( + name = "python3-test", + main = "python-test.py", srcs = ["python-test.py"], python_version = "PY3", srcs_version = "PY3", @@ -25,12 +34,15 @@ runCommand "test-nixpkgs-python-toolchain" """ if __name__ == "__main__": - python_bin = "${python3}/bin/python" + if sys.version_info.major == 2: + python_bin = "${python2}/bin/python" + else: + python_bin = "${python3}/bin/python" if not sys.executable == python_bin: - print(_failure_message.format( + sys.stderr.write(_failure_message.format( expected = python_bin, actual = sys.executable, - ), file=sys.stderr) + )) sys.exit(1) EOF_PYTHON '' From 7e7f5a5fb8817b51beea16837ce1e087dd304fd1 Mon Sep 17 00:00:00 2001 From: Andreas Herrmann Date: Fri, 7 Feb 2020 16:50:15 +0100 Subject: [PATCH 4/4] Run buildifier formatter --- nixpkgs/nixpkgs.bzl | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/nixpkgs/nixpkgs.bzl b/nixpkgs/nixpkgs.bzl index ec0c52d32..96d9b950c 100644 --- a/nixpkgs/nixpkgs.bzl +++ b/nixpkgs/nixpkgs.bzl @@ -162,10 +162,12 @@ def _nixpkgs_package_impl(repository_ctx): # ensure that the output is a directory test_path = repository_ctx.which("test") - _execute_or_fail(repository_ctx, [test_path, "-d", output_path], - failure_message = "nixpkgs_package '@{}' outputs a single file which is not supported by rules_nixpkgs. Please only use directories.".format( - repository_ctx.name - ), + _execute_or_fail( + repository_ctx, + [test_path, "-d", output_path], + failure_message = "nixpkgs_package '@{}' outputs a single file which is not supported by rules_nixpkgs. Please only use directories.".format( + repository_ctx.name, + ), ) # Build a forest of symlinks (like new_local_package() does) to the @@ -605,7 +607,7 @@ def _find_children(repository_ctx, target_dir): "-print0", ] exec_result = _execute_or_fail(repository_ctx, find_args) - return exec_result.stdout.rstrip("\0").split("\0") + return exec_result.stdout.rstrip("\000").split("\000") def _executable_path(repository_ctx, exe_name, extra_msg = ""): """Try to find the executable, fail with an error."""