bazelbuild
feat(toolchains): Add armv7 linux platform to available toolchain platforms
I'm not sure what the intended behavior is here. I've compiled a python toolchain for armv7 which works totally fine in my environment but rules_python gets mad at me for describing a platform that it doesn't currently know about. I would love to be able to inform the rules but I don't really know how to go about that. I get that this platform is not currently defined since it's not otherwise available like the rest of the toolchains but this restriction gates me from developing on new platforms. I figured I'd try to open this PR and see what maintainers say so I know if I need to live with a "forever" patch or if there's a worthy contribution to be made.
I think this is in the right direction. From what I remember, I think that PLATFORMS variable is used to generate the urls to download and the repo names?
In general I'd love it if we didn't have to keep an enumerated list of platforms around.
but rules_python gets mad at me for describing a platform that it doesn't currently know about.
Can you explain this part a bit more? I was going to merge this PR, but this comment makes it sound like, while CI is passing, you are seeing errors elsewhere, something that CI isn't covering?
I think when we added...I think it was the apple arm platform? Most of the issues of having a platform entry in there that didn't have a corresponding thing (url? repo?) were flushed out. Could always be more.
I've compiled a python toolchain for armv7
Nice! So that must mean you're manually defining py_runtime(), toolchain() etc to wire it into the rest of things? Does this mean you'd be interested in something like python.local_toolchain(path="/path/to/my/runtime") in MODULE.bazel?
@UebelAndre , could you please paste the error you get without this PR? Maybe we are only fixing the symptom if we add the linux arm constant to the platforms, that said, I am fine to have the real fix in a followup PR if the the change may become too invasive.
@UebelAndre , could you please paste the error you get without this PR? Maybe we are only fixing the symptom if we add the linux arm constant to the platforms, that said, I am fine to have the real fix in a followup PR if the the change may become too invasive.
Without this patch I run into
ERROR: /home/user/Code/WORKSPACE.bazel:107:18: //external:python_3_10_armv7-unknown-linux-gnu: invalid value in 'platform' attribute: has to be one of 'aarch64-apple-darwin', 'aarch64-unknown-linux-gnu', 'ppc64le-unknown-linux-gnu', 's390x-unknown-linux-gnu', 'x86_64-apple-darwin', 'x86_64-pc-windows-msvc' or 'x86_64-unknown-linux-gnu' instead of 'armv7-unknown-linux-gnu'
ERROR: Error computing the main repository mapping: at /home/user/Code/tools/python/pip_deps.bzl:3:6: Encountered error while reading extension file '3.10/defs.bzl': no such package '@@python//3.10': error loading package 'external': Could not load //external package
When I add the following to my WORKSPACE file
python_repository(
name = "python_3_10_armv7-unknown-linux-gnu",
platform = "armv7-unknown-linux-gnu",
python_version = "3.10",
release_filename = "cpython-3.10-armv7-unknown-linux-gnueabihf-noopt.tar.gz",
sha256 = "4d5d44c0b25c6a1605be503e032be384e1472ba704536b7257459399e305be51",
strip_prefix = "python",
urls = ["https://python.tar.gz"],
)
Can you explain this part a bit more? I was going to merge this PR, but this comment makes it sound like, while CI is passing, you are seeing errors elsewhere, something that CI isn't covering?
I was referring to the error here: https://github.com/bazelbuild/rules_python/pull/1770#issuecomment-1967079750
Nice! So that must mean you're manually defining py_runtime(), toolchain() etc to wire it into the rest of things? Does this mean you'd be interested in something like
python.local_toolchain(path="/path/to/my/runtime")in MODULE.bazel?
No, I structured my artifact to be similar to https://github.com/indygreg/python-build-standalone, so I'm not really manually defining anything, just trying to use the existing plumbing like what I described on https://github.com/bazelbuild/rules_python/pull/1770#issuecomment-1967079750
It looks like the platform is not used for anything but checking if windows is in the platform string, which suspects me that we could potentially remove the keyed attribute. I understand that it would be then basically unkeyed free-form string, but I don't think if having something like this is sustainable in the long run either. Maybe having an os, cpu, libc triplet could be a better choice?
I am inclined to just merge this as is as this improves the compatibility. Any objections @rickeylev?
I am inclined to just merge this as is as this improves the compatibility. Any objections @rickeylev?
Does this mean no objections? 😅
Could you please rebase? I think the changelog would be weirdly formatted if we merge as is.
Could you please rebase? I think the changelog would be weirdly formatted if we merge as is.
@aignas done!