Merge pull request tensorflow#10158 from meteorcloudy/fix_cuda_win_co…

…nfig Fix Cuda configuration on Windows
mahiuchun · May 26, 2017 · 91546f5 · 91546f5
2 parents edb9535 + 18cdb0a
commit 91546f5
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 70 deletions.
diff --git a/tensorflow/tools/ci_build/windows/gpu/pip/build_tf_windows.sh b/tensorflow/tools/ci_build/windows/gpu/pip/build_tf_windows.sh
@@ -42,10 +42,10 @@ source "tensorflow/tools/ci_build/windows/bazel/common_env.sh" \
 source "tensorflow/tools/ci_build/windows/bazel/bazel_test_lib.sh" \
   || { echo "Failed to source bazel_test_lib.sh" >&2; exit 1; }
 
-clean_output_base
-
 run_configure_for_gpu_build
 
+clean_output_base
+
 bazel build -c opt --config=win-cuda $BUILD_OPTS tensorflow/tools/pip_package:build_pip_package || exit $?
 
 # Create a python test directory to avoid package name conflict
@@ -66,5 +66,5 @@ 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
-bazel test -c opt --config=win-cuda $BUILD_OPTS -k $passing_tests --define=no_tensorflow_py_deps=true --test_output=errors --local_test_jobs=5
+# GPU tests are very flaky when running concurently, so set local_test_jobs=2
+bazel test -c opt --config=win-cuda $BUILD_OPTS -k $passing_tests --define=no_tensorflow_py_deps=true --test_output=errors --local_test_jobs=2
diff --git a/third_party/gpus/cuda_configure.bzl b/third_party/gpus/cuda_configure.bzl
@@ -41,8 +41,8 @@ def find_cc(repository_ctx):
   """Find the C++ compiler."""
   # On Windows, we use Bazel's MSVC CROSSTOOL for GPU build
   # Return a dummy value for GCC detection here to avoid error
-  if _cpu_value(repository_ctx) == "Windows":
-    return "/use/--config x64_windows_msvc/instead"
+  if _is_windows(repository_ctx):
+    return "/use/--config=win-cuda --cpu=x64_windows_msvc/instead"
 
   if _use_cuda_clang(repository_ctx):
     target_cc_name = "clang"
@@ -122,10 +122,10 @@ def get_cxx_inc_directories(repository_ctx, cc):
 
 
 def auto_configure_fail(msg):
-  """Output failure message when auto configuration fails."""
+  """Output failure message when cuda configuration fails."""
   red = "\033[0;31m"
   no_color = "\033[0m"
-  fail("\n%sAuto-Configuration Error:%s %s\n" % (red, no_color, msg))
+  fail("\n%sCuda Configuration Error:%s %s\n" % (red, no_color, msg))
 # END cc_configure common functions (see TODO above).
 
 
@@ -421,6 +421,10 @@ def _cpu_value(repository_ctx):
   return result.stdout.strip()
 
 
