Add Windows build #893

mtsokol · 2025-06-03T14:08:23Z

Here's a PR with Windows build support, without array_record dependency.

Outdated:

I think it's worth considering to switch to setup-bazel and setup-python Actions for Bazel and Python setup for all environments, rather than downloading and installing them in the script.
I couldn't make it work with the current pyenv and bazel installation on Windows - switching to GitHub Actions worked out of the box.
The Bazel setup complained about Module.bazel dependencies versions - I upgraded them.
It seems that as of now the wheel build is blocked by array-record - package: https://pypi.org/project/array-record/#history. The current setup requires 0.7.2 but Windows wheels for it are missing and the last one goes back to 0.4.1: https://github.com/mtsokol/grain/actions/runs/15418522140/job/43387085894.
I think that first we need to make sure that all dependencies have Windows distributions.

📚 Documentation preview 📚: https://google-grain--893.org.readthedocs.build/

iindyk · 2025-06-09T17:35:01Z

thanks Mateusz!

I think it's worth considering to switch to setup-bazel and setup-python Actions for Bazel and Python setup for all environments, rather than downloading and installing them in the script.

Agree, would you be able to try this out? maybe in a separate PR to make things easier

The Bazel setup complained about Module.bazel dependencies versions - I upgraded them.

this part is tricky, especially the protobuf dependency version. Downstream users are very sensitive to it. In particular, TensorFlow has pretty old protobuf dependency and using the newer ones may result into a crash in the env with grain + tensorflow. We wouldn't want that ideally. Would it be possible to not upgrade the dependencies yet? were the complaints errors?

It seems that as of now the wheel build is blocked by array-record - package

yeah, there's an ongoing work to fix array-record build (it's broken at main), once that is in, or folks will add Windows build. For now, we shouldn't block on that (i.e. let the install fail, at least verify that we can build wheels).

mtsokol · 2025-06-09T18:23:27Z

Agree, would you be able to try this out? maybe in a separate PR to make things easier

Sure! I'll take care of it.

Would it be possible to not upgrade the dependencies yet? were the complaints errors?

I think I upgraded only those that otherwise gave a dependency resolve error. But I can rerun it and paste details here - maybe there's a workaround for it.

mtsokol · 2025-06-10T12:26:29Z

@iindyk some updates:

Opened refactor PR: Move Bazel and Python installation to GitHub Action #900 <- let's merge it first, then I'll rebase this one.
Indeed dependencies doesn't need to be updated. Once I figured out how to pin Bazel version on installation USE_BAZEL_VERSION: "7.1.1" the current setup is fine, so I reverted these changes.
About the array-record: Currently bazel build fails with missing array-record for Windows. BUILD file requires it, so letting it fail means, bazel build fails, so we can't build the wheel anyway (i.e. we rsync bazel-bin/grain outputs to the wheel build directory). Am I missing anything?

iindyk · 2025-06-10T23:04:52Z

Am I missing anything?

no, you're right. Would you be able to fork our build and publish workflow (no need for nightly) with added windows support and send out a PR to array-record? I will be able to submit it from inside and add trusted publishing. We can submit this in Grain either way, and test after AR release with Windows.

here's the workflow I used for the last release: https://github.com/iindyk/array_record/blob/main/.github/workflows/build_and_publish.yml

.github/workflows/build_and_publish_template.yml

@@ -46,6 +46,9 @@ jobs:
        uses: actions/setup-python@v5
        with:
          python-version: ${{ matrix.python-version }}
+      - name: Install rsync for Windows


mtsokol · 2025-07-03T10:22:45Z

MODULE.bazel

@@ -21,7 +21,7 @@ bazel_dep(name = "bazel_skylib", version = "1.6.1")
 bazel_dep(name = "platforms", version = "0.0.9")
 bazel_dep(name = "rules_python", version = "0.37.0")
 bazel_dep(name = "rules_cc", version = "0.0.9")
-bazel_dep(name = "pybind11_bazel", version = "2.11.1")
+bazel_dep(name = "pybind11_bazel", version = "2.11.1.bzl.3")


Building extension modules on Windows was fixed in that release: https://github.com/pybind/pybind11_bazel/releases/tag/v2.11.1.bzl.3

mtsokol · 2025-07-03T10:24:17Z

grain/_src/python/data_sources.py

 import threading
 import time
 import typing
 from typing import Any, Generic, Optional, Protocol, SupportsIndex, TypeVar, Union

 from absl import logging
-import array_record.python.array_record_data_source as array_record
 from etils import epath


