Skip to content

Conversation

@angerson
Copy link
Contributor

This is one way of getting around the problem where TF is too big to build on Windows. If bazel is told to create the runfiles symlink tree explicitly, Windows can use the same method as Linux to build the pip package without needing to zip everything up, and simple_console_for_windows is no longer needed.

The caveats are:

  • The MSYS2 environment may need to be configured specifically to support symlinks; I am not sure. I set my test environment to use MSYS=winsymlinks:nativestrict but didn't test this solution without that.
  • The build environment needs rsync, which replaces cp in one case where cp was giving me symlink-related errors.

I extracted this out of a work-in-progress environment I've been exploring, so I'm not 100% sure it will work right... but it seems very promising.

@google-ml-butler google-ml-butler bot added the size:M CL Change Size: Medium label Aug 17, 2023
@gbaned gbaned requested a review from mihaimaruseac August 18, 2023 10:17
@google-ml-butler google-ml-butler bot added the awaiting review Pull request awaiting review label Aug 18, 2023
Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

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

This seems like it could work. Thank you!

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Aug 18, 2023
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 18, 2023
copybara-service bot pushed a commit to google/tsl that referenced this pull request Aug 18, 2023
Imported from GitHub PR tensorflow/tensorflow#61619

This is one way of getting around the problem where TF is too big to build on Windows. If bazel is told to create the runfiles symlink tree explicitly, Windows can use the same method as Linux to build the pip package without needing to zip everything up, and simple_console_for_windows is no longer needed.

The caveats are:

- The MSYS2 environment may need to be configured specifically to support symlinks; I am not sure. I set my test environment to use `MSYS=winsymlinks:nativestrict` but didn't test this solution without that.
- The build environment needs `rsync`, which replaces `cp` in one case where `cp` was giving me symlink-related errors.

I extracted this out of a work-in-progress environment I've been exploring, so I'm not 100% sure it will work right... but it seems very promising.
Copybara import of the project:

--
93596226a7df33dd085712d4bed9f66ee1b55e27 by Austin Anderson <angerson@google.com>:

Remove dependency on simple_console_for_windows

Merging this change closes #61619

FUTURE_COPYBARA_INTEGRATE_REVIEW=tensorflow/tensorflow#61619 from angerson:master 93596226a7df33dd085712d4bed9f66ee1b55e27
PiperOrigin-RevId: 558131327
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Aug 18, 2023
copybara-service bot pushed a commit to google/tsl that referenced this pull request Aug 18, 2023
Imported from GitHub PR tensorflow/tensorflow#61619

This is one way of getting around the problem where TF is too big to build on Windows. If bazel is told to create the runfiles symlink tree explicitly, Windows can use the same method as Linux to build the pip package without needing to zip everything up, and simple_console_for_windows is no longer needed.

The caveats are:

- The MSYS2 environment may need to be configured specifically to support symlinks; I am not sure. I set my test environment to use `MSYS=winsymlinks:nativestrict` but didn't test this solution without that.
- The build environment needs `rsync`, which replaces `cp` in one case where `cp` was giving me symlink-related errors.

I extracted this out of a work-in-progress environment I've been exploring, so I'm not 100% sure it will work right... but it seems very promising.
Copybara import of the project:

--
93596226a7df33dd085712d4bed9f66ee1b55e27 by Austin Anderson <angerson@google.com>:

Remove dependency on simple_console_for_windows

Merging this change closes #61619

PiperOrigin-RevId: 558183664
copybara-service bot pushed a commit that referenced this pull request Aug 18, 2023
PiperOrigin-RevId: 558183664
copybara-service bot pushed a commit to openxla/xla that referenced this pull request Aug 18, 2023
Imported from GitHub PR tensorflow/tensorflow#61619

This is one way of getting around the problem where TF is too big to build on Windows. If bazel is told to create the runfiles symlink tree explicitly, Windows can use the same method as Linux to build the pip package without needing to zip everything up, and simple_console_for_windows is no longer needed.

The caveats are:

- The MSYS2 environment may need to be configured specifically to support symlinks; I am not sure. I set my test environment to use `MSYS=winsymlinks:nativestrict` but didn't test this solution without that.
- The build environment needs `rsync`, which replaces `cp` in one case where `cp` was giving me symlink-related errors.

I extracted this out of a work-in-progress environment I've been exploring, so I'm not 100% sure it will work right... but it seems very promising.
Copybara import of the project:

--
93596226a7df33dd085712d4bed9f66ee1b55e27 by Austin Anderson <angerson@google.com>:

Remove dependency on simple_console_for_windows

Merging this change closes #61619

PiperOrigin-RevId: 558183664
@mraunak
Copy link
Contributor

mraunak commented Aug 19, 2023

Hi @angerson and @mihaimaruseac thank you for the PR. It is working well, wheels are generated successfully and sanity tests are passing.

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Aug 19, 2023
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 19, 2023
@mihaimaruseac
Copy link
Collaborator

Thank you for confirming!

@copybara-service copybara-service bot merged commit 6552f8a into tensorflow:master Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review Pull request awaiting review ready to pull PR ready for merge process size:M CL Change Size: Medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants