fix(write_source_files): fix writing to workspace root #53
Conversation
| cmd = "mkdir -p $@ && echo 'test' > $@/test.txt", | ||
| visibility = ["//visibility:private"], | ||
| ) | ||
| write_source_files( |
There was a problem hiding this comment.
@gregmagolan did you actually get your write_dist running as a test? This only failed previously when doing bazel run and not bazel test...
There was a problem hiding this comment.
Needs an integration test but I'd prefer to avoid the added complexity of bazel-in-bazel
jbedard
force-pushed
the
write-source-root
branch
from
dd298c5
to
4eb8024
Compare
| name = "write_source_file_root-test", | ||
| diff_test = False, | ||
| files = {"test-out/dist/write_source_file_root-test": ":write_source_file_root"}, | ||
| ) |
There was a problem hiding this comment.
Want to add //visibility:private to this as well?
There was a problem hiding this comment.
Shouldn't tests be public?
There was a problem hiding this comment.
Hm, the angle I was gettting at was whether this should be available to anyone who depends on our repository.
There was a problem hiding this comment.
It is private by default unless there is a default_visiblity in the BUILD to set another default
|
Nothing actually runs this test, right? I'm wondering if we should just leave it out of WORKSPACE. |
Or at least call it out as a manual test that ought to be some sort of e2e test. |
|
This is the same as what @gregmagolan did in |
That should be pretty easy to do. Just add another run step in - name: manual integration test
run: bazel run //:write_source_file_root-test |
lib/private/write_source_file.bzl
Outdated
| @@ -266,7 +266,7 @@ def _write_source_file_impl(ctx): | |||
| else: | |||
| fail("in file %s must be a single file or a target that provides DefaultOutputPathInfo or DirectoryPathInfo" % ctx.attr.in_file.label) | |||
|
|
|||
| out_path = "/".join([ctx.label.package, ctx.attr.out_file]) | |||
| out_path = "/".join([ctx.label.package, ctx.attr.out_file]) if len(ctx.label.package) > 0 else ctx.attr.out_file | |||
There was a problem hiding this comment.
I believe you can shorten this to
out_path = "/".join([ctx.label.package, ctx.attr.out_file]) if ctx.label.package else ctx.attr.out_file
empty string is a falsy in starlark
jbedard
force-pushed
the
write-source-root
branch
2 times, most recently
from
86bc900
to
50b5490
Compare
jbedard
force-pushed
the
write-source-root
branch
from
50b5490
to
2851427
Compare
Currently contains #54, see the second commit.