I couldn't use from etils import epy; epy.lazy_imports() as Union didn't allow epy's returned type.

mtsokol · 2025-07-03T10:24:57Z

grain/oss/build_whl.sh

+    *)
+      INCLUDE_EXT="*.so"
+      # Also reduce noise during build.
+      write_to_bazelrc "build --cxxopt=-Wno-deprecated-declarations --host_cxxopt=-Wno-deprecated-declarations"


I searched it and AFAIK There are no counterparts in MSVC.

mtsokol · 2025-07-03T10:28:22Z

Hi @iindyk,

The Windows build for Grain is ready for review. I made minimal changes that were required to make it work, incl. array_record as an optional dependency.

Here's a successful build in my fork: https://github.com/mtsokol/grain/actions/runs/16052901995

mtsokol · 2025-07-03T10:36:38Z

grain/oss/build_whl.sh

-  rsync -avm -L --exclude="__pycache__/*" grain "${TMPDIR}"
-  rsync -avm -L  --include="*.so" --include="*_pb2.py" \
+
+  rsync -avm -L --exclude="__pycache__/*" grain tmp_folder


This one was pretty tricky. Installed rsync (cygwin version) had a problem with absolute paths in Windows, namely for /tmp folder, as on Windows it's c:\users\runner~1\appdata\local\temp\, causing:

ssh: Could not resolve hostname c: Name or service not known

thinking that Windows C:\... path means some server C. So I'm using an intermediate folder, and then copy to the desired one (doesn't matter on Linux/MacOS).

Here's the same issue on SO:
https://stackoverflow.com/questions/51536762/rsync-does-not-recognize-windows-path

could you pls add a comment about the intermediate folder here?

mtsokol · 2025-07-03T14:18:46Z

.github/workflows/build_and_publish_template.yml

+        if: runner.os == 'Windows'
+        run: |
+          echo "TEMP_DIR=${PWD}" >> $GITHUB_ENV
+          echo "WHL_DIR=." >> $GITHUB_ENV


Had an issue with missing wheel files, looks like absolute paths on Windows aren't supported in actions/upload-artifact: actions/upload-artifact#519 (comment)

mtsokol · 2025-07-07T09:16:41Z

grain/BUILD

+    data = select({
+        "@platforms//os:windows": ["//grain/_src/python/experimental/index_shuffle/python:index_shuffle_module.pyd"],
+        "//conditions:default": ["//grain/_src/python/experimental/index_shuffle/python:index_shuffle_module.so"],
+    }),


Does Bazel allow to do it in a more concise manner? (Move path to a variable an glue the extension separately?)

not sure, I think this is ok

iindyk

thanks Mateusz!

iindyk · 2025-07-16T23:39:37Z

grain/BUILD

+    data = select({
+        "@platforms//os:windows": ["//grain/_src/python/experimental/index_shuffle/python:index_shuffle_module.pyd"],
+        "//conditions:default": ["//grain/_src/python/experimental/index_shuffle/python:index_shuffle_module.so"],
+    }),


not sure, I think this is ok

iindyk · 2025-07-16T23:41:39Z

grain/_src/python/dataset/transformations/shuffle.py

-from grain._src.python.experimental.index_shuffle.python import index_shuffle_module as index_shuffle
+
+with epy.lazy_imports():
+  from grain._src.python.experimental.index_shuffle.python import index_shuffle_module as index_shuffle


does this import not work on Windows?

Right - this doesn't import array_record and works on Windows, reverted.

iindyk · 2025-07-16T23:42:51Z

grain/oss/build_whl.sh

-  rsync -avm -L --exclude="__pycache__/*" grain "${TMPDIR}"
-  rsync -avm -L  --include="*.so" --include="*_pb2.py" \
+
+  rsync -avm -L --exclude="__pycache__/*" grain tmp_folder


could you pls add a comment about the intermediate folder here?

mtsokol · 2025-07-18T09:47:23Z

@iindyk I addressed your comments. In the follow-up I will add skips for 10 tests that fail on windows (e.g. due to unix-like paths in assert strings or compile_pip_requirements tests that check for array_record) and move JAX tests to pytest, so that CI with "Run unit tests" option will pass.

PORTED FROM #893 PiperOrigin-RevId: 784697196

Ported from #893 PiperOrigin-RevId: 784697196

Ported from #893 PiperOrigin-RevId: 785285748

mtsokol · 2025-07-22T19:56:13Z

Should we close this PR?

