fix: Fixing select when we don't have specific platforms #1364
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
d4721af to
8785634
Compare
|
@rickeylev any idea why we are doing: alias(name = "python_headers", actual = select({":" + item: "@python_3_10_" + item + "//:python_headers" for item in TOOL_VERSIONS['3.10.9']['sha256'].keys()}))Instead of: alias(name = "python_headers", actual = "@python_3_10_x86_64-unknown-linux-gnu//:python_headers")I'm uncertain how to test that this change does not break anything. Here is most of the BUILD.bazel file that is created by this rule: alias(name = "files", actual = select({":" + item: "@python_3_10_" + item + "//:files" for item in TOOL_VERSIONS['3.10.9']['sha256'].keys()}))
alias(name = "includes", actual = select({":" + item: "@python_3_10_" + item + "//:includes" for item in TOOL_VERSIONS['3.10.9']['sha256'].keys()}))
alias(name = "libpython", actual = select({":" + item: "@python_3_10_" + item + "//:libpython" for item in TOOL_VERSIONS['3.10.9']['sha256'].keys()}))
alias(name = "py3_runtime", actual = select({":" + item: "@python_3_10_" + item + "//:py3_runtime" for item in TOOL_VERSIONS['3.10.9']['sha256'].keys()}))
#alias(name = "python_headers", actual = select({":" + item: "@python_3_10_" + item + "//:python_headers" for item in TOOL_VERSIONS['3.10.9']['sha256'].keys()}))
alias(name = "python_headers", actual = "@python_3_10_x86_64-unknown-linux-gnu//:python_headers")
alias(name = "python_runtimes", actual = select({":" + item: "@python_3_10_" + item + "//:python_runtimes" for item in TOOL_VERSIONS['3.10.9']['sha256'].keys()}))
alias(name = "python3", actual = select({":" + item: "@python_3_10_" + item + "//:" + ("python.exe" if "windows" in item else "bin/python3") for item in TOOL_VERSIONS['3.10.9']Running: Works but only gives you the headers for specific host platform with the change. With the select statement, we get the headers for every platform. |
|
Based on a conversation with @rickeylev we need to have a select with all of the PLATFORMS that are supported in it. This PR wins ;) |
aignas
reviewed
Aug 7, 2023
Comment on lines
+156
to
+159
| full_python_version = rctx.attr.python_version | ||
| if TOOL_VERSIONS.get(full_python_version) == None: | ||
| full_python_version = MINOR_MAPPING[full_python_version] |
There was a problem hiding this comment.
And a nit: this could be changed to be full_python_version = MINOR_MAPPING.get(rctx.attr.python_version, rctx.attr.python_version) as we should have an entry in MINOR_MAPPING for each entry in TOOL_VERSIONS.
There was a problem hiding this comment.
I will wait for #1340 to get in and use that.
In certain releases of Python we do not have various platforms like ppc. When using the call PLATFORMS.keys() we get all of the platforms instead of the platforms that are included with the Python version. We get the error "no such package" because, for instance, python_3_10_ppc64le-unknown-linux-gnu does not exist in Python 3.10.9 (see versions.bzl). You can recreate this error by running a bazel query command inside the examples/bzlmod directory. If you run: bazel query "deps(@python_3_10//:python_headers)" With a version of the rules previous to this commit, you will get an error. With this commit, you get a printout of all of the python_headers for all of the platforms that are supported in the Python 3.10 version used by the repository. Instead of using PLATFORMS.keys(), we use the sha256 keys in TOOL_VERSIONS dict based on the full Python version. This change now returns only the platforms supported in the Python version. Fixes: bazel-contrib#1326
8785634 to
8fdd983
Compare
|
I will wait for #1340 to get in and use the code to find the full version. |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 27, 2023
… X.Y.Z (#1423) Before this PR in numerous places we would check the MINOR_MAPPING dict. This PR adds a function that also fails if the X.Y format is not in the MINOR_MAPPING dict making the code more robust. Split from #1340 to unblock #1364. --------- Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
|
Implemented by #1473 and I have verified that the bug no longer exists. Verified that succeeds in the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In certain releases of Python we do not have various platforms like ppc. When using the call PLATFORMS.keys() we get all of the platforms instead of the platforms that are included with the Python version.
We get the error "no such package" because, for instance, python_3_10_ppc64le-unknown-linux-gnu does not exist in Python 3.10.9 (see versions.bzl).
You can recreate this error by running a bazel query command inside the examples/bzlmod directory. If you run:
bazel query "deps(@python_3_10//:python_headers)"
With a version of the rules previous to this commit, you will get an error. With this commit, you get a printout of all of the python_headers for all of the platforms that are supported in the Python 3.10 version used by the repository.
Instead of using PLATFORMS.keys(), we use the sha256 keys in TOOL_VERSIONS dict based on the full Python version. This change now returns only the platforms supported in the Python version.
Fixes: #1326