From 04fdec9215143a062e033d3eda72dd8e0a2bf95d Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 5 Jun 2023 06:07:31 -0700 Subject: [PATCH] Remove output_to_genfiles from proto_library output_to_genfiles is deprecated. The result is that descriptor sets generated by proto_library move from genfiles dir to bindir. This is a no-op in Bazel, because bin_dir == genfiles_dir. PiperOrigin-RevId: 537852317 Change-Id: I460ea0df81ff0f797f642c828c684fac5f57251f --- .../starlark/builtins_bzl/common/cc/cc_helper.bzl | 6 +++--- .../builtins_bzl/common/cc/cc_proto_library.bzl | 10 +++++----- .../builtins_bzl/common/objc/j2objc_aspect.bzl | 1 - .../builtins_bzl/common/proto/proto_info.bzl | 14 +++++++------- .../builtins_bzl/common/proto/proto_library.bzl | 3 +-- 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl index 46f7c351487e50..d67bb9ba709050 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl @@ -1209,12 +1209,12 @@ def _copts_filter(ctx, additional_make_variable_substitutions): # Expand nocopts and create CoptsFilter. return _expand(ctx, nocopts, additional_make_variable_substitutions) -def _proto_output_root(proto_root, genfiles_dir_path, bin_dir_path): +def _proto_output_root(proto_root, bin_dir_path): if proto_root == ".": return bin_dir_path - if proto_root.startswith(genfiles_dir_path): - return bin_dir_path + "/" + proto_root[len(genfiles_dir_path):] + if proto_root.startswith(bin_dir_path): + return proto_root else: return bin_dir_path + "/" + proto_root diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_proto_library.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_proto_library.bzl index e0bb4ac7a50b4e..48f0c8e4086f04 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_proto_library.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_proto_library.bzl @@ -49,13 +49,13 @@ def _check_proto_libraries_in_deps(deps): def _create_proto_compile_action(ctx, outputs, proto_info): proto_root = proto_info.proto_source_root - if proto_root.startswith(ctx.genfiles_dir.path): - genfiles_path = proto_root + if proto_root.startswith(ctx.bin_dir.path): + path = proto_root else: - genfiles_path = ctx.genfiles_dir.path + "/" + proto_root + path = ctx.bin_dir.path + "/" + proto_root if proto_root == ".": - genfiles_path = ctx.genfiles_dir.path + path = ctx.bin_dir.path if len(outputs) != 0: proto_common.compile( @@ -63,7 +63,7 @@ def _create_proto_compile_action(ctx, outputs, proto_info): proto_info = proto_info, proto_lang_toolchain_info = ctx.attr._aspect_cc_proto_toolchain[ProtoLangToolchainInfo], generated_files = outputs, - plugin_output = genfiles_path, + plugin_output = path, ) def _get_output_files(ctx, target, suffixes): diff --git a/src/main/starlark/builtins_bzl/common/objc/j2objc_aspect.bzl b/src/main/starlark/builtins_bzl/common/objc/j2objc_aspect.bzl index 75597f319b0704..5913032d311171 100644 --- a/src/main/starlark/builtins_bzl/common/objc/j2objc_aspect.bzl +++ b/src/main/starlark/builtins_bzl/common/objc/j2objc_aspect.bzl @@ -76,7 +76,6 @@ def _proto(target, ctx): filtered_proto_sources, _ = proto_common.experimental_filter_sources(target[ProtoInfo], proto_lang_toolchain_info) objc_file_path = cc_helper.proto_output_root( proto_root = target[ProtoInfo].proto_source_root, - genfiles_dir_path = ctx.genfiles_dir.path, bin_dir_path = ctx.bin_dir.path, ) j2objc_source = _proto_j2objc_source(ctx, target[ProtoInfo], filtered_proto_sources, objc_file_path) diff --git a/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl b/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl index 5fe40ef3d1150e..1fb669ee8e37e9 100644 --- a/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl +++ b/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl @@ -44,7 +44,7 @@ def _from_root(root, repo, relpath): # - with sibling layout: `{root}/package/path` return _join(root, "" if repo.startswith("../") else repo, relpath) -def _create_proto_info(*, srcs, deps, descriptor_set, proto_path = "", workspace_root = "", genfiles_dir = None): +def _create_proto_info(*, srcs, deps, descriptor_set, proto_path = "", workspace_root = "", bin_dir = None): """Constructs ProtoInfo. Args: @@ -55,7 +55,7 @@ def _create_proto_info(*, srcs, deps, descriptor_set, proto_path = "", workspace stripping is needed, the files should be symlinked into `_virtual_imports/target_name` directory. Only such paths are accepted. workspace_root: (str) Set to ctx.workspace_root if this is not the main repository. - genfiles_dir: (str) Set to ctx.genfiles_dir if _virtual_imports are used. + bin_dir: (str) Set to ctx.bin_dir if _virtual_imports are used. Returns: (ProtoInfo) @@ -77,8 +77,8 @@ def _create_proto_info(*, srcs, deps, descriptor_set, proto_path = "", workspace fail("proto_path needs to contain '_virtual_imports' directory") if proto_path.split("/")[-2] != "_virtual_imports": fail("proto_path needs to be formed like '_virtual_imports/target_name'") - if not genfiles_dir: - fail("genfiles_dir parameter should be set when _virtual_imports are used") + if not bin_dir: + fail("bin_dir parameter should be set when _virtual_imports are used") direct_proto_sources = srcs transitive_proto_sources = depset( @@ -117,8 +117,8 @@ def _create_proto_info(*, srcs, deps, descriptor_set, proto_path = "", workspace exported_sources = depset(transitive = [dep._exported_sources for dep in deps]) if "_virtual_imports/" in proto_path: - #TODO(b/281812523): remove genfiles_dir from proto_source_root (when users assuming it's there are migrated) - proto_source_root = _empty_to_dot(_from_root(genfiles_dir, workspace_root, proto_path)) + #TODO(b/281812523): remove bin_dir from proto_source_root (when users assuming it's there are migrated) + proto_source_root = _empty_to_dot(_from_root(bin_dir, workspace_root, proto_path)) elif workspace_root.startswith("../"): proto_source_root = proto_path else: @@ -155,7 +155,7 @@ ProtoInfo, _ = provider( `import c/d.proto` In principle, the `proto_source_root` directory itself should always - be relative to the output directory (`ctx.bin_dir` or `ctx.genfiles_dir`). + be relative to the output directory (`ctx.bin_dir`). This is at the moment not true for `proto_libraries` using (additional and/or strip) import prefixes. `proto_source_root` is in this case prefixed with the output diff --git a/src/main/starlark/builtins_bzl/common/proto/proto_library.bzl b/src/main/starlark/builtins_bzl/common/proto/proto_library.bzl index 6014ac69574667..5d384f982f0146 100644 --- a/src/main/starlark/builtins_bzl/common/proto/proto_library.bzl +++ b/src/main/starlark/builtins_bzl/common/proto/proto_library.bzl @@ -77,7 +77,7 @@ def _proto_library_impl(ctx): descriptor_set = descriptor_set, proto_path = proto_path, workspace_root = ctx.label.workspace_root, - genfiles_dir = ctx.genfiles_dir.path, + bin_dir = ctx.bin_dir.path, ) _write_descriptor_set(ctx, proto_info, deps, exports, descriptor_set) @@ -237,6 +237,5 @@ proto_library = rule( }, **semantics.EXTRA_ATTRIBUTES), fragments = ["proto"] + semantics.EXTRA_FRAGMENTS, provides = [ProtoInfo], - output_to_genfiles = True, # TODO(b/204266604) move to bin dir exec_groups = semantics.EXEC_GROUPS, )