Skip to content

Conversation

@KushalP
Copy link
Contributor

@KushalP KushalP commented Oct 24, 2025

@bazel-io
Copy link
Member

Hello @holzensp, @sitaktif, @shs96c, @bioball, @stackoverflow, @srueg, modules you maintain (rules_pkl) have been updated in this PR.
Please review the changes. You can view a diff against the previous version in the "Generate module diff" check.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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.

Comment on lines +117 to +146
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
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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.

Copy link
Member

@bazel-io bazel-io left a 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.

@KushalP
Copy link
Contributor Author

KushalP commented Oct 24, 2025

@bazel-io skip_check incompatible_flags

@bazel-io bazel-io added the skip-incompatible-flags-test Bypass the incompatible flags test in BCR presubmit label Oct 24, 2025
@bazel-io bazel-io merged commit 0df49da into bazelbuild:main Oct 24, 2025
20 checks passed
@bazel-io bazel-io added the auto-merged This PR is automatically merged by the BCR reviewer bot. label Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merged This PR is automatically merged by the BCR reviewer bot. skip-incompatible-flags-test Bypass the incompatible flags test in BCR presubmit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants