-
Notifications
You must be signed in to change notification settings - Fork 574
Publish 0.13.0 release of rules_pkl
#6290
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
Conversation
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.
Code Review
This pull request publishes version 0.13.0 of the rules_pkl module. The changes include adding the new version's MODULE.bazel, presubmit.yml, and source.json files, and updating metadata.json. The changes are mostly correct, but I've found one issue in MODULE.bazel regarding the use of top-level http_archive calls for development dependencies, which is not idiomatic for bzlmod and should be refactored for better clarity and maintainability.
| http_archive = use_repo_rule("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") | ||
|
|
||
| hawkeye_coordinates = [ | ||
| { | ||
| "name": "hawkeye-aarch64-apple-darwin", | ||
| "url": "https://github.com/korandoru/hawkeye/releases/download/v6.0.2/hawkeye-aarch64-apple-darwin.tar.xz", | ||
| "sha256": "9f300ad84201e500c4eb09ed710239797d2a6a17de5b7c08e1d3af1704ca6bb7", | ||
| }, | ||
| { | ||
| "name": "hawkeye-x86_64-apple-darwin", | ||
| "url": "https://github.com/korandoru/hawkeye/releases/download/v6.0.2/hawkeye-x86_64-apple-darwin.tar.xz", | ||
| "sha256": "6f8850adb9204b6218f4912fb987106a71fcd8d742ce20227697ddccc44cec38", | ||
| }, | ||
| { | ||
| "name": "hawkeye-x86_64-unknown-linux-gnu", | ||
| "url": "https://github.com/korandoru/hawkeye/releases/download/v6.0.2/hawkeye-x86_64-unknown-linux-gnu.tar.xz", | ||
| "sha256": "d5b9d3cdb4293ac84fa27e5021c77b6c062c61071760ab05fcacfbec1ac14850", | ||
| }, | ||
| ] | ||
|
|
||
| [ | ||
| http_archive( | ||
| name = coords["name"], | ||
| build_file = "//scripts:hawkeye.BUILD", | ||
| sha256 = coords["sha256"], | ||
| strip_prefix = coords["name"], | ||
| url = coords["url"], | ||
| ) | ||
| for coords in hawkeye_coordinates | ||
| ] |
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.
Using http_archive at the top level of a MODULE.bazel file is discouraged. These repository rules are ignored when rules_pkl is used as a transitive dependency, which makes them behave like development dependencies. However, this is not the idiomatic way to declare dev dependencies in bzlmod and can be confusing.
The correct approach is to move this logic into a module extension defined in a .bzl file, and then use that extension with dev_dependency = True.
For example, you could create a //tools:extensions.bzl file in your repository:
# //tools:extensions.bzl
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
hawkeye_coordinates = [
{
"name": "hawkeye-aarch64-apple-darwin",
"url": "https://github.com/korandoru/hawkeye/releases/download/v6.0.2/hawkeye-aarch64-apple-darwin.tar.xz",
"sha256": "9f300ad84201e500c4eb09ed710239797d2a6a17de5b7c08e1d3af1704ca6bb7",
},
{
"name": "hawkeye-x86_64-apple-darwin",
"url": "https://github.com/korandoru/hawkeye/releases/download/v6.0.2/hawkeye-x86_64-apple-darwin.tar.xz",
"sha256": "6f8850adb9204b6218f4912fb987106a71fcd8d742ce20227697ddccc44cec38",
},
{
"name": "hawkeye-x86_64-unknown-linux-gnu",
"url": "https://github.com/korandoru/hawkeye/releases/download/v6.0.2/hawkeye-x86_64-unknown-linux-gnu.tar.xz",
"sha256": "d5b9d3cdb4293ac84fa27e5021c77b6c062c61071760ab05fcacfbec1ac14850",
},
]
def _dev_tools_impl(_ctx):
for coords in hawkeye_coordinates:
http_archive(
name = coords["name"],
build_file = "//scripts:hawkeye.BUILD",
sha256 = coords["sha256"],
strip_prefix = coords["name"],
url = coords["url"],
)
dev_tools = module_extension(implementation = _dev_tools_impl)And then in MODULE.bazel:
# MODULE.bazel
dev_tools = use_extension("//tools:extensions.bzl", "dev_tools", dev_dependency = True)
use_repo(
dev_tools,
"hawkeye-aarch64-apple-darwin",
"hawkeye-x86_64-apple-darwin",
"hawkeye-x86_64-unknown-linux-gnu",
)This makes it explicit that hawkeye is a development dependency and aligns with modern bzlmod best practices.
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.
All modules in this PR have been approved by their maintainers. This PR will be merged if all presubmit checks pass.
|
@bazel-io skip_check incompatible_flags |
Release notes: https://github.com/apple/rules_pkl/releases/tag/v0.13.0