+def _is_windows(repository_ctx):
+  """Returns true if the host operating system is windows."""
+  return _cpu_value(repository_ctx) == "Windows"
+
 def _lib_name(lib, cpu_value, version="", static=False):
   """Constructs the platform-specific name of a library.
 
@@ -769,14 +773,48 @@ def _create_dummy_repository(repository_ctx):
   repository_ctx.file("crosstool/BUILD", _DUMMY_CROSSTOOL_BUILD_FILE)
 
 
+def _execute(repository_ctx, cmdline, error_msg=None, error_details=None,
+             empty_stdout_fine=False):
+  """Executes an arbitrary shell command.
+
+  Args:
+    repository_ctx: the repository_ctx object
+    cmdline: list of strings, the command to execute
+    error_msg: string, a summary of the error if the command fails
+    error_details: string, details about the error or steps to fix it
+    empty_stdout_fine: bool, if True, an empty stdout result is fine, otherwise
+      it's an error
+  Return:
+    the result of repository_ctx.execute(cmdline)
+  """
+  result = repository_ctx.execute(cmdline)
+  if result.stderr or not (empty_stdout_fine or result.stdout):
+    auto_configure_fail(
+        "\n".join([
+            error_msg.strip() if error_msg else "Repository command failed",
+            result.stderr.strip(),
+            error_details if error_details else ""]))
+  return result
+
+
+def _norm_path(path):
+  """Returns a path with '/' and remove the trailing slash."""
+  path = path.replace("\\", "/")
+  if path[-1] == "/":
+    path = path[:-1]
+  return path
+
+
 def _symlink_genrule_for_dir(repository_ctx, src_dir, dest_dir, genrule_name,
     src_files = [], dest_files = []):
-  """Returns a genrule to symlink a set of files.
+  """Returns a genrule to symlink(or copy if on Windows) a set of files.
 
   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 = _norm_path(src_dir)
+    dest_dir = _norm_path(dest_dir)
     files = _read_dir(repository_ctx, src_dir)
     # Create a list with the src_dir stripped to use for outputs.
     dest_files = files.replace(src_dir, '').splitlines()
@@ -787,8 +825,10 @@ def _symlink_genrule_for_dir(repository_ctx, src_dir, dest_dir, genrule_name,
     if dest_files[i] != "":
       # If we have only one file to link we do not want to use the dest_dir, as
       # $(@D) will include the full path to the file.
-      dest = ' $(@D)/' + dest_dir + dest_files[i] if len(dest_files) != 1 else ' $(@D)/' + dest_files[i]
-      command.append('ln -s ' + src_files[i] + dest)
+      dest = '$(@D)/' + dest_dir + dest_files[i] if len(dest_files) != 1 else '$(@D)/' + dest_files[i]
+      # On Windows, symlink is not supported, so we just copy all the files.
+      cmd = 'cp -f' if _is_windows(repository_ctx) else 'ln -s'
+      command.append(cmd + ' "%s" "%s"' % (src_files[i] , dest))
       outs.append('      "' + dest_dir + dest_files[i] + '",')
   genrule = _genrule(src_dir, genrule_name, " && ".join(command),
                      "\n".join(outs))
@@ -821,10 +861,20 @@ def _read_dir(repository_ctx, src_dir):
   symlinks. The returned string contains the full path of all files
   separated by line breaks.
   """
-  find_result = repository_ctx.execute([
-      "find", src_dir, "-follow", "-type", "f"
-  ])
-  return find_result.stdout
+  if _is_windows(repository_ctx):
+    src_dir = src_dir.replace("/", "\\")
+    find_result = _execute(
+        repository_ctx, ["cmd.exe", "/c", "dir", src_dir, "/b", "/s", "/a-d"],
+        empty_stdout_fine=True)
+    # src_files will be used in genrule.outs where the paths must
+    # use forward slashes.
+    result = find_result.stdout.replace("\\", "/")
+  else:
+    find_result = _execute(
+        repository_ctx, ["find", src_dir, "-follow", "-type", "f"],
+        empty_stdout_fine=True)
+    result = find_result.stdout
+  return result
 
 
 def _use_cuda_clang(repository_ctx):

