From 4904e7e8306ca8344450fe010d46d0be9485e42f Mon Sep 17 00:00:00 2001 From: Jordan Mele Date: Mon, 4 Dec 2023 18:20:19 +1100 Subject: [PATCH 1/2] Optimise copying of `data` inputs in `npm_install` and `yarn_install` --- internal/npm_install/BUILD.bazel | 7 +++-- internal/npm_install/bulk_copy.sh | 12 +++++++++ internal/npm_install/npm_install.bzl | 40 +++++++++++++++++++++++----- 3 files changed, 50 insertions(+), 9 deletions(-) create mode 100755 internal/npm_install/bulk_copy.sh diff --git a/internal/npm_install/BUILD.bazel b/internal/npm_install/BUILD.bazel index 0a745f10..43b8ca58 100755 --- a/internal/npm_install/BUILD.bazel +++ b/internal/npm_install/BUILD.bazel @@ -11,8 +11,11 @@ bzl_library( visibility = ["//visibility:public"], ) -# Exported to be consumed for generating stardoc. -exports_files(["npm_install.bzl"]) +exports_files([ + "bulk_copy.sh", + # Exported to be consumed for generating stardoc. + "npm_install.bzl", +]) filegroup( name = "generate_build_file", diff --git a/internal/npm_install/bulk_copy.sh b/internal/npm_install/bulk_copy.sh new file mode 100755 index 00000000..20e8afda --- /dev/null +++ b/internal/npm_install/bulk_copy.sh @@ -0,0 +1,12 @@ +#!/bin/sh + +set -eu -o pipefail + +# The list of files to copy, relative to base so directories are preserved +input_files_list="$1" +# The base path which input files will be resolved from +input_files_base="$2" +# Destination directory for files +out_dir="$3" + +tar --create --file - --directory "$input_files_base" --files-from "$input_files_list"| tar --extract --file - --directory "$out_dir" diff --git a/internal/npm_install/npm_install.bzl b/internal/npm_install/npm_install.bzl index 2633dc1a..64a498ae 100755 --- a/internal/npm_install/npm_install.bzl +++ b/internal/npm_install/npm_install.bzl @@ -516,13 +516,37 @@ def _symlink_file(repository_ctx, f): def _copy_data_dependencies(repository_ctx): """Add data dependencies to the repository.""" - total = len(repository_ctx.attr.data) - for i, f in enumerate(repository_ctx.attr.data): - repository_ctx.report_progress("Copying data dependencies (%s/%s)" % (i, total)) - # Make copies of the data files instead of symlinking - # as yarn under linux will have trouble using symlinked - # files as npm file:// packages - _copy_file(repository_ctx, f) + # Collect inputs + input_files_list = [] + input_files_base = str(repository_ctx.workspace_root) + for label in repository_ctx.attr.data: + if label.workspace_root != "": + fail("Only files from the main workspace can be given to `data`, but got %s" % label) + file = str(repository_ctx.path(label)) + relative_file = file.removeprefix(input_files_base + "/") + input_files_list.append(relative_file) + out_dir = str(repository_ctx.path(_WORKSPACE_REROOTED_PATH)) + + # Write files list + input_files_list_file = "data_files.txt" + repository_ctx.file( + input_files_list_file, + content = "\n".join(input_files_list), + ) + + # Run copy script + result = repository_ctx.execute( + [ + repository_ctx.path(Label("//internal/npm_install:bulk_copy.sh")), + input_files_list_file, + input_files_base, + out_dir, + ], + quiet = repository_ctx.attr.quiet, + ) + + if result.return_code != 0: + fail("bulk_copy.sh failed: \nSTDOUT:\n%s\nSTDERR:\n%s" % (result.stdout, result.stderr)) def _add_node_repositories_info_deps(repository_ctx): # Add a dep to the node_info & yarn_info files from node_repositories @@ -590,6 +614,7 @@ def _npm_install_impl(repository_ctx): repository_ctx.path(Label("//internal/npm_install:index.js")) repository_ctx.path(Label("@nodejs_%s//:node_info" % os_name(repository_ctx))) repository_ctx.path(Label("@nodejs_%s//:yarn_info" % os_name(repository_ctx))) + repository_ctx.path(Label("//internal/npm_install:bulk_copy.sh")) is_windows_host = is_windows_os(repository_ctx) @@ -740,6 +765,7 @@ def _yarn_install_impl(repository_ctx): repository_ctx.path(Label("//internal/npm_install:index.js")) repository_ctx.path(Label("@nodejs_%s//:node_info" % os_name(repository_ctx))) repository_ctx.path(Label("@nodejs_%s//:yarn_info" % os_name(repository_ctx))) + repository_ctx.path(Label("//internal/npm_install:bulk_copy.sh")) is_windows_host = is_windows_os(repository_ctx) From 4627020c2f62a7ce3561b4084beccbde79d28b65 Mon Sep 17 00:00:00 2001 From: Jordan Mele Date: Mon, 4 Dec 2023 18:21:33 +1100 Subject: [PATCH 2/2] Formatting --- internal/npm_install/bulk_copy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/npm_install/bulk_copy.sh b/internal/npm_install/bulk_copy.sh index 20e8afda..43ffd3a9 100755 --- a/internal/npm_install/bulk_copy.sh +++ b/internal/npm_install/bulk_copy.sh @@ -9,4 +9,4 @@ input_files_base="$2" # Destination directory for files out_dir="$3" -tar --create --file - --directory "$input_files_base" --files-from "$input_files_list"| tar --extract --file - --directory "$out_dir" +tar --create --file - --directory "$input_files_base" --files-from "$input_files_list" | tar --extract --file - --directory "$out_dir"