bazel-skylib icon indicating copy to clipboard operation
bazel-skylib copied to clipboard

bzl_library "builds" with missing deps

Open thomasvl opened this issue 6 years ago • 4 comments

As seen in #49, you can update a .bzl file to add another load statement, but failing to add the dep for the skylark_library doesn't cause the build to break. If the skylark_library is then used to copy files for something like integration tests, those will fail as the file won't be found in the integration test's sandbox to be load()ed.

thomasvl avatar Jun 13 '18 18:06 thomasvl

Ideally skylark_library should generate an action that scans its srcs .bzl files for all direct load() targets, and validates that those all appear among the transitive closure of deps. (Indirect load() targets are covered inductively by the other skylark_library targets that are depended on.)

brandjon avatar Jun 13 '18 19:06 brandjon

Looks like deps can be source files directly or skylark_library labels. So to handling indirect you'd also have to double check the deps for direct files and check those.

You'd likely also end up making it strict deps. right now load() of something that is a transitive deps would still work fine, but would be harder to validate without doing all the indirect handing oneself.

thomasvl avatar Jun 13 '18 19:06 thomasvl

I didn't realize .bzl files could appear as deps entries. I don't know what other rules do, but I imagine we should validate the .bzl file the same way regardless of whether it's listed as a dep or a src. Namely, a .bzl in deps should only load() other .bzl files transitively available from deps.

I would say that if a .bzl file appears only as a dep rather than a src, then the skylark_library should not be considered as providing that .bzl file for the purpose of validating other downstream targets. Drawing this distinction would probably require reworking the schema of SkylarkLibraryInfo.

As for strict-deps, it might be a good idea. But if we don't want to implement it that way, we could also dump out the depset of transitively-provided .bzls to a file that the validation script would check. I don't think there's much indirect handling beyond that.

brandjon avatar Jun 13 '18 19:06 brandjon

See also #16 (explicit request for strict deps in bzl_library).

Would a validation script require interpreter support?

tetromino avatar Apr 17 '21 16:04 tetromino