diff --git a/third_party/py/python_configure.bzl b/third_party/py/python_configure.bzl
@@ -82,74 +82,79 @@ def _execute(repository_ctx, cmdline, error_msg=None, error_details=None,
   return result
 
 
-def _symlink_genrule_for_dir(repository_ctx, src_dir, dest_dir, genrule_name):
-  """returns a genrule to symlink all files in a directory."""
-  # Get the list of files under this directory
-  find_result = None
-  if src_dir[-1] == "/":
-    src_dir = src_dir[:-1]
-  if dest_dir[-1] == "/":
-    dest_dir = dest_dir[:-1]
+def _read_dir(repository_ctx, src_dir):
+  """Returns a string with all files in a directory.
+
+  Finds all files inside a directory, traversing subfolders and following
+  symlinks. The returned string contains the full path of all files
+  separated by line breaks.
+  """
   if _is_windows(repository_ctx):
     src_dir = src_dir.replace("/", "\\")
     find_result = _execute(
         repository_ctx, ["cmd.exe", "/c", "dir", src_dir, "/b", "/s", "/a-d"],
         empty_stdout_fine=True)
     # src_files will be used in genrule.outs where the paths must
     # use forward slashes.
-    src_files = find_result.stdout.replace("\\", "/").splitlines()
+    result = find_result.stdout.replace("\\", "/")
   else:
     find_result = _execute(
         repository_ctx, ["find", src_dir, "-follow", "-type", "f"],
         empty_stdout_fine=True)
-    src_files = find_result.stdout.splitlines()
+    result = find_result.stdout
+  return result
 
-  dest_files = [e[len(src_dir) + 1:] for e in src_files]
-  outs = ['      "%s/%s",' % (dest_dir, e) for e in dest_files]
-  cmd_script = None
 
-  if _is_windows(repository_ctx):
-    dest_dir = dest_dir.replace("/", "\\")
-    # Copy the source directory to the output directory.
-    # Creating a junction pointing to the source directory and declaring the
-    # outputs like we do on Linux, then expecting that Bazel would pick up these
-    # outputs via the junction is ill-conceived, because Bazel deletes the
-    # output files from previous build prior to executing the action, meaning it
-    # would delete the actual files via the old junction.
-    cmd_script = "%s-cmd.cmd" % genrule_name
-    repository_ctx.file(
-        cmd_script,
-        executable = True,
-        content = "\n".join([
-            '@rem Auto-generated by //third_party/py/python_configure.bzl',
-            '@rem -------------------------------------------------------',
-            '@set DEST_DIR=%%1\\%s\\' % dest_dir,
-            '@rmdir /s /q %DEST_DIR%',
-            '@xcopy /s /e /q /y "%s" "%%DEST_DIR%%"' % src_dir]))
-    cmd = "cmd.exe /c \\\"$$(echo $(location %s) $(@D) | sed 's,/,\\\\\\\\,g')\\\"" % cmd_script
-  else:
-    cmd_script = "%s-cmd.sh" % genrule_name
-    repository_ctx.file(
-        cmd_script,
-        executable = True,
-        content = "\n".join(
-            ['#!/bin/bash',
-             '# Auto-generated by //third_party/py/python_configure.bzl',
-             'DEST_DIR="$1/%s"' % dest_dir] +
-            ['ln -s "%s" "${DEST_DIR}/%s"' % (src_files[i], dest_files[i])
-             for i in range(len(src_files))]))
-    cmd = "$(location %s) $(@D)" % cmd_script
-
-  return "\n".join([
-      'genrule(',
-      '    name = "%s",' % genrule_name,
-      '    outs = [',
-      ] + outs + [
-      '    ],',
-      '    cmd = "%s",' % cmd,
-      '    visibility = ["//visibility:private"],',
-      '    tools = ["%s"],' % cmd_script,
-      ')'])
+def _genrule(src_dir, genrule_name, command, outs):
+  """Returns a string with a genrule.
+
+  Genrule executes the given command and produces the given outputs.
+  """
+  return (
+      'genrule(\n' +
+      '    name = "' +
+      genrule_name + '",\n' +
+      '    outs = [\n' +
+      outs +
+      '    ],\n' +
+      '    cmd = """\n' +
+      command +
+      '    """,\n' +
+      ')\n\n'
+  )
+
+
+def _norm_path(path):
+  """Returns a path with '/' and remove the trailing slash."""
+  path = path.replace("\\", "/")
+  if path[-1] == "/":
+    path = path[:-1]
+  return path
+
+
+def _symlink_genrule_for_dir(repository_ctx, src_dir, dest_dir, genrule_name):
+  """Returns a genrule to symlink(or copy if on Windows) a set of files.
+  """
+  src_dir = _norm_path(src_dir)
+  dest_dir = _norm_path(dest_dir)
+  files = _read_dir(repository_ctx, src_dir)
+  # Create a list with the src_dir stripped to use for outputs.
+  dest_files = files.replace(src_dir, '').splitlines()
+  src_files = files.splitlines()
+  command = []
+  outs = []
+  for i in range(len(dest_files)):
+    if dest_files[i] != "":
+      # If we have only one file to link we do not want to use the dest_dir, as
+      # $(@D) will include the full path to the file.
+      dest = '$(@D)/' + dest_dir + dest_files[i] if len(dest_files) != 1 else '$(@D)/' + dest_files[i]
+      # On Windows, symlink is not supported, so we just copy all the files.
+      cmd = 'cp -f' if _is_windows(repository_ctx) else 'ln -s'
+      command.append(cmd + ' "%s" "%s"' % (src_files[i] , dest))
+      outs.append('      "' + dest_dir + dest_files[i] + '",')
+  genrule = _genrule(src_dir, genrule_name, " && ".join(command),
+                     "\n".join(outs))
+  return genrule
 
 
 def _get_python_lib(repository_ctx, python_bin):