-
Notifications
You must be signed in to change notification settings - Fork 556
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
Added windows support to hermetic toolchains #628
Added windows support to hermetic toolchains #628
Conversation
92bf7dd
to
bb6eee6
Compare
👋 Thanks for getting on this. I've wrangled my Macbook's Windows 10 VM, and installed this PR using (Sidebar: if anyone has a good guide on how to make Windows VMs on OSX not an awful experience, let me know. I'm using Virtualbox and throwing RAM (32) and CPU (8) at this thing and it's still super sluggish. Graphics issues?) I'm hitting the following minor problem when I run the PR:
Looks like a straightforward hash mismatch. |
python/private/versions.bzl
Outdated
# buildifier: disable=unsorted-dict-items | ||
TOOL_VERSIONS = { | ||
"3.8.12": { | ||
"x86_64-apple-darwin": "sha384-es0kCVBb4q5xSC09lOw83TKXtR6qdt0NeU56JtK7Y5M5V784k9MM2q8leE3QWGH6", | ||
"x86_64-unknown-linux-gnu": "sha384-61BjosOUm+Uo2yDtkLcRqbSIAXIPiHKmvySGO2J/GSOHXkj5QBf4cwzrDuOWR0H2", | ||
"x86_64-apple-darwin": "sha256-wmUK2QuFpdJRh5yvGHG9zgKtLGC0e3rBt2aRJ4tOto8=", |
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.
Can we provide these integrity strings as 'standard' hex sha256 strings? I see that the docs say use "Subresource Integrity format" but it seems like a small usability issue that the source code integrity strings are in a different format to what Bazel spits out in logs.
Update: Just tried it, and you unfortunately cannot. Disregard my comment.
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.
I see no reason why these couldn't become sha256
values. It would just require updating the attribute in repositories.bzl
I leave this decision up to @f0rmiga #618 (comment)
Fixing the checksum locally has the basics working on my Windows VM 🎉
|
@thundergolfer in case it's not obvious, this PR is targeting #618 |
@thundergolfer perhaps you're running into astral-sh/python-build-standalone#116 (comment) |
Updated the checksums |
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.
Awesome! Thanks for working on the Windows portion.
is_host_windows = "win" in rctx.os.name | ||
is_target_windows = "windows" in rctx.attr.platform | ||
python_version = rctx.attr.python_version | ||
build = "static-install_only" if is_target_windows else "install_only" |
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.
I'm not a Windows user. Would a static binary for Windows have the same limitation as a fully static binary for Linux that a shared object (i.e. a native Python extension) cannot be loaded at runtime (not able to do dlopen)?
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.
I'm not sure myself, but this was the only artifact that worked. My thought was to hopefully see something merged that works in some cases and fix other issues as they're reported.
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.
Well, this could possibly be a blocker for merging my PR into main. I'll take this for now, but will have to investigate before releasing this feature.
include = [ | ||
"bin/**", | ||
"DLLs/**", | ||
"extensions/**", | ||
"include/**", | ||
"lib/**", | ||
"libs/**", | ||
"Scripts/**", | ||
"share/**", | ||
], |
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.
A select
with the platforms would be nicer here.
Co-authored-by: Thulio Ferraz Assis <[email protected]>
Co-authored-by: Thulio Ferraz Assis <[email protected]>
Co-authored-by: Thulio Ferraz Assis <[email protected]>
Co-authored-by: Thulio Ferraz Assis <[email protected]>
* Added windows support to hermetic toolchains * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> Co-authored-by: Thulio Ferraz Assis <[email protected]>
Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: compile zstd if missing Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: buildifier Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: make python_repositories reproducible Signed-off-by: Thulio Ferraz Assis <[email protected]> rename: python_repositories -> python_repository Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: linter Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: make interpreter files publicly visible Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: add files to py_runtime Signed-off-by: Thulio Ferraz Assis <[email protected]> Account for some platforms not having all versions Added windows support to hermetic toolchains (bazelbuild#628) * Added windows support to hermetic toolchains * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> Co-authored-by: Thulio Ferraz Assis <[email protected]> refactor: simplify logic for release urls Also, added a helper target to print the release hashes. Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: Provide a host platform alias (bazelbuild#635) * feat: Provide a host platform alias This lets users and repository rules access the interpreter for whatever host the repository is running on. * Apply suggestions from code review Co-authored-by: Thulio Ferraz Assis <[email protected]> fix: files excludes Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: macOS dislikes --recursive Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: buildifier issues Signed-off-by: Thulio Ferraz Assis <[email protected]> Allow previous indygreg releases (bazelbuild#636) This gives us more python patch versions fix: put back zstd support for older releases Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: hash calculator Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: use hermetic interpreter with pip_parse and pip_install Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: add missing attrs back for zstd Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: expose zstd attributes Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: normalize OS names Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: linting issues Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: support windows in the aliases Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: linting issues Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: windows python.exe instead of python3.exe Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: use consts for OS names Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: always use latest toolchain for test Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: expose versions.bzl Signed-off-by: Thulio Ferraz Assis <[email protected]> refactor: move toolchain tests out of private Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: acceptance tests for the toolchains Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: rewrite test in py to work on windows Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: README example Co-authored-by: UebelAndre <[email protected]> fix: use toolchain to run acceptance tests Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: use matrix for acceptance tests Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: support acceptance_tests on windows Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: alias for pip Signed-off-by: Thulio Ferraz Assis <[email protected]> fix?: include call to windows cmd Signed-off-by: Thulio Ferraz Assis <[email protected]> Fix windows acceptance tests (bazelbuild#641) * Fix windows acceptance tests * test * todo: remove Co-authored-by: Thulio Ferraz Assis <[email protected]> refactor: polishing Windows testing support Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: unset py2_runtime Signed-off-by: Thulio Ferraz Assis <[email protected]> rename: host -> resolved_interpreter Signed-off-by: Thulio Ferraz Assis <[email protected]> doc: add reference to quirks in python-build-standalone Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: allow a distutils.cfg to be passed Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: buildifier (again) Signed-off-by: Thulio Ferraz Assis <[email protected]> Minor code review suggestions
Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: compile zstd if missing Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: buildifier Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: make python_repositories reproducible Signed-off-by: Thulio Ferraz Assis <[email protected]> rename: python_repositories -> python_repository Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: linter Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: make interpreter files publicly visible Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: add files to py_runtime Signed-off-by: Thulio Ferraz Assis <[email protected]> Account for some platforms not having all versions Added windows support to hermetic toolchains (#628) * Added windows support to hermetic toolchains * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> Co-authored-by: Thulio Ferraz Assis <[email protected]> refactor: simplify logic for release urls Also, added a helper target to print the release hashes. Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: Provide a host platform alias (#635) * feat: Provide a host platform alias This lets users and repository rules access the interpreter for whatever host the repository is running on. * Apply suggestions from code review Co-authored-by: Thulio Ferraz Assis <[email protected]> fix: files excludes Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: macOS dislikes --recursive Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: buildifier issues Signed-off-by: Thulio Ferraz Assis <[email protected]> Allow previous indygreg releases (#636) This gives us more python patch versions fix: put back zstd support for older releases Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: hash calculator Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: use hermetic interpreter with pip_parse and pip_install Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: add missing attrs back for zstd Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: expose zstd attributes Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: normalize OS names Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: linting issues Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: support windows in the aliases Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: linting issues Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: windows python.exe instead of python3.exe Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: use consts for OS names Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: always use latest toolchain for test Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: expose versions.bzl Signed-off-by: Thulio Ferraz Assis <[email protected]> refactor: move toolchain tests out of private Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: acceptance tests for the toolchains Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: rewrite test in py to work on windows Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: README example Co-authored-by: UebelAndre <[email protected]> fix: use toolchain to run acceptance tests Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: use matrix for acceptance tests Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: support acceptance_tests on windows Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: alias for pip Signed-off-by: Thulio Ferraz Assis <[email protected]> fix?: include call to windows cmd Signed-off-by: Thulio Ferraz Assis <[email protected]> Fix windows acceptance tests (#641) * Fix windows acceptance tests * test * todo: remove Co-authored-by: Thulio Ferraz Assis <[email protected]> refactor: polishing Windows testing support Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: unset py2_runtime Signed-off-by: Thulio Ferraz Assis <[email protected]> rename: host -> resolved_interpreter Signed-off-by: Thulio Ferraz Assis <[email protected]> doc: add reference to quirks in python-build-standalone Signed-off-by: Thulio Ferraz Assis <[email protected]> feat: allow a distutils.cfg to be passed Signed-off-by: Thulio Ferraz Assis <[email protected]> fix: buildifier (again) Signed-off-by: Thulio Ferraz Assis <[email protected]> Minor code review suggestions
* feat: cpython toolchains for linux and macos Signed-off-by: Thulio Ferraz Assis <[email protected]> * feat: compile zstd if missing Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: buildifier Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: make python_repositories reproducible Signed-off-by: Thulio Ferraz Assis <[email protected]> * rename: python_repositories -> python_repository Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: linter Signed-off-by: Thulio Ferraz Assis <[email protected]> * feat: make interpreter files publicly visible Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: add files to py_runtime Signed-off-by: Thulio Ferraz Assis <[email protected]> * Account for some platforms not having all versions * Added windows support to hermetic toolchains (#628) * Added windows support to hermetic toolchains * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <[email protected]> Co-authored-by: Thulio Ferraz Assis <[email protected]> * refactor: simplify logic for release urls Also, added a helper target to print the release hashes. Signed-off-by: Thulio Ferraz Assis <[email protected]> * feat: Provide a host platform alias (#635) * feat: Provide a host platform alias This lets users and repository rules access the interpreter for whatever host the repository is running on. * Apply suggestions from code review Co-authored-by: Thulio Ferraz Assis <[email protected]> * fix: files excludes Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: macOS dislikes --recursive Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: buildifier issues Signed-off-by: Thulio Ferraz Assis <[email protected]> * Allow previous indygreg releases (#636) This gives us more python patch versions * fix: put back zstd support for older releases Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: hash calculator Signed-off-by: Thulio Ferraz Assis <[email protected]> * feat: use hermetic interpreter with pip_parse and pip_install Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: add missing attrs back for zstd Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: expose zstd attributes Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: normalize OS names Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: linting issues Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: support windows in the aliases Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: linting issues Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: windows python.exe instead of python3.exe Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: use consts for OS names Signed-off-by: Thulio Ferraz Assis <[email protected]> * feat: always use latest toolchain for test Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: expose versions.bzl Signed-off-by: Thulio Ferraz Assis <[email protected]> * refactor: move toolchain tests out of private Signed-off-by: Thulio Ferraz Assis <[email protected]> * feat: acceptance tests for the toolchains Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: rewrite test in py to work on windows Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: README example Co-authored-by: UebelAndre <[email protected]> * fix: use toolchain to run acceptance tests Signed-off-by: Thulio Ferraz Assis <[email protected]> * feat: use matrix for acceptance tests Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: support acceptance_tests on windows Signed-off-by: Thulio Ferraz Assis <[email protected]> * feat: alias for pip Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix?: include call to windows cmd Signed-off-by: Thulio Ferraz Assis <[email protected]> * Fix windows acceptance tests (#641) * Fix windows acceptance tests * test * todo: remove Co-authored-by: Thulio Ferraz Assis <[email protected]> * refactor: polishing Windows testing support Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: unset py2_runtime Signed-off-by: Thulio Ferraz Assis <[email protected]> * rename: host -> resolved_interpreter Signed-off-by: Thulio Ferraz Assis <[email protected]> * doc: add reference to quirks in python-build-standalone Signed-off-by: Thulio Ferraz Assis <[email protected]> * feat: allow a distutils.cfg to be passed Signed-off-by: Thulio Ferraz Assis <[email protected]> * fix: buildifier (again) Signed-off-by: Thulio Ferraz Assis <[email protected]> * Minor code review suggestions (#642) * Minor code review suggestions * Apply suggestions from code review Co-authored-by: Thulio Ferraz Assis <[email protected]> * fix: depset concat Signed-off-by: Thulio Ferraz Assis <[email protected]> Co-authored-by: Alex Eagle <[email protected]> Co-authored-by: UebelAndre <[email protected]>
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information