[4/4] Support for macOS - Address review comments

google-deepmind · Mar 21, 2021 · eef29e5 · eef29e5
1 parent 8fbb22d
commit eef29e5
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 46 deletions.
diff --git a/configure.py b/configure.py
@@ -38,7 +38,6 @@
 """
 import argparse
 import os
-import platform
 import subprocess
 import sys
 
@@ -71,7 +70,7 @@ def main():
   setup_python(environ_cp)
 
   write_to_bazelrc('')
-  if platform.system() == 'Darwin':
+  if sys.platform == 'darwin':
     write_to_bazelrc('# https://github.com/googleapis/google-cloud-cpp-spanner/issues/1003')
     write_to_bazelrc('build --copt=-DGRPC_BAZEL_BUILD')
   else:

diff --git a/oss_build.sh b/oss_build.sh
@@ -88,40 +88,27 @@ for python_version in $PYTHON_VERSIONS; do
     bazel clean
   fi
 
-  if [ "$(uname)" = "Darwin" ]; then
-    if [ "$python_version" = "3.6" ]; then
-      export PYTHON_BIN_PATH=python3.6
-    elif [ "$python_version" = "3.7" ]; then
-      export PYTHON_BIN_PATH=python3.7
-      ABI=cp37
-    elif [ "$python_version" = "3.8" ]; then
-      export PYTHON_BIN_PATH=python3.8
-      ABI=cp38
-    else
-      echo "Error unknown --python. Only [3.6|3.7|3.8]"
-      exit
-    fi
+  if [ "$python_version" = "3.6" ]; then
+    ABI=cp36
+  elif [ "$python_version" = "3.7" ]; then
+    ABI=cp37
+  elif [ "$python_version" = "3.8" ]; then
+    ABI=cp38
+  else
+    echo "Error unknown --python. Only [3.6|3.7|3.8]"
+    exit 1
+  fi
 
-    export PYTHON_LIB_PATH=`$PYTHON_BIN_PATH -c 'import site; print("\\n".join(site.getsitepackages()))'`
+  export PYTHON_BIN_PATH=`which python${python_version}`
+  export PYTHON_LIB_PATH=`${PYTHON_BIN_PATH} -c 'import site; print(site.getsitepackages()[0])'`
 
+  if [ "$(uname)" = "Darwin" ]; then
     bazel_config=""
     version=`sw_vers -productVersion | sed 's/\./_/g' | cut -d"_" -f1,2`
-    PLATFORM="macosx_${version}_x86_64"
+    PLATFORM="macosx_${version}_"`uname -m`
   else
-    if [ "$python_version" = "3.6" ]; then
-      export PYTHON_BIN_PATH=/usr/bin/python3.6 && export PYTHON_LIB_PATH=/usr/local/lib/python3.6/dist-packages
-    elif [ "$python_version" = "3.7" ]; then
-      export PYTHON_BIN_PATH=/usr/local/bin/python3.7 && export PYTHON_LIB_PATH=/usr/local/lib/python3.7/dist-packages
-      ABI=cp37
-    elif [ "$python_version" = "3.8" ]; then
-      export PYTHON_BIN_PATH=/usr/bin/python3.8 && export PYTHON_LIB_PATH=/usr/local/lib/python3.8/dist-packages
-      ABI=cp38
-    else
-      echo "Error unknown --python. Only [3.6|3.7|3.8]"
-      exit
-    fi
-
     bazel_config="--config=manylinux2010"
+    bazel_config=""
     PLATFORM="manylinux2010_x86_64"
   fi
 

diff --git a/reverb/cc/platform/default/repo.bzl b/reverb/cc/platform/default/repo.bzl
@@ -94,18 +94,20 @@ def _find_python_solib_path(repo_ctx):
         fail("Could not locate python shared library path:\n{}"
             .format(exec_result.stderr))
     version = exec_result.stdout.splitlines()[-1]
-    basename = "lib{}.so".format(version)
     exec_result = repo_ctx.execute(
         ["{}-config".format(version), "--configdir"],
         quiet = True,
     )
     if exec_result.return_code != 0:
         fail("Could not locate python shared library path:\n{}"
             .format(exec_result.stderr))
-    solib_dir = exec_result.stdout.splitlines()[-1]
+
     if is_darwin(repo_ctx):
         basename = "lib{}m.dylib".format(version)
-        solib_dir = "/".join(solib_dir.split("/")[:-2])
+        solib_dir = "/".join(exec_result.stdout.splitlines()[-1].split("/")[:-2])
+    else:
+        basename = "lib{}.so".format(version)
+        solib_dir = exec_result.stdout.splitlines()[-1]
 
     full_path = repo_ctx.path("{}/{}".format(solib_dir, basename))
     if not full_path.exists:
@@ -228,20 +230,21 @@ filegroup(
 def _tensorflow_solib_repo_impl(repo_ctx):
     tf_lib_path = _find_tf_lib_path(repo_ctx)
     repo_ctx.symlink(tf_lib_path, "tensorflow_solib")
-    suffix = "so.2"
     if is_darwin(repo_ctx):
         suffix = "2.dylib"
+    else:
+        suffix = "so.2"
 
     repo_ctx.file(
         "BUILD",
         content = """
 cc_library(
     name = "framework_lib",
-    srcs = ["tensorflow_solib/libtensorflow_framework.{}"],
+    srcs = ["tensorflow_solib/libtensorflow_framework.{suffix}"],
     deps = ["@python_includes", "@python_includes//:numpy_includes"],
     visibility = ["//visibility:public"],
 )
