Skip to content
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

Fix Cuda configuration on Windows #10158

Merged
merged 3 commits into from
May 26, 2017

Conversation

meteorcloudy
Copy link
Member

@meteorcloudy meteorcloudy commented May 24, 2017

When creating a genrule for a set of files, we should use copy instead of
symlink on Windows.

This addresses an error reported at stackoverflow:
https://stackoverflow.com/questions/44038367/genrules-without-outputs-dont-make-sense-bazel-windows-10-build

Applied the same approach to python_config.bzl to simplify the code.

When creating a genrule for a set of files, we use copy instead of
symlink on Windows.

Applied the same approach to python_config.bzl to simplify the code.
@@ -67,4 +67,4 @@ passing_tests=$(bazel query "kind(py_test, //${PY_TEST_DIR}/tensorflow/python/.
# Define no_tensorflow_py_deps=true so that every py_test has no deps anymore,
# which will result testing system installed tensorflow
# GPU tests are very flaky when running concurently, so set local_test_jobs=5

Choose a reason for hiding this comment

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

nit: please update comment to reflect the change in script.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Done.

@maciekcc
Copy link

Jenkins test this please.


If src_dir is passed, files will be read from the given directory; otherwise
we assume files are in src_files and dest_files
"""
if src_dir != None:
src_dir = src_dir.replace("\\", "/")
dest_dir = dest_dir.replace("\\", "/")

Choose a reason for hiding this comment

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

One more nit: can you please extract a method to perform this path slash replacement and trimming, and then use it in both the files.

Copy link
Member Author

Choose a reason for hiding this comment

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

No problem. Done.

@maciekcc maciekcc added the stat:awaiting response Status - Awaiting response from author label May 26, 2017
@maciekcc maciekcc merged commit 91546f5 into tensorflow:master May 26, 2017
@meteorcloudy meteorcloudy deleted the fix_cuda_win_config branch March 5, 2018 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes stat:awaiting response Status - Awaiting response from author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants