From 06d54eef4cc1b2693729b8d4eeb5338b04c8cc1d Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Mon, 1 Aug 2022 09:56:52 -0700 Subject: [PATCH] fix: improve handling of duplicate files in copy_to_directory (#205) --- lib/private/copy_to_directory.bzl | 31 +++++++++++++++---- .../copy_to_directory_action/BUILD.bazel | 9 ++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/lib/private/copy_to_directory.bzl b/lib/private/copy_to_directory.bzl index c3a2bc9..ab528be 100644 --- a/lib/private/copy_to_directory.bzl +++ b/lib/private/copy_to_directory.bzl @@ -412,6 +412,25 @@ def _copy_paths( return src_path, output_path, src_file +def _merge_into_copy_path(copy_paths, src_path, dst_path, src_file): + for i, s in enumerate(copy_paths): + _, maybe_dst_path, maybe_src_file = s + if dst_path == maybe_dst_path: + if src_file == maybe_src_file: + return True + if src_file.short_path == maybe_src_file.short_path: + if maybe_src_file.is_source and not src_file.is_source: + # If the files are the at the same path but one in the source tree and one in + # the output tree, always copy the output tree file. This is also the default + # Bazel behavior for layout out runfiles if there are files that have the same + # path in the source tree and the output tree. This can happen, for example, if + # the source file and a generated file that is a copy to the source file are + # both added to the package which can happen, for example, through 'additional_files' + # in 'copy_to_directory_action'. + copy_paths[i] = (src_path, dst_path, src_file) + return True + return False + def _copy_to_dir_bash(ctx, copy_paths, dst_dir, allow_overwrites): cmds = [ "set -o errexit -o nounset -o pipefail", @@ -682,7 +701,8 @@ def copy_to_directory_action( ) if src_path != None: dst_path = skylib_paths.normalize("/".join([dst.path, output_path])) - copy_paths.append((src_path, dst_path, src_file)) + if not _merge_into_copy_path(copy_paths, src_path, dst_path, src_file): + copy_paths.append((src_path, dst_path, src_file)) if DefaultInfo in src: for src_file in src[DefaultInfo].files.to_list(): found_input_paths = True @@ -698,11 +718,9 @@ def copy_to_directory_action( ) if src_path != None: dst_path = skylib_paths.normalize("/".join([dst.path, output_path])) - copy_paths.append((src_path, dst_path, src_file)) + if not _merge_into_copy_path(copy_paths, src_path, dst_path, src_file): + copy_paths.append((src_path, dst_path, src_file)) for additional_file in additional_files: - if additional_file in ctx.files.srcs: - # already added above - continue found_input_paths = True src_path, output_path, src_file = _copy_paths( src = additional_file, @@ -716,7 +734,8 @@ def copy_to_directory_action( ) if src_path != None: dst_path = skylib_paths.normalize("/".join([dst.path, output_path])) - copy_paths.append((src_path, dst_path, src_file)) + if not _merge_into_copy_path(copy_paths, src_path, dst_path, src_file): + copy_paths.append((src_path, dst_path, src_file)) if not found_input_paths: fail("No files or directories found in srcs.") diff --git a/lib/tests/copy_to_directory_action/BUILD.bazel b/lib/tests/copy_to_directory_action/BUILD.bazel index 1bf06b4..52a4fa2 100644 --- a/lib/tests/copy_to_directory_action/BUILD.bazel +++ b/lib/tests/copy_to_directory_action/BUILD.bazel @@ -1,14 +1,23 @@ load("//lib:diff_test.bzl", "diff_test") load("//lib:copy_to_directory.bzl", "copy_to_directory") +load("//lib:copy_to_bin.bzl", "copy_to_bin") load(":lib.bzl", "lib") load(":pkg.bzl", "pkg") +copy_to_bin( + name = "copy_1", + srcs = ["1"], +) + lib( name = "lib", srcs = ["1"], # intentionally dup on "1" to make sure it is gracefully handled others = [ "1", + # also pass in a copy_to_bin copy of "1" to spice things up; + # this case is handled in the fix in https://github.com/aspect-build/bazel-lib/pull/205 + "copy_1", "2", ], )