Add MODULE.bazel files

[BalestraPatrick]

bazelbuild/bazel-central-registry#63 added apple_support to the BCR with some patches. This PR will allow to remove the patch from BCR with the next release.

[keith]

huh this seems strange, I'm surprised it's required

[BalestraPatrick]

I took inspiration from what was done in here: bazelbuild/bazel-central-registry#60

It's possible that this is only needed when using this as a patch inside of BCR, and might not be needed if we check it in here.

[keith]

shouldn't this "inheret" from the primary MODULE.bazel?

[BalestraPatrick]

Actually, rules_jni does the same thing in their tests module: https://github.com/fmeum/rules_jni/blob/6c457ed979d9e5648b1162b42018509a059cb2e6/tests/MODULE.bazel#L8-L11

[keith]

Deleting this entire file and running USE_BAZEL_VERSION=5.0.0rc4 bazelisk test ... --experimental_enable_bzlmod seems to still work for me, how do you repro needing it?

[BalestraPatrick]

At some point in bazelbuild/bazel-central-registry#63 we had @apple_support//... in the presubmit and it was failing on CI: https://buildkite.com/bazel/bcr-presubmit/builds/166#8b97a65f-9e9d-4f26-90fb-79f8eaea9aea

@meteorcloudy suggested having a patch that would expose it in BCR, but I wanted to have it in this repo which is of course better than a patch.

[BalestraPatrick]

Actually added it in the BCR PR as well: bazelbuild/bazel-central-registry#63

[meteorcloudy]

Oh, I see the confusion here.

The MODULE.bazel file doesn't yet replace the WORKSPACE file's functionality to mark the workspace root.

In rules_jni/tests, there is an additional WORKSPACE file there. So that's actually a new Bazel project that depends on the main project as if it's an external dependency.

But "apple_support/test" is only a part of the main workspace. Without a WORKSPACE file, the test/MODULE.bazel won't any any effect.

So there are two ways forward:

1. Keep apple_support/test as a sub package of the main repo. But in order to run those tests with Bzlmod, you'll have to avoid referencing the main repo as @build_bazel_apple_support// (it should just be //). Because @build_bazel_apple_support won't be available to the module itself (@<module name> is available, in this case it's "apple_support").

2. Add a WORKSPACE to apple_support/test. Then the test/MODULE.bazel file actually works, and you can add bazel_dep(name = "apple_support", version = "", repo_name = "build_bazel_apple_support") to make the expected repo name available.

I would recommend option 2. Because it's testing the target module with a more real world use case. And in case you need additional test dependencies, you can add them in test/MODULE.bazel, instead of polluting the MODULE.bazel file of the target module. WDYT?

FYI, there are a lot of discussion about how should we test modules in the BCR: https://groups.google.com/a/bazel.build/g/external-deps/c/GcV1cpRd-Ls

[keith]

For option 1, I don't have a strong preference of how labels are referenced from the root, but I am currently under the impression that using the fully qualified label is required in some cases? I know there has been some push to change the names of repositories when adding them to the BCR, but this makes me feel like we should actually just maintain the build_bazel_apple_support name so that we don't have this confusion.

How would option 2 work in a more complex case where your tests directory references labels outside of the tests dir, like if you had tools/ beside it that were used in the normal rules and the tests? I definitely see the advantage if we had test dependencies, but without it that currently just seems like more for us to maintain in this repo.

[meteorcloudy]

but I am currently under the impression that using the fully qualified label is required in some cases?

If you are just running as a subpackage of the main repo, I'm sure you don't need the "@build_bazel_apple_support" part to reference targets in your own repo, if you do, we need to figure out why and fix the bug.

How would option 2 work in a more complex case where your tests directory references labels outside of the tests dir, like if you had tools/ beside it that were used in the normal rules and the tests?

With option 2, if it's outside of the test directory, it belongs to the main module right? When the test module depends on the main module, you should just reference the target as @build_bazel_apple_support//tools:foo. Does that make sense?

I definitely see the advantage if we had test dependencies, but without it that currently just seems like more for us to maintain in this repo.

Yeah, I don't have strong preference for apple_support since it's quite small. I'm fine with keeping the tests in a subpackage, it's just you'll have to remove the @build_bazel_apple_support references to make them work with Bzlmod. See a similar change we made to protobuf: protocolbuffers/protobuf#9187

[keith]

so i guess we have to bump this and skylib in sync with new versions in the future?