mtsokol force-pushed the windows-build branch 2 times, most recently from 0bf68ef to e205ea8 Compare June 10, 2025 18:34

mtsokol force-pushed the windows-build branch from e205ea8 to ab3986c Compare July 3, 2025 10:17

Add Windows build

5577301

mtsokol force-pushed the windows-build branch from ab3986c to 5577301 Compare July 3, 2025 10:21

mtsokol marked this pull request as ready for review July 3, 2025 10:21

mtsokol changed the title ~~Windows build~~ Add Windows build Jul 3, 2025

mtsokol commented Jul 3, 2025

View reviewed changes

.github/workflows/build_and_publish_template.yml

@@ -46,6 +46,9 @@ jobs:

uses: actions/setup-python@v5

with:

python-version: ${{ matrix.python-version }}

- name: Install rsync for Windows

This comment was marked as resolved.

Sign in to view

mtsokol commented Jul 3, 2025

View reviewed changes

mtsokol force-pushed the windows-build branch 6 times, most recently from 8193c75 to 6255902 Compare July 3, 2025 14:12

Fix artifact upload for Windows

6f45fcb

mtsokol force-pushed the windows-build branch from 6255902 to 6f45fcb Compare July 3, 2025 14:16

mtsokol commented Jul 3, 2025

View reviewed changes

Test check

0e7be33

mtsokol commented Jul 7, 2025

View reviewed changes

iindyk reviewed Jul 16, 2025

View reviewed changes

Apply review comments

caded3c

mtsokol requested a review from iindyk July 18, 2025 09:44

mtsokol mentioned this pull request Jul 18, 2025

[ENH] test suite cleanup #950

Open

copybara-service bot pushed a commit that referenced this pull request Jul 18, 2025

Add Windows build

309bbf4

PORTED FROM #893 PiperOrigin-RevId: 784697196

copybara-service bot mentioned this pull request Jul 18, 2025

Add Windows build #951

Closed

copybara-service bot pushed a commit that referenced this pull request Jul 18, 2025

Add Windows build

89d7c14

PORTED FROM #893 PiperOrigin-RevId: 784697196

copybara-service bot pushed a commit that referenced this pull request Jul 18, 2025

Add Windows build

763880a

Ported from #893 PiperOrigin-RevId: 784697196

copybara-service bot pushed a commit that referenced this pull request Jul 18, 2025

Add Windows build

635c914

Ported from #893 PiperOrigin-RevId: 784697196

copybara-service bot pushed a commit that referenced this pull request Jul 18, 2025

Add Windows build

5a11a1d

Ported from #893 PiperOrigin-RevId: 784697196

copybara-service bot pushed a commit that referenced this pull request Jul 18, 2025

Add Windows build

07dfccc

Ported from #893 PiperOrigin-RevId: 784697196

copybara-service bot pushed a commit that referenced this pull request Jul 21, 2025

Add Windows build

9a1adfd

Ported from #893 PiperOrigin-RevId: 784697196

copybara-service bot pushed a commit that referenced this pull request Jul 21, 2025

Add Windows build

a2630fe

Ported from #893 PiperOrigin-RevId: 784697196

copybara-service bot pushed a commit that referenced this pull request Jul 21, 2025

Add Windows build

015913b

Ported from #893 PiperOrigin-RevId: 784697196

copybara-service bot pushed a commit that referenced this pull request Jul 21, 2025

Add Windows build

3d655b4

Ported from #893 PiperOrigin-RevId: 784697196

copybara-service bot pushed a commit that referenced this pull request Jul 21, 2025

Add Windows build

ff4b87e

Ported from #893 PiperOrigin-RevId: 784697196

copybara-service bot pushed a commit that referenced this pull request Jul 21, 2025

Add Windows build

9772d55

Ported from #893 PiperOrigin-RevId: 785285748

Add Windows build #893

Are you sure you want to change the base?

Add Windows build #893

Conversation

mtsokol commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iindyk commented Jun 9, 2025

Uh oh!

mtsokol commented Jun 9, 2025

Uh oh!

mtsokol commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iindyk commented Jun 10, 2025

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtsokol commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtsokol Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iindyk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtsokol commented Jul 18, 2025

Uh oh!

mtsokol commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mtsokol commented Jun 3, 2025 •

edited

Loading

mtsokol commented Jun 10, 2025 •

edited

Loading

mtsokol commented Jul 3, 2025 •

edited

Loading

mtsokol Jul 3, 2025 •

edited

Loading

mtsokol commented Jul 22, 2025 •

edited

Loading