Automated rollback of change 153736477

Change: 153825726

configure

@@ -86,6 +86,9 @@ while true; do
   PYTHON_BIN_PATH=""
   # Retry
 done
+export PYTHON_BIN_PATH
+write_action_env_to_bazelrc "PYTHON_BIN_PATH" "$PYTHON_BIN_PATH"
+# TODO(ngiraldo): allow the user to optionally set PYTHON_INCLUDE_PATH and NUMPY_INCLUDE_PATH
 
 ## Set up MKL related environment settings
 if false; then # Disable building with MKL for now
@@ -243,7 +246,7 @@ fi
 
 
 # Invoke python_config and set up symlinks to python includes
-./util/python/python_config.sh --setup "$PYTHON_BIN_PATH"
+./util/python/python_config.sh "$PYTHON_BIN_PATH"
 
 # Append CC optimization flags to bazel.rc
 echo >> tools/bazel.rc

tensorflow/workspace.bzl

@@ -5,6 +5,7 @@ load("//third_party/sycl:sycl_configure.bzl", "sycl_configure")
 load("@io_bazel_rules_closure//closure/private:java_import_external.bzl", "java_import_external")
 load("@io_bazel_rules_closure//closure:defs.bzl", "filegroup_external")
 load("@io_bazel_rules_closure//closure:defs.bzl", "webfiles_external")
+load("//third_party/py:python_configure.bzl", "python_configure")
 
 
 # Parse the bazel version string from `native.bazel_version`.
@@ -119,6 +120,7 @@ def tf_workspace(path_prefix="", tf_repo_name=""):
   check_version("0.4.5")
   cuda_configure(name="local_config_cuda")
   sycl_configure(name="local_config_sycl")
+  python_configure(name="local_config_python")
   if path_prefix:
     print("path_prefix was specified to tf_workspace but is no longer used " +
           "and will be removed in the future.")

third_party/py/BUILD.tpl

@@ -0,0 +1,53 @@
+licenses(["restricted"])
+
+package(default_visibility = ["//visibility:public"])
+
+cc_library(
+    name = "python_headers",
+    hdrs = select({
+        "windows" : [
+            "python_include_windows",
+        ],
+        "//conditions:default" : [
+            "python_include",
+        ],
+    }),
+    includes = select({
+        "windows" : [
+            "python_include_windows",
+        ],
+        "//conditions:default" : [
+            "python_include",
+        ],
+    }),
+)
+
+cc_library(
+    name = "numpy_headers",
+    hdrs = select({
+        "windows" : [
+            "numpy_include_windows",
+        ],
+        "//conditions:default" : [
+            "numpy_include",
+        ],
+    }),
+    includes = select({
+        "windows" : [
+            "numpy_include_windows",
+        ],
+        "//conditions:default" : [
+            "numpy_include",
+        ],
+    }),
+)
+
+config_setting(
+    name = "windows",
+    values = {"cpu": "x64_windows"},
+    visibility = ["//visibility:public"],
+)
+
+%{PYTHON_INCLUDE_GENRULE}
+
+%{NUMPY_INCLUDE_GENRULE}

third_party/py/numpy/BUILD

@@ -8,11 +8,9 @@ py_library(
     srcs_version = "PY2AND3",
 )
 
