Simplify Eigen package config (#3288)

* Simplify Eigen package config

* Add missing unsupported/Eigen/*

* Fix pip setup.py

* Adjust new eigen header

* Fix bazel include dependency error

* Adjust Makefile to work with Eigen changes

* Remove nvcc workaround for CUDA <= 6.0

CUDA versions prior to 6.5 gave an

    error: kernel launches from templates are not allowed in system files

error when using gcc v4.8 and including code that uses templated
kernel launches via `-isystem`.
In order to work around this, the GPU crosstool converted `-isystem`
arguments containing the cuda headers into `-iquote` arguments.
This workaround has now been removed.

* Configure cmake and make to get eigen version from tensorflow/workspace.bzl

eigen.BUILD

@@ -1,9 +1,8 @@
 package(default_visibility = ["//visibility:public"])
 
-archive_dir = "eigen-eigen-b4fa9622b809"
 cc_library(
     name = "eigen",
-    hdrs = glob([archive_dir+"/**/*.h", archive_dir+"/unsupported/Eigen/*", archive_dir+"/unsupported/Eigen/CXX11/*", archive_dir+"/Eigen/*"]),
+    hdrs = glob(["**/*.h", "unsupported/Eigen/*", "unsupported/Eigen/CXX11/*", "Eigen/*"]),
-    includes = [ archive_dir ],
+    includes = [ '.' ],
     visibility = ["//visibility:public"],
 )

tensorflow/contrib/cmake/external/eigen.cmake

@@ -7,16 +7,30 @@
 
 include (ExternalProject)
 
-set(eigen_archive_hash "b4fa9622b809")
+# We parse the current Eigen version and archive hash from the bazel configuration
+file(STRINGS ${PROJECT_SOURCE_DIR}/../../workspace.bzl workspace_contents)
+foreach(line ${workspace_contents})
+    string(REGEX MATCH ".*eigen_version.*=.*\"(.*)\"" has_version ${line})
+    if(has_version)
+        set(eigen_version ${CMAKE_MATCH_1})
+        break()
+    endif()
+endforeach()
+foreach(line ${workspace_contents})
+    string(REGEX MATCH ".*eigen_sha256.*=.*\"(.*)\"" has_hash ${line})
+    if(has_hash)
+        set(eigen_hash ${CMAKE_MATCH_1})
+        break()
+    endif()
+endforeach()
 
 set(eigen_INCLUDE_DIRS
     ${CMAKE_CURRENT_BINARY_DIR}
     ${CMAKE_CURRENT_BINARY_DIR}/external/eigen_archive
-    ${CMAKE_CURRENT_BINARY_DIR}/external/eigen_archive/eigen-eigen-${eigen_archive_hash}
     ${tensorflow_source_dir}/third_party/eigen3
 )
-set(eigen_URL https://bitbucket.org/eigen/eigen/get/${eigen_archive_hash}.tar.gz)
+set(eigen_URL https://bitbucket.org/eigen/eigen/get/${eigen_version}.tar.gz)
-set(eigen_HASH SHA256=2862840c2de9c0473a4ef20f8678949ae89ab25965352ee53329e63ba46cec62)
+set(eigen_HASH SHA256=${eigen_hash})
 set(eigen_BUILD ${CMAKE_CURRENT_BINARY_DIR}/eigen/src/eigen)
 set(eigen_INSTALL ${CMAKE_CURRENT_BINARY_DIR}/eigen/install)
 
@@ -30,5 +44,5 @@ ExternalProject_Add(eigen
         -DCMAKE_BUILD_TYPE:STRING=Release
         -DCMAKE_VERBOSE_MAKEFILE:BOOL=OFF
         -DCMAKE_INSTALL_PREFIX:STRING=${eigen_INSTALL}
-        -DINCLUDE_INSTALL_DIR:STRING=${CMAKE_CURRENT_BINARY_DIR}/external/eigen_archive/eigen-eigen-${eigen_archive_hash}
+        -DINCLUDE_INSTALL_DIR:STRING=${CMAKE_CURRENT_BINARY_DIR}/external/eigen_archive
 )

tensorflow/contrib/makefile/Makefile

@@ -38,8 +38,8 @@ HOST_OBJDIR := $(MAKEFILE_DIR)/gen/host_obj/
 HOST_BINDIR := $(MAKEFILE_DIR)/gen/host_bin/
 HOST_GENDIR := $(MAKEFILE_DIR)/gen/host_obj/
 
-# Find the current Eigen version name from the Bazel build file
+# Find the current Eigen version from the Bazel configuration
-EIGEN_HASH := $(shell cat eigen.BUILD | grep archive_dir | head -1 | cut -f3 -d- | cut -f1 -d\")
+EIGEN_VERSION := $(shell grep eigen_version tensorflow/workspace.bzl | head -1 | sed -e 's/.*eigen_version\s*=\s*"\(.*\)"/\1/')
 
 # Settings for the host compiler.
 HOST_CXX := $(CC_PREFIX) gcc
@@ -53,7 +53,7 @@ HOST_LDOPTS += -L/usr/local/lib
 HOST_INCLUDES := \
 -I. \
 -I$(MAKEFILE_DIR)/downloads/ \
--I$(MAKEFILE_DIR)/downloads/eigen-eigen-$(EIGEN_HASH) \
+-I$(MAKEFILE_DIR)/downloads/eigen-eigen-$(EIGEN_VERSION) \
 -I$(HOST_GENDIR)
 ifeq ($(HAS_GEN_HOST_PROTOC),true)
 	HOST_INCLUDES += -I$(MAKEFILE_DIR)/gen/protobuf-host/include
@@ -123,7 +123,7 @@ LDFLAGS := \
 INCLUDES := \
 -I. \
 -I$(MAKEFILE_DIR)/downloads/ \
--I$(MAKEFILE_DIR)/downloads/eigen-eigen-$(EIGEN_HASH) \
+-I$(MAKEFILE_DIR)/downloads/eigen-eigen-$(EIGEN_VERSION) \
 -I$(PROTOGENDIR) \
 -I$(PBTGENDIR)
 ifeq ($(HAS_GEN_HOST_PROTOC),true)
@@ -208,7 +208,7 @@ ifeq ($(TARGET),ANDROID)
 -I$(NDK_ROOT)/sources/cxx-stl/gnu-libstdc++/4.9/libs/armeabi/include \
 -I. \
 -I$(MAKEFILE_DIR)/downloads/ \
--I$(MAKEFILE_DIR)/downloads/eigen-eigen-$(EIGEN_HASH) \
+-I$(MAKEFILE_DIR)/downloads/eigen-eigen-$(EIGEN_VERSION) \
 -I$(MAKEFILE_DIR)/gen/protobuf/include \
 -I$(PROTOGENDIR) \
 -I$(PBTGENDIR)

tensorflow/tools/pip_package/setup.py

@@ -108,21 +108,16 @@ class InstallHeaders(Command):
     # directories for -I
     install_dir = re.sub('/google/protobuf/src', '', install_dir)
 
-    # Copy eigen code into tensorflow/include,
+    # Copy eigen code into tensorflow/include.
-    # tensorflow/include/external/eigen_archive/eigen-eigen-<revision>,
-    # and tensorflow/include/eigen-eigen-<revision>.
     # A symlink would do, but the wheel file that gets created ignores
     # symlink within the directory hierarchy.
     # NOTE(keveman): Figure out how to customize bdist_wheel package so
     # we can do the symlink.
-    if re.search(r'(external/eigen_archive/eigen-eigen-\w+)', install_dir):
+    if 'external/eigen_archive/' in install_dir:
-      extra_dirs = [re.sub('/external/eigen_archive', '', install_dir),
+      extra_dir = install_dir.replace('external/eigen_archive', '')
-                    re.sub(r'external/eigen_archive/eigen-eigen-\w+', '',
+      if not os.path.exists(extra_dir):
-                           install_dir)]
+        self.mkpath(extra_dir)
-      for extra_dir in extra_dirs:
+      self.copy_file(header, extra_dir)
-        if not os.path.exists(extra_dir):
-          self.mkpath(extra_dir)
-        self.copy_file(header, extra_dir)
 
     if not os.path.exists(install_dir):
       self.mkpath(install_dir)

tensorflow/workspace.bzl

@@ -4,10 +4,17 @@
 # within the workspace (e.g. "tensorflow/"), and tf_repo_name is the name of the
 # local_repository rule (e.g. "@tf").
 def tf_workspace(path_prefix = "", tf_repo_name = ""):
+
+  # These lines need to be changed when updating Eigen. They are parsed from
+  # this file by the cmake and make builds to determine the eigen version and hash.
+  eigen_version = "b4fa9622b809"
+  eigen_sha256 = "2862840c2de9c0473a4ef20f8678949ae89ab25965352ee53329e63ba46cec62"
+
   native.new_http_archive(
     name = "eigen_archive",
-    url = "https://bitbucket.org/eigen/eigen/get/b4fa9622b809.tar.gz",
+    url = "https://bitbucket.org/eigen/eigen/get/" + eigen_version + ".tar.gz",
-    sha256 = "2862840c2de9c0473a4ef20f8678949ae89ab25965352ee53329e63ba46cec62",
+    sha256 = eigen_sha256,
+    strip_prefix = "eigen-eigen-" + eigen_version,
     build_file = path_prefix + "eigen.BUILD",
   )
 

third_party/eigen3/BUILD

@@ -13,7 +13,6 @@ cc_library(
         "unsupported/Eigen/CXX11/FixedPoint",
         "unsupported/Eigen/CXX11/src/FixedPoint/*.h",
     ]),
-    includes = ["."],
     visibility = ["//visibility:public"],
     deps = [
         "@eigen_archive//:eigen",

third_party/eigen3/Eigen/Cholesky

@@ -1 +1 @@
-#include "eigen-eigen-b4fa9622b809/Eigen/Cholesky"
+#include "Eigen/Cholesky"

third_party/eigen3/Eigen/Core

@@ -1 +1 @@
-#include "eigen-eigen-b4fa9622b809/Eigen/Core"
+#include "Eigen/Core"

third_party/eigen3/Eigen/Eigenvalues

@@ -1 +1 @@
-#include "eigen-eigen-b4fa9622b809/Eigen/Eigenvalues"
+#include "Eigen/Eigenvalues"

third_party/eigen3/Eigen/LU

@@ -1 +1 @@
-#include "eigen-eigen-b4fa9622b809/Eigen/LU"
+#include "Eigen/LU"

third_party/eigen3/Eigen/QR

@@ -1 +1 @@
-#include "eigen-eigen-b4fa9622b809/Eigen/QR"
+#include "Eigen/QR"

third_party/eigen3/unsupported/Eigen/CXX11/Tensor

@@ -1 +1 @@
-#include "eigen-eigen-b4fa9622b809/unsupported/Eigen/CXX11/Tensor"
+#include "unsupported/Eigen/CXX11/Tensor"

third_party/eigen3/unsupported/Eigen/SpecialFunctions

@@ -1 +1 @@
-#include "eigen-eigen-b4fa9622b809/unsupported/Eigen/SpecialFunctions"
+#include "unsupported/Eigen/SpecialFunctions"

third_party/gpus/crosstool/clang/bin/crosstool_wrapper_driver_is_not_gcc

@@ -100,36 +100,15 @@ def GetHostCompilerOptions(argv):
   args, _ = parser.parse_known_args(argv)
 
   opts = ''
-  # This is a temporary workaround for b/12960069.
-  # NVIDIA is going to fix this in CUDA 6.5, but until then this workaround
-  # will let us compile Thrust with the cuda crosstool.
-  # bazel passes all include directories as '-isystem dir' to the crosstool.
-  # This causes nvcc to think that there are kernel launches from system
-  # directories (which apparently is not supported by the compiler). This
-  # workaround changes '-isystem third_party/gpus/cuda/include' to
-  # '-iquote third_party/gpus/cuda/include'.
-  isystem_args = [x for x in args.isystem
-                  if 'third_party/gpus/cuda/include' not in x]
-  iquote_args = (args.iquote +
-                 [x for x in args.isystem
-                  if 'third_party/gpus/cuda/include' in x])
-  # This hack is needed so that we can compile eigen3. We need to include
-  # third_party/eigen3 with -I. Some eigen file include using the
-  # include <Eigen/Core> syntax, and -iquote doesn't work for that.
-  has_eigen = ['third_party/eigen3'] in isystem_args
-  if has_eigen:
-    isystem_args.remove(['third_party/eigen3'])
 
-  if isystem_args:
+  if args.isystem:
-    opts += '-isystem ' + ' -isystem '.join(sum(isystem_args, []))
+    opts += ' -isystem ' + ' -isystem '.join(sum(args.isystem, []))
-  if iquote_args:
+  if args.iquote:
-    opts += ' -iquote ' + ' -iquote '.join(sum(iquote_args, []))
+    opts += ' -iquote ' + ' -iquote '.join(sum(args.iquote, []))
   if args.g:
     opts += ' -g' + ' -g'.join(sum(args.g, []))
   if args.sysroot:
     opts += ' --sysroot ' + args.sysroot[0]
-  if has_eigen:
-    opts += ' -I third_party/eigen3'
 
   return opts