-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Separate testing module and bzlmod tests for toolchains/posix #323
Conversation
The previous test case was too lenient and would also succeed if the resolved toolchain was not provided by rules_nixpkgs but instead the default toolchain generated within a Nix shell.
Before common `.bazelrc` configuration was shared by making `.bazelrc` a symlink to a common configuration file. However, this approach had a few short-comings: 1) Any required modification meant that the symlink no longer worked and instead the file had to be replicated and adapted. Leading to duplicated code. 2) `%workspace%` relative paths had to be valid relative to all workspaces using the file. In this new setup common parts are factored out into `.bazelrc.common`, `.bazelrc.cc`, and `.bazelrc.java` in the top-level and can be imported into the workspace specific `.bazelrc` files as needed.
🎉 All dependencies have been resolved ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
import %workspace%/.bazelrc.common | ||
import %workspace%/.bazelrc.cc | ||
import %workspace%/.bazelrc.java |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is much cleaner.
def _non_module_deps_impl(ctx): | ||
nixpkgs_repositories(bzlmod = True) | ||
|
||
non_module_deps = module_extension( | ||
implementation = _non_module_deps_impl, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is implied by your TODO
comment in testing/posix/MODULE.bazel
, but is the idea to later remove this non_module_deps
extension and directly call the various *_configure
macros that nixpkgs_repositories
currently invokes once they've been modularised?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly, that's the plan once #183 is implemented (or as part of it).
Depends on #320
Creates a new module in
testing/posix
and moves the tests fromtoolchains/posix
into this module following the motivation from tweag/rules_sh#41.Makes the POSIX toolchain registration optional, as the corresponding
native.register_toolchains
cannot be called in a repository macro under bzlmod.Refactors the
.bazelrc
configuration to avoid duplication. See the corresponding commit message for details.