Add default_info_files rule & DirectoryPathInfo support in write_source_files and diff_test #48
Conversation
gregmagolan
force-pushed
the
write_source_files_feats
branch
from
c632746
to
ded1c93
Compare
gregmagolan
changed the title
gregmagolan
force-pushed
the
write_source_files_feats
branch
3 times, most recently
from
24cfaa7
to
a4f0e7e
Compare
gregmagolan
changed the title
lib/write_source_files.bzl
Outdated
| if not single_target: | ||
| _write_source_files( | ||
| name = name, | ||
| additional_update_targets = update_targets + additional_update_targets, | ||
| is_windows = select({ | ||
| "@bazel_tools//src/conditions:host_windows": True, | ||
| "//conditions:default": False, | ||
| }), | ||
| visibility = kwargs.get("visibility"), | ||
| tags = kwargs.get("tags"), | ||
| ) |
lib/private/write_source_files.bzl
Outdated
| _WriteSourceFilesInfo = provider( | ||
| WriteSourceFilesInfo = provider( |
There was a problem hiding this comment.
Should we expose this if it isn't used elsewhere?
There was a problem hiding this comment.
The rename was mainly so the error message when a target doesn't provide this is clearer. David at Robinhood recently tried to pipe a generated_file_test into the additional_update_targets of a write_source_files. It should error out and say that a WriteSourceFilesInfo provider must be provided.
I suppose we could just accept any old "binary" here but I think I prefer to set the provider as required so that uses are forced to be homogeneous
lib/default_info_files.bzl
Outdated
| """A rule that provides file(s) from a target's DefaultInfo | ||
| """ | ||
|
|
||
| load( | ||
| "//lib/private:default_info_files.bzl", | ||
| _default_info_files = "default_info_files", | ||
| _make_default_info_files = "make_default_info_files", | ||
| ) |
There was a problem hiding this comment.
@alexeagle I think this is what we discussed doing for your proto write_source_files case but then decided not to do it. I suppose you could use this now that we have it.
There was a problem hiding this comment.
👍 Piping the output of protobuf rules into write_source_files was the motivation for all of this
gregmagolan
force-pushed
the
write_source_files_feats
branch
from
a4f0e7e
to
8f0f132
Compare
gregmagolan
force-pushed
the
write_source_files_feats
branch
from
8f0f132
to
f84e0d1
Compare
gregmagolan
force-pushed
the
write_source_files_feats
branch
3 times, most recently
from
a70121c
to
ef16d76
Compare
…ccept it Also update write_source_files to accept DirectoryPathInfo
gregmagolan
force-pushed
the
write_source_files_feats
branch
from
ef16d76
to
1fc47c3
Compare
This will makes write_source_files more useful.
A bit of refactoring to update write_source_files to take a single in_file and out_file to simplify the implementation of the rule so that adding DirectoryPathInfo does not over complicate it. The macro API of
write_source_filesremains unchanged. The complication of updating multiple files is moved to the macro whereadditional_update_targetsis used to support updating multiple files. You now get 1 diff_test target for 1 update target in the graph which is also nice.