From 323329f119e3db812b56b5d0cf38e9c6e4c3c488 Mon Sep 17 00:00:00 2001 From: Douglas Parker Date: Tue, 10 Oct 2023 07:16:13 -0700 Subject: [PATCH] fix: always include files from the same workspace as the build target in `copy_to_directory()` (#360) * fix: always include files from the same workspace as the build target in `copy_to_directory` Fixes #359. This updates the `copy_to_directory` tool to accept a workspace name representing the workspace of the target it is executing under. Any files in this workspace are automatically included, regardless of the `include_external_repositories` option. This makes it support usage within an external target (such as `@wksp//:dir`). * test: add e2e test which uses `copy_to_directory` within an external workspace Refs #359. This should catch regressions where no files are copied when built within an external workspace and not using `include_external_repositories`. * ci: fix stray workspace refs --------- Co-authored-by: Alex Eagle --- .aspect/workflows/config.yaml | 4 +++ .github/workflows/ci.yaml | 1 + docs/copy_to_directory.md | 2 +- e2e/external_copy_to_directory/.bazelignore | 1 + e2e/external_copy_to_directory/.bazelrc | 0 e2e/external_copy_to_directory/.bazelversion | 1 + e2e/external_copy_to_directory/BUILD.bazel | 13 +++++++ e2e/external_copy_to_directory/WORKSPACE | 28 +++++++++++++++ e2e/external_copy_to_directory/app/.bazelrc | 0 .../app/.bazelversion | 1 + .../app/BUILD.bazel | 6 ++++ e2e/external_copy_to_directory/app/WORKSPACE | 31 ++++++++++++++++ e2e/external_copy_to_directory/app/test.sh | 19 ++++++++++ e2e/external_copy_to_directory/directory.bzl | 36 +++++++++++++++++++ e2e/external_copy_to_directory/foo.txt | 1 + lib/private/copy_to_directory.bzl | 4 +-- 16 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 e2e/external_copy_to_directory/.bazelignore create mode 100644 e2e/external_copy_to_directory/.bazelrc create mode 120000 e2e/external_copy_to_directory/.bazelversion create mode 100644 e2e/external_copy_to_directory/BUILD.bazel create mode 100644 e2e/external_copy_to_directory/WORKSPACE create mode 100644 e2e/external_copy_to_directory/app/.bazelrc create mode 120000 e2e/external_copy_to_directory/app/.bazelversion create mode 100644 e2e/external_copy_to_directory/app/BUILD.bazel create mode 100644 e2e/external_copy_to_directory/app/WORKSPACE create mode 100755 e2e/external_copy_to_directory/app/test.sh create mode 100644 e2e/external_copy_to_directory/directory.bzl create mode 100644 e2e/external_copy_to_directory/foo.txt diff --git a/.aspect/workflows/config.yaml b/.aspect/workflows/config.yaml index 6148051..a971e20 100644 --- a/.aspect/workflows/config.yaml +++ b/.aspect/workflows/config.yaml @@ -13,6 +13,10 @@ workspaces: - e2e/copy_to_directory: gazelle: without: true + - e2e/external_copy_to_directory: + gazelle: + without: true + bazel: flags: - --remote_download_minimal diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 31ce7ce..e5b0398 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -88,6 +88,7 @@ jobs: - "." - "e2e/coreutils" - "e2e/copy_to_directory" + - "e2e/external_copy_to_directory" - "e2e/smoke" exclude: # Don't test MacOS with RBE to minimize MacOS minutes (billed at 10X) diff --git a/docs/copy_to_directory.md b/docs/copy_to_directory.md index db4f3a5..e7fb748 100644 --- a/docs/copy_to_directory.md +++ b/docs/copy_to_directory.md @@ -59,7 +59,7 @@ for more information on supported globbing patterns. | exclude_srcs_packages | List of Bazel packages (with glob support) to exclude from output directory.

Files in srcs are not copied to the output directory if the Bazel package of the file matches one of the patterns specified.

Forward slashes (/) should be used as path separators. A first character of "." will be replaced by the target's package path.

Files that have do not have matching Bazel packages are subject to subsequent filters and transformations to determine if they are copied and what their path in the output directory will be.

Globs are supported (see rule docstring above). | List of strings | optional | [] | | exclude_srcs_patterns | List of paths (with glob support) to exclude from output directory.

Files in srcs are not copied to the output directory if their output directory path, after applying root_paths, matches one of the patterns specified.

Forward slashes (/) should be used as path separators.

Files that do not have matching output directory paths are subject to subsequent filters and transformations to determine if they are copied and what their path in the output directory will be.

Globs are supported (see rule docstring above). | List of strings | optional | [] | | hardlink | Controls when to use hardlinks to files instead of making copies.

Creating hardlinks is much faster than making copies of files with the caveat that hardlinks share file permissions with their source.

Since Bazel removes write permissions on files in the output tree after an action completes, hardlinks to source files are not recommended since write permissions will be inadvertently removed from sources files.

- auto: hardlinks are used for generated files already in the output tree - off: all files are copied - on: hardlinks are used for all files (not recommended) | String | optional | "auto" | -| include_external_repositories | List of external repository names (with glob support) to include in the output directory.

Files from external repositories are only copied into the output directory if the external repository they come from matches one of the external repository patterns specified.

When copied from an external repository, the file path in the output directory defaults to the file's path within the external repository. The external repository name is _not_ included in that path.

For example, the following copies @external_repo//path/to:file to path/to/file within the output directory.

 copy_to_directory(     name = "dir",     include_external_repositories = ["external_*"],     srcs = ["@external_repo//path/to:file"], )


Files that come from matching external are subject to subsequent filters and transformations to determine if they are copied and what their path in the output directory will be. The external repository name of the file from an external repository is not included in the output directory path and is considered in subsequent filters and transformations.

Globs are supported (see rule docstring above). | List of strings | optional | [] | +| include_external_repositories | List of external repository names (with glob support) to include in the output directory.

Files from external repositories are only copied into the output directory if the external repository they come from matches one of the external repository patterns specified or if they are in the same external repository as this target.

When copied from an external repository, the file path in the output directory defaults to the file's path within the external repository. The external repository name is _not_ included in that path.

For example, the following copies @external_repo//path/to:file to path/to/file within the output directory.

 copy_to_directory(     name = "dir",     include_external_repositories = ["external_*"],     srcs = ["@external_repo//path/to:file"], )


Files that come from matching external are subject to subsequent filters and transformations to determine if they are copied and what their path in the output directory will be. The external repository name of the file from an external repository is not included in the output directory path and is considered in subsequent filters and transformations.

Globs are supported (see rule docstring above). | List of strings | optional | [] | | include_srcs_packages | List of Bazel packages (with glob support) to include in output directory.

Files in srcs are only copied to the output directory if the Bazel package of the file matches one of the patterns specified.

Forward slashes (/) should be used as path separators. A first character of "." will be replaced by the target's package path.

Defaults to ["**"] which includes sources from all packages.

Files that have matching Bazel packages are subject to subsequent filters and transformations to determine if they are copied and what their path in the output directory will be.

Globs are supported (see rule docstring above). | List of strings | optional | ["**"] | | include_srcs_patterns | List of paths (with glob support) to include in output directory.

Files in srcs are only copied to the output directory if their output directory path, after applying root_paths, matches one of the patterns specified.

Forward slashes (/) should be used as path separators.

Defaults to ["**"] which includes all sources.

Files that have matching output directory paths are subject to subsequent filters and transformations to determine if they are copied and what their path in the output directory will be.

Globs are supported (see rule docstring above). | List of strings | optional | ["**"] | | out | Path of the output directory, relative to this package.

If not set, the name of the target is used. | String | optional | "" | diff --git a/e2e/external_copy_to_directory/.bazelignore b/e2e/external_copy_to_directory/.bazelignore new file mode 100644 index 0000000..908f7bb --- /dev/null +++ b/e2e/external_copy_to_directory/.bazelignore @@ -0,0 +1 @@ +app/ diff --git a/e2e/external_copy_to_directory/.bazelrc b/e2e/external_copy_to_directory/.bazelrc new file mode 100644 index 0000000..e69de29 diff --git a/e2e/external_copy_to_directory/.bazelversion b/e2e/external_copy_to_directory/.bazelversion new file mode 120000 index 0000000..96cf949 --- /dev/null +++ b/e2e/external_copy_to_directory/.bazelversion @@ -0,0 +1 @@ +../../.bazelversion \ No newline at end of file diff --git a/e2e/external_copy_to_directory/BUILD.bazel b/e2e/external_copy_to_directory/BUILD.bazel new file mode 100644 index 0000000..390f2e6 --- /dev/null +++ b/e2e/external_copy_to_directory/BUILD.bazel @@ -0,0 +1,13 @@ +load("@bazel_skylib//rules:build_test.bzl", "build_test") +load(":directory.bzl", "directory") + +directory( + name = "dir", + srcs = ["foo.txt"], + visibility = ["//visibility:public"], +) + +build_test( + name = "test", + targets = [":dir"], +) diff --git a/e2e/external_copy_to_directory/WORKSPACE b/e2e/external_copy_to_directory/WORKSPACE new file mode 100644 index 0000000..8399ec1 --- /dev/null +++ b/e2e/external_copy_to_directory/WORKSPACE @@ -0,0 +1,28 @@ +workspace(name = "external_copy_to_directory") + +local_repository( + name = "aspect_bazel_lib", + path = "../..", +) + +load("@aspect_bazel_lib//:internal_deps.bzl", "bazel_lib_internal_deps") + +bazel_lib_internal_deps() + +load("@aspect_bazel_lib//lib:repositories.bzl", "aspect_bazel_lib_dependencies") + +aspect_bazel_lib_dependencies() + +load("@aspect_bazel_lib//:deps.bzl", "go_dependencies") + +go_dependencies() + +load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") + +go_rules_dependencies() + +go_register_toolchains(version = "1.18.3") + +load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies") + +gazelle_dependencies() diff --git a/e2e/external_copy_to_directory/app/.bazelrc b/e2e/external_copy_to_directory/app/.bazelrc new file mode 100644 index 0000000..e69de29 diff --git a/e2e/external_copy_to_directory/app/.bazelversion b/e2e/external_copy_to_directory/app/.bazelversion new file mode 120000 index 0000000..2006aa2 --- /dev/null +++ b/e2e/external_copy_to_directory/app/.bazelversion @@ -0,0 +1 @@ +../../../.bazelversion \ No newline at end of file diff --git a/e2e/external_copy_to_directory/app/BUILD.bazel b/e2e/external_copy_to_directory/app/BUILD.bazel new file mode 100644 index 0000000..95a5eb4 --- /dev/null +++ b/e2e/external_copy_to_directory/app/BUILD.bazel @@ -0,0 +1,6 @@ +sh_test( + name = "test", + srcs = ["test.sh"], + data = ["@external_copy_to_directory//:dir"], + deps = ["@bazel_tools//tools/bash/runfiles"], +) diff --git a/e2e/external_copy_to_directory/app/WORKSPACE b/e2e/external_copy_to_directory/app/WORKSPACE new file mode 100644 index 0000000..71e0ede --- /dev/null +++ b/e2e/external_copy_to_directory/app/WORKSPACE @@ -0,0 +1,31 @@ +local_repository( + name = "external_copy_to_directory", + path = "..", +) + +local_repository( + name = "aspect_bazel_lib", + path = "../../..", +) + +load("@aspect_bazel_lib//:internal_deps.bzl", "bazel_lib_internal_deps") + +bazel_lib_internal_deps() + +load("@aspect_bazel_lib//lib:repositories.bzl", "aspect_bazel_lib_dependencies") + +aspect_bazel_lib_dependencies() + +load("@aspect_bazel_lib//:deps.bzl", "go_dependencies") + +go_dependencies() + +load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") + +go_rules_dependencies() + +go_register_toolchains(version = "1.18.3") + +load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies") + +gazelle_dependencies() diff --git a/e2e/external_copy_to_directory/app/test.sh b/e2e/external_copy_to_directory/app/test.sh new file mode 100755 index 0000000..d8dd6d5 --- /dev/null +++ b/e2e/external_copy_to_directory/app/test.sh @@ -0,0 +1,19 @@ +# --- begin runfiles.bash initialization v2 --- +# Copy-pasted from the Bazel Bash runfiles library v2. +set -uo pipefail; set +e; f=bazel_tools/tools/bash/runfiles/runfiles.bash +source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \ + source "$0.runfiles/$f" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ + { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e +# --- end runfiles.bash initialization v2 --- + +# Read external directory and make sure it exists with a file. +readonly DIR=$(rlocation external_copy_to_directory/dir/) +readonly FILES=$(ls "${DIR}" | wc -l) +if [[ ${FILES} != 1 ]]; then + echo "Expected exactly 1 file under external directory, but found ${FILES}:" >&2 + ls "${DIR}" >&2 + exit 1 +fi diff --git a/e2e/external_copy_to_directory/directory.bzl b/e2e/external_copy_to_directory/directory.bzl new file mode 100644 index 0000000..ec06e97 --- /dev/null +++ b/e2e/external_copy_to_directory/directory.bzl @@ -0,0 +1,36 @@ +"""Test rule executing `copy_to_directory_bin_action`.""" + +load("@aspect_bazel_lib//lib:copy_to_directory.bzl", "copy_to_directory_bin_action") + +def _directory_impl(ctx): + dst = ctx.actions.declare_directory(ctx.attr.name) + + copy_to_directory_bin_action( + ctx, + name = ctx.attr.name, + copy_to_directory_bin = ctx.executable._tool, + dst = dst, + files = ctx.files.srcs, + verbose = True, + ) + + return DefaultInfo(files = depset([dst])) + +directory = rule( + implementation = _directory_impl, + attrs = { + "srcs": attr.label_list( + mandatory = True, + allow_files = True, + ), + "_tool": attr.label( + executable = True, + cfg = "exec", + default = "@aspect_bazel_lib//tools/copy_to_directory", + ), + }, + doc = """ + Copies the given source files to a directory with + `copy_to_directory_bin_action()`. + """, +) diff --git a/e2e/external_copy_to_directory/foo.txt b/e2e/external_copy_to_directory/foo.txt new file mode 100644 index 0000000..257cc56 --- /dev/null +++ b/e2e/external_copy_to_directory/foo.txt @@ -0,0 +1 @@ +foo diff --git a/lib/private/copy_to_directory.bzl b/lib/private/copy_to_directory.bzl index 4a623cd..5aeb050 100644 --- a/lib/private/copy_to_directory.bzl +++ b/lib/private/copy_to_directory.bzl @@ -79,7 +79,7 @@ Globs are supported (see rule docstring above). Files from external repositories are only copied into the output directory if the external repository they come from matches one of the external repository patterns -specified. +specified or if they are in the same external repository as this target. When copied from an external repository, the file path in the output directory defaults to the file's path within the external repository. The external repository @@ -492,7 +492,7 @@ def copy_to_directory_bin_action( inputs = file_inputs + [config_file], outputs = [dst], executable = copy_to_directory_bin, - arguments = [config_file.path], + arguments = [config_file.path, ctx.label.workspace_name], mnemonic = "CopyToDirectory", progress_message = "Copying files to directory %s" % _progress_path(dst), execution_requirements = override_execution_requirements or execution_requirements_for_copy(ctx),