-cc_library(
+alias(
     name = "headers",
-    hdrs = glob(["numpy_include/**/*.h"]),
-    data = ["//util/python:python_checked"],
-    includes = ["numpy_include"],
+    actual = "@local_config_python//:numpy_headers",
 )
 
 genrule(

third_party/py/python_configure.bzl

@@ -0,0 +1,206 @@
+# -*- Python -*-
+"""Repository rule for Python autoconfiguration.
+
+`python_configure` depends on the following environment variables:
+
+  * `NUMPY_INCLUDE_PATH`: Location of Numpy libraries.
+  * `PYTHON_BIN_PATH`: location of python binary.
+  * `PYTHON_INCLUDE_PATH`: Location of python binaries.
+"""
+
+_NUMPY_INCLUDE_PATH = "NUMPY_INCLUDE_PATH"
+_PYTHON_BIN_PATH = "PYTHON_BIN_PATH"
+_PYTHON_INCLUDE_PATH = "PYTHON_INCLUDE_PATH"
+
+
+def _tpl(repository_ctx, tpl, substitutions={}, out=None):
+  if not out:
+    out = tpl
+  repository_ctx.template(
+      out,
+      Label("//third_party/py:%s.tpl" % tpl),
+      substitutions)
+
+
+def _python_configure_warning(msg):
+  """Output warning message during auto configuration."""
+  yellow = "\033[1;33m"
+  no_color = "\033[0m"
+  print("\n%sPython Configuration Warning:%s %s\n" % (yellow, no_color, msg))
+
+
+def _python_configure_fail(msg):
+  """Output failure message when auto configuration fails."""
+  red = "\033[0;31m"
+  no_color = "\033[0m"
+  fail("\n%sPython Configuration Error:%s %s\n" % (red, no_color, msg))
+
+
+def _get_env_var(repository_ctx, name, default = None, enable_warning = True):
+  """Find an environment variable in system path."""
+  if name in repository_ctx.os.environ:
+    return repository_ctx.os.environ[name]
+  if default != None:
+    if enable_warning:
+      _python_configure_warning(
+          "'%s' environment variable is not set, using '%s' as default" % (name, default))
+    return default
+  _python_configure_fail("'%s' environment variable is not set" % name)
+
+
+def _is_windows(repository_ctx):
+  """Returns true if the host operating system is windows."""
+  os_name = repository_ctx.os.name.lower()
+  if os_name.find("windows") != -1:
+    return True
+  return False
+
+
+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 _is_windows(repository_ctx):
+    find_result = repository_ctx.execute([
+        "dir", src_dir, "/b", "/s", "/a-d",
+    ])
+  else:
+    find_result = repository_ctx.execute([
+        "find", src_dir, "-follow", "-type", "f",
+    ])
+  # Create a list with the src_dir stripped to use for outputs.
+  dest_files = find_result.stdout.replace(src_dir, '').splitlines()
+  src_files = find_result.stdout.splitlines()
+  command = []
+  command_windows = []
+  outs = []
+  outs_windows = []
+  for i in range(len(dest_files)):
+    if dest_files[i] != "":
+      command.append('ln -s ' + src_files[i] + ' $(@D)/' +
+                     dest_dir + dest_files[i])
+      # ln -sf is actually implemented as copying in msys since creating
+      # symbolic links is privileged on Windows. But copying is too slow, so
+      # invoke mklink to create junctions on Windows.
+      command_windows.append('mklink /J ' + src_files[i] + ' $(@D)/' +
+                             dest_dir + dest_files[i])
+      outs.append('      "' + dest_dir + dest_files[i] + '",')
+      outs_windows.append('      "' + dest_dir + '_windows' +
+                          dest_files[i] + '",')
+  genrule = _genrule(src_dir, genrule_name, ' && '.join(command),
+                     '\n'.join(outs))
+  genrule_windows = _genrule(src_dir, genrule_name + '_windows',
+                             "cmd /c \"" + ' && '.join(command_windows) + "\"",
+                             '\n'.join(outs_windows))
+  return genrule + '\n' + genrule_windows
+
+
+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'
+  )
+
+
+def _check_python_bin(repository_ctx, python_bin):
+  """Checks the python bin path."""
+  cmd =  '[[ -x "%s" ]] && [[ ! -d "%s" ]]' % (python_bin, python_bin)
+  result = repository_ctx.execute(["bash", "-c", cmd])
+  if result.return_code == 1:
+    _python_configure_fail(
+        "PYTHON_BIN_PATH is not executable.  Is it the python binary?")
+
+
+def _get_python_include(repository_ctx, python_bin):
+  """Gets the python include path."""
+  result = repository_ctx.execute([python_bin, "-c",
+                                   'from __future__ import print_function;' +
+                                   'from distutils import sysconfig;' +
+                                   'print(sysconfig.get_python_inc())'])
+  if result == "":
+    _python_configure_fail(
+        "Problem getting python include path.  Is distutils installed?")
+  return result.stdout.splitlines()[0]
+
+
+def _get_numpy_include(repository_ctx, python_bin):
+  """Gets the numpy include path."""
+  result = repository_ctx.execute([python_bin, "-c",
+                                   'from __future__ import print_function;' +
+                                   'import numpy;' +
+                                   ' print(numpy.get_include());'])
+  if result == "":
+    _python_configure_fail(
+        "Problem getting numpy include path.  Is numpy installed?")
+  return result.stdout.splitlines()[0]
+
+
+def _create_python_repository(repository_ctx):
+  """Creates the repository containing files set up to build with Python."""
+  python_include = None
+  numpy_include = None
+  # If local checks were requested, the python and numpy include will be auto
+  # detected on the host config (using _PYTHON_BIN_PATH).
+  if repository_ctx.attr.local_checks:
+    python_bin = _get_env_var(repository_ctx, _PYTHON_BIN_PATH)
+    _check_python_bin(repository_ctx, python_bin)
+    python_include = _get_python_include(repository_ctx, python_bin)
+    numpy_include = _get_numpy_include(repository_ctx, python_bin) + '/numpy'
+  else:
+    # Otherwise, we assume user provides all paths (via ENV or attrs)
+    python_include = _get_env_var(repository_ctx, _PYTHON_INCLUDE_PATH,
+                                  repository_ctx.attr.python_include)
+    numpy_include = _get_env_var(repository_ctx, _NUMPY_INCLUDE_PATH,
+                                 repository_ctx.attr.numpy_include) + '/numpy'
+
+  python_include_rule = _symlink_genrule_for_dir(
+      repository_ctx, python_include, 'python_include', 'python_include')
+  numpy_include_rule = _symlink_genrule_for_dir(
+      repository_ctx, numpy_include, 'numpy_include/numpy', 'numpy_include')
+  _tpl(repository_ctx, "BUILD", {
+      "%{PYTHON_INCLUDE_GENRULE}": python_include_rule,
+      "%{NUMPY_INCLUDE_GENRULE}": numpy_include_rule,
+  })
+
+
+def _python_autoconf_impl(repository_ctx):
+  """Implementation of the python_autoconf repository rule."""
+  _create_python_repository(repository_ctx)
+
+
+python_configure = repository_rule(
+    implementation = _python_autoconf_impl,
+    attrs = {
+        "local_checks": attr.bool(mandatory = False, default = True),
+        "python_include": attr.string(mandatory = False),
+        "numpy_include": attr.string(mandatory = False),
+    },
+    environ = [
+        _PYTHON_BIN_PATH,
+        _PYTHON_INCLUDE_PATH,
+        _NUMPY_INCLUDE_PATH,
+    ],
+)
+"""Detects and configures the local Python.
+
+Add the following to your WORKSPACE FILE:
+
+```python
+python_configure(name = "local_config_python")
+```
+
+Args:
+  name: A unique name for this workspace rule.
+"""

util/python/BUILD

@@ -2,31 +2,7 @@ licenses(["restricted"])
 
 package(default_visibility = ["//visibility:public"])
 
-cc_library(
+alias(
     name = "python_headers",
-    hdrs = glob([
-        "python_include/**/*.h",
-    ]),
-    data = [":python_checked"],
-    includes = ["python_include"],
-)
-
-genrule(
-    name = "python_check",
-    srcs = [
-        "python_config.sh",
-        "configure_files",
-    ],
-    outs = [
-        "python_checked",
-    ],
-    cmd = "OUTPUTDIR=\"$(@D)/\"; $(location :python_config.sh) --check && touch $$OUTPUTDIR/python_checked",
-    local = 1,
-)
-
-filegroup(
-    name = "configure_files",
-    data = glob([
-        "*",
-    ]),
+    actual = "@local_config_python//:python_headers",
 )

util/python/python_config.sh

@@ -26,23 +26,9 @@ else
   script_path=${script_path:-.}
 fi
 
-EXPECTED_PATHS="$script_path/util/python/python_include"\
-" $script_path/util/python/python_lib"\
-" $script_path/third_party/py/numpy/numpy_include"
-
 function main {
-  argument="$1"
-  shift
-  case $argument in
-    --check)
-      check_python
-      exit 0
-      ;;
-    --setup)
-      setup_python "$1"
-      exit 0
-      ;;
-  esac
+  setup_python "$1"
+  exit 0
 }
 
 function python_path {
@@ -93,6 +79,7 @@ END
 function setup_python {
   PYTHON_BIN_PATH="$1";
 
+  # TODO(ngiraldo): move most of these checks to root configure
   if [ -z "$PYTHON_BIN_PATH" ]; then
     echo "PYTHON_BIN_PATH was not provided.  Did you run configure?"
     exit 1
@@ -108,12 +95,7 @@ function setup_python {
     exit 1
   fi
 
-  local python_include="$("${PYTHON_BIN_PATH}" -c 'from __future__ import print_function; from distutils import sysconfig; print(sysconfig.get_python_inc());')"
-  if [ "$python_include" == "" ]; then
-    echo -e "\n\nERROR: Problem getting python include path.  Is distutils installed?"
-    exit 1
-  fi
-
+  # TODO(ngiraldo): confirm if these checks are really necessary, remove if not
   if [ -z "$PYTHON_LIB_PATH" ]; then
     local python_lib_path
     # Split python_path into an array of paths, this allows path containing spaces
@@ -149,35 +131,12 @@ function setup_python {
     exit 1
   fi
 
-  local numpy_include=$("${PYTHON_BIN_PATH}" -c 'from __future__ import print_function; import numpy; print(numpy.get_include());')
-  if [ "$numpy_include" == "" ]; then
-    echo -e "\n\nERROR: Problem getting numpy include path.  Is numpy installed?"
-    exit 1
-  fi
-
-  for x in $EXPECTED_PATHS; do
-    if [ -e "$x" ]; then
-      rm -rf "$x"
-    fi
-  done
-
-# ln -sf is actually implemented as copying in msys since creating symbolic
-# links is privileged on Windows. But copying is too slow, so invoke mklink
-# to create junctions on Windows.
-  if is_windows; then
-    cmd /c "mklink /J util\\python\\python_include \"${python_include}\""
-    cmd /c "mklink /J util\\python\\python_lib \"${python_lib}\""
-    cmd /c "mklink /J third_party\\py\\numpy\\numpy_include \"${numpy_include}\""
-  else
-    ln -sf "${python_include}" util/python/python_include
-    ln -sf "${python_lib}" util/python/python_lib
-    ln -sf "${numpy_include}" third_party/py/numpy/numpy_include
-  fi
   # Convert python path to Windows style before writing into bazel.rc
   if is_windows; then
     PYTHON_BIN_PATH="$(cygpath -m "$PYTHON_BIN_PATH")"
   fi
 
+  # TODO(ngiraldo): move all below to root configure
   # Write tools/bazel.rc
   echo "# Autogenerated by configure: DO NOT EDIT" > tools/bazel.rc
   sed -e "s/\$PYTHON_MAJOR_VERSION/$python_major_version/g" \
@@ -197,29 +156,4 @@ function is_windows() {
   fi
 }
 
-function check_python {
-  for x in $EXPECTED_PATHS; do
-    if [ ! -e "$x" ]; then
-      echo -e "\n\nERROR: Cannot find '${x}'.  Did you run configure?\n\n" 1>&2
-      exit 1
-    fi
-    # Don't check symbolic link on Windows
-    if ! is_windows && [ ! -L "${x}" ]; then
-      echo -e "\n\nERROR: '${x}' is not a symbolic link.  Internal error.\n\n" 1>&2
-      exit 1
-    fi
-    if is_windows; then
-      # In msys, readlink <path> doesn't work, because no symbolic link on
-      # Windows. readlink -f <path> returns the real path of a junction.
-      true_path=$(readlink -f "${x}")
-    else
-      true_path=$(readlink "${x}")
-    fi
-    if [ ! -d "${true_path}" ]; then
-      echo -e "\n\nERROR: '${x}' does not refer to an existing directory: ${true_path}.  Do you need to rerun configure?\n\n" 1>&2
-      exit 1
-    fi
-  done
-}
-
 main "$@"