Skip to content

Commit

Permalink
Remove output_to_genfiles from proto_library
Browse files Browse the repository at this point in the history
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
  • Loading branch information
comius authored and copybara-github committed Jun 5, 2023
1 parent 50c16b3 commit 04fdec9
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 18 deletions.
6 changes: 3 additions & 3 deletions src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 5 additions & 5 deletions src/main/starlark/builtins_bzl/common/cc/cc_proto_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,21 @@ 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(
actions = ctx.actions,
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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 7 additions & 7 deletions src/main/starlark/builtins_bzl/common/proto/proto_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
Expand All @@ -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(
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
)

0 comments on commit 04fdec9

Please sign in to comment.