Skip to content
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

Load py_runtime/py_runtime_pair from rules_python #1189

Merged
merged 3 commits into from
Jan 18, 2025

Conversation

mortenmj
Copy link
Contributor

@mortenmj mortenmj commented Dec 9, 2024

Fixes #1188

@michaelschuett-tomtom
Copy link

Hey, I just ran into this one as well. Would be awesome to get this landed as it looks to be blocking me from moving to bazel 8.x because of being pulled in by https://github.com/bazel-contrib/rules_jsonnet/tree/master. Although possibly that repo should actually change something so that BUILD files in this repo don't matter.

Copy link
Collaborator

@johnbartholomew johnbartholomew left a comment

Choose a reason for hiding this comment

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

In platform_defs/BUILD, the default_python3_toolchain target, the toolchain_type
property should also change to the one in python_rules:

toolchain(
    name = "default_python3_toolchain",
    toolchain = ":just_python3",
    toolchain_type = "@rules_python//python:toolchain_type",
)

@johnbartholomew
Copy link
Collaborator

Thanks for the fix!

@deblasis
Copy link

@mortenmj thanks for the fix, I guess you just need to update this:

master...deblasis:jsonnet:master#diff-8db7ed2f5e

image

@johnbartholomew
Copy link
Collaborator

I've added a commit to use toolchain_type = "@rules_python//python:toolchain_type",. Unfortunately there is another failure showing in the CI build-and-test-using-Bazel action. It looks like WORKSPACE refers to quite an old rules_jsonnet, and this probably also needs to be updated, though it may only be breaking our ability to run Jsonnet's own test suite with Bazel.

I'll look into it some more hopefully later today or early next week.

@johnbartholomew johnbartholomew merged commit 4ab62a1 into google:master Jan 18, 2025
5 of 6 checks passed
@simonff
Copy link

simonff commented Jan 18, 2025

Thank you for the fix. I'm still getting the error below in a rule using jsonnet. Repro:

git clone https://github.com/google/earthengine-catalog
cd earthengine-catalog
bazel test --define jsonnet_port=cpp //...


ERROR: Traceback (most recent call last):
File "...bazel_simonf/be0a056b459fb929c3961680f76bd588/external/jsonnet+/platform_defs/BUILD", line 10, column 16, in
py_runtime_pair(
File "..._bazel_simonf/be0a056b459fb929c3961680f76bd588/external/bazel_tools/tools/python/toolchain.bzl", line 17, column 9, in py_runtime_pair
fail("@bazel_tools//toolchain.bzl py_runtime_pair is deprecated. Use version from rules_python")
Error in fail: @bazel_tools//toolchain.bzl py_runtime_pair is deprecated. Use version from rules_python
ERROR: /tmp/earthengine-catalog/checker/tree/BUILD:44:8: While resolving toolchains for target //checker/tree:url_id_test (edcaa02): invalid registered toolchain '//platform_defs:default_python3_toolchain': no such target '@@jsonnet+//platform_defs:default_python3_toolchain': target 'default_python3_toolchain' not declared in package 'platform_defs' defined by ..._bazel_simonf/be0a056b459fb929c3961680f76bd588/external/jsonnet+/platform_defs/BUILD

@johnbartholomew
Copy link
Collaborator

I'm still getting the error below in a rule using jsonnet.

There hasn't been a new jsonnet release yet, nor a new rules_jsonnet bazel module release; I would expect both are needed. I intend to make a new jsonnet release soon, but usually the jsonnet and go-jsonnet releases are coordinated so there is some extra work for me to do there. And then rules_jsonnet probably needs a new release as well (I'm not a maintainer for rules_jsonnet).

If you want to get things working for now without waiting for a release, you can use git_override in the earthengine-catalog MODULE.bazel to use the latest unreleased jsonnet as source, overriding the version rules_jsonnet points to. Add this at the start of MODULE.bazel:

# https://bazel.build/rules/lib/globals/module#git_override
git_override(
	module_name = "jsonnet",
	commit = "4487bfb8ba7ed7a3d91d79d4a1aa2205e7839558",
	remote = "https://github.com/google/jsonnet.git",
)

@simonff
Copy link

simonff commented Jan 19, 2025

Thank you, that works. Do you have an ETA for rolling out the fix officially as you described?

@johnbartholomew
Copy link
Collaborator

Do you have an ETA for rolling out the fix officially as you described?

I can't make any guarantees or high confidence predictions as this is spare-time-only effort. But with a little luck I'll publish a new release by the end of January.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Import Python Bazel rules from rules_python
5 participants