-""".format(suffix))
+""".format(suffix=suffix))
 
 def _python_includes_repo_impl(repo_ctx):
     python_include_path = _find_python_include_path(repo_ctx)
@@ -254,10 +257,11 @@ def _python_includes_repo_impl(repo_ctx):
         python_solib.basename,
     )
 
-    python_includes_srcs = 'srcs = ["%s"],' % python_solib.basename
     if is_darwin(repo_ctx):
         # Fix Fatal Python error: PyThreadState_Get: no current thread
         python_includes_srcs = ""
+    else:
+        python_includes_srcs = 'srcs = ["%s"],' % python_solib.basename
 
     # Note, "@python_includes" is a misnomer since we include the
     # libpythonX.Y.so in the srcs, so we can get access to python's various
@@ -268,7 +272,7 @@ def _python_includes_repo_impl(repo_ctx):
 cc_library(
     name = "python_includes",
     hdrs = glob(["python_includes/**/*.h"]),
-    {}
+    {srcs}
     includes = ["python_includes"],
     visibility = ["//visibility:public"],
 )
@@ -278,7 +282,7 @@ cc_library(
     includes = ["numpy_includes"],
     visibility = ["//visibility:public"],
 )
-""".format(python_includes_srcs),
+""".format(srcs=python_includes_srcs),
         executable = False,
     )
 
@@ -346,15 +350,15 @@ def _reverb_protoc_archive(ctx):
         sha256 = ""
         version = override_version
 
-    urls = [
-        "https://github.com/protocolbuffers/protobuf/releases/download/v%s/protoc-%s-linux-x86_64.zip" % (version, version),
-    ]
     if is_darwin(ctx):
-        urls = [
-            "https://github.com/protocolbuffers/protobuf/releases/download/v%s/protoc-%s-osx-x86_64.zip" % (version, version),
-        ]
-        sha256 = ""  # TODO(Feiteng) set this in WORKSPACE
+        platform = "osx"
+        sha256 = ""
+    else:
+        platform = "linux"
 
+    urls = [
+        "https://github.com/protocolbuffers/protobuf/releases/download/v%s/protoc-%s-%s-x86_64.zip" % (version, version, platform),
+    ]
     ctx.download_and_extract(
         url = urls,
         sha256 = sha256,