From 57869121cb52aac186935fa197610125f9fc9c1e Mon Sep 17 00:00:00 2001 From: Feiteng Date: Sat, 21 Nov 2020 13:45:38 +0800 Subject: [PATCH 1/9] [1/4] Support for macOS - update reverb/cc --- .bazelrc | 1 - configure.py | 8 ++++++++ docker/dev.dockerfile | 1 + docker/release.dockerfile | 1 + reverb/BUILD | 9 +++++++++ reverb/cc/platform/default/build_rules.bzl | 11 ++++++++--- reverb/cc/platform/default/repo.bzl | 18 +++++++++++++++--- 7 files changed, 42 insertions(+), 7 deletions(-) diff --git a/.bazelrc b/.bazelrc index 1de01090..b0f44c02 100644 --- a/.bazelrc +++ b/.bazelrc @@ -16,7 +16,6 @@ build --cxxopt="-std=c++14" build --cxxopt="-D_GLIBCXX_USE_CXX11_ABI=0" build --auto_output_filter=subpackages build --copt="-Wall" --copt="-Wno-sign-compare" -build --linkopt="-lrt -lm" # TF isn't built in dbg mode, so our dbg builds will segfault due to inconsistency # of defines when using tf's headers. In particular in refcount.h. diff --git a/configure.py b/configure.py index 5f28aa51..aa549c73 100644 --- a/configure.py +++ b/configure.py @@ -38,6 +38,7 @@ """ import argparse import os +import platform import subprocess import sys @@ -69,6 +70,13 @@ def main(): reset_configure_bazelrc() setup_python(environ_cp) + write_to_bazelrc('\n') + if platform.system() == 'Darwin': + write_to_bazelrc('# https://github.com/googleapis/google-cloud-cpp-spanner/issues/1003') + write_to_bazelrc('build --copt=-DGRPC_BAZEL_BUILD') + else: + write_to_bazelrc('build --linkopt="-lrt -lm"') + def get_from_env_or_user_or_default(environ_cp, var_name, ask_for_var, var_default): diff --git a/docker/dev.dockerfile b/docker/dev.dockerfile index 9185ffcb..ce624068 100644 --- a/docker/dev.dockerfile +++ b/docker/dev.dockerfile @@ -80,6 +80,7 @@ ARG pip_dependencies=' \ numpy \ oauth2client \ pandas \ + platform \ portpicker' diff --git a/docker/release.dockerfile b/docker/release.dockerfile index 2243e908..08df52c0 100644 --- a/docker/release.dockerfile +++ b/docker/release.dockerfile @@ -63,6 +63,7 @@ ARG pip_dependencies=' \ numpy \ oauth2client \ pandas \ + platform \ portpicker' # TODO(b/154930404): Update to 2.2.0 once it's out. May need to diff --git a/reverb/BUILD b/reverb/BUILD index 06d3d9cc..55c333db 100644 --- a/reverb/BUILD +++ b/reverb/BUILD @@ -16,6 +16,15 @@ licenses(["notice"]) exports_files(["LICENSE"]) +config_setting( + name = "macos", + values = { + "apple_platform_type": "macos", + "cpu": "darwin", + }, + visibility = ["//visibility:public"], +) + reverb_pytype_strict_library( name = "reverb", srcs = ["__init__.py"], diff --git a/reverb/cc/platform/default/build_rules.bzl b/reverb/cc/platform/default/build_rules.bzl index d9298fd3..9e21a5dd 100644 --- a/reverb/cc/platform/default/build_rules.bzl +++ b/reverb/cc/platform/default/build_rules.bzl @@ -310,7 +310,6 @@ def reverb_gen_op_wrapper_py(name, out, kernel_lib, linkopts = [], **kwargs): ], linkshared = 1, linkopts = linkopts + _rpath_linkopts(module_name) + [ - "-Wl,--version-script", "$(location %s)" % version_script_file, ], **kwargs @@ -359,7 +358,14 @@ def _rpath_linkopts(name): # ops) are picked up as long as they are in either the same or a parent # directory in the tensorflow/ tree. levels_to_root = native.package_name().count("/") + name.count("/") - return ["-Wl,%s" % (_make_search_paths("$$ORIGIN", levels_to_root),)] + return select({ + "//reverb:macos": [ + "-Wl,%s" % (_make_search_paths("$$ORIGIN", levels_to_root),), + ], + "//conditions:default": [ + "-Wl,--version-script,%s" % (_make_search_paths("$$ORIGIN", levels_to_root),), + ], + }) def reverb_pybind_extension( name, @@ -442,7 +448,6 @@ def reverb_pybind_extension( "-fvisibility=hidden", # avoid pybind symbol clashes between DSOs. ], linkopts = linkopts + _rpath_linkopts(module_name) + [ - "-Wl,--version-script", "$(location %s)" % version_script_file, ], deps = depset(deps + [ diff --git a/reverb/cc/platform/default/repo.bzl b/reverb/cc/platform/default/repo.bzl index 200e9d11..62aedf76 100644 --- a/reverb/cc/platform/default/repo.bzl +++ b/reverb/cc/platform/default/repo.bzl @@ -90,6 +90,9 @@ def _find_python_solib_path(repo_ctx): .format(exec_result.stderr)) version = exec_result.stdout.splitlines()[-1] basename = "lib{}.so".format(version) + if repo_ctx.os.name.lower().find("mac") != -1: + basename = "lib{}m.a".format(version) + exec_result = repo_ctx.execute( ["{}-config".format(version), "--configdir"], quiet = True, @@ -219,17 +222,20 @@ 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 = "2.so" + if repo_ctx.os.name.lower().find("mac") != -1: + suffix = "2.dylib" + repo_ctx.file( "BUILD", content = """ cc_library( name = "framework_lib", - srcs = ["tensorflow_solib/libtensorflow_framework.so.2"], + srcs = ["tensorflow_solib/libtensorflow_framework.{}"], deps = ["@python_includes", "@python_includes//:numpy_includes"], visibility = ["//visibility:public"], ) -""", - ) +""".format(suffix)) def _python_includes_repo_impl(repo_ctx): python_include_path = _find_python_include_path(repo_ctx) @@ -332,6 +338,12 @@ def _reverb_protoc_archive(ctx): urls = [ "https://github.com/protocolbuffers/protobuf/releases/download/v%s/protoc-%s-linux-x86_64.zip" % (version, version), ] + if ctx.os.name.lower().find("mac") != -1: + 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 + ctx.download_and_extract( url = urls, sha256 = sha256, From b6063a3aea9f770da5810b8374f5d3fe58c96c2b Mon Sep 17 00:00:00 2001 From: Feiteng Date: Sat, 21 Nov 2020 19:50:45 +0800 Subject: [PATCH 2/9] [2/4] Support for macOS - Fix python code build --- configure.py | 2 +- oss_build.sh | 52 +++++++++++++++------- reverb/cc/platform/default/build_rules.bzl | 34 ++++++++------ reverb/cc/platform/default/repo.bzl | 9 ++-- reverb/pip_package/build_pip_package.sh | 8 +++- 5 files changed, 70 insertions(+), 35 deletions(-) diff --git a/configure.py b/configure.py index aa549c73..b2ce1f2b 100644 --- a/configure.py +++ b/configure.py @@ -70,7 +70,7 @@ def main(): reset_configure_bazelrc() setup_python(environ_cp) - write_to_bazelrc('\n') + write_to_bazelrc('') if platform.system() == 'Darwin': write_to_bazelrc('# https://github.com/googleapis/google-cloud-cpp-spanner/issues/1003') write_to_bazelrc('build --copt=-DGRPC_BAZEL_BUILD') diff --git a/oss_build.sh b/oss_build.sh index 4888a68f..eeb35d81 100644 --- a/oss_build.sh +++ b/oss_build.sh @@ -87,17 +87,39 @@ for python_version in $PYTHON_VERSIONS; do bazel clean fi - 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 + 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 + + bazel_config="" + version=`sw_vers | grep ProductVersion | awk '{print $2}' | sed 's/\./_/g' | cut -d"_" -f1,2` + PLATFORM="macosx_${version}_x86_64" else - echo "Error unknown --python. Only [3.6|3.7|3.8]" - exit + 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" + PLATFORM="manylinux2010_x86_64" fi # Configures Bazel environment for selected Python version. @@ -110,11 +132,11 @@ for python_version in $PYTHON_VERSIONS; do # someone's system unexpectedly. We are executing the python tests after # installing the final package making this approach satisfactory. # TODO(b/157223742): Execute Python tests as well. - bazel test -c opt --copt=-mavx --config=manylinux2010 --test_output=errors //reverb/cc/... + bazel test -c opt --copt=-mavx $bazel_config --test_output=errors //reverb/cc/... - # Builds Reverb and creates the wheel package. - bazel build -c opt --copt=-mavx --config=manylinux2010 reverb/pip_package:build_pip_package - ./bazel-bin/reverb/pip_package/build_pip_package --dst $OUTPUT_DIR $PIP_PKG_EXTRA_ARGS + # # Builds Reverb and creates the wheel package. + bazel build -c opt --copt=-mavx $bazel_config reverb/pip_package:build_pip_package + ./bazel-bin/reverb/pip_package/build_pip_package --dst $OUTPUT_DIR $PIP_PKG_EXTRA_ARGS --plat "$PLATFORM" # Installs pip package. $PYTHON_BIN_PATH -mpip install ${OUTPUT_DIR}*${ABI}*.whl @@ -123,7 +145,7 @@ for python_version in $PYTHON_VERSIONS; do echo "Run Python tests..." set +e - bash run_python_tests.sh |& tee ./unittest_log.txt + bash run_python_tests.sh 2>&1| tee ./unittest_log.txt UNIT_TEST_ERROR_CODE=$? set -e if [[ $UNIT_TEST_ERROR_CODE != 0 ]]; then diff --git a/reverb/cc/platform/default/build_rules.bzl b/reverb/cc/platform/default/build_rules.bzl index 9e21a5dd..487b8b17 100644 --- a/reverb/cc/platform/default/build_rules.bzl +++ b/reverb/cc/platform/default/build_rules.bzl @@ -309,9 +309,16 @@ def reverb_gen_op_wrapper_py(name, out, kernel_lib, linkopts = [], **kwargs): "-fvisibility=hidden", # avoid symbol clashes between DSOs. ], linkshared = 1, - linkopts = linkopts + _rpath_linkopts(module_name) + [ - "$(location %s)" % version_script_file, - ], + linkopts = linkopts + _rpath_linkopts(module_name) + select({ + "//reverb:macos": [ + "-Wl", + "$(location %s)" % version_script_file, + ], + "//conditions:default": [ + "-Wl,--version-script", + "$(location %s)" % version_script_file, + ], + }), **kwargs ) native.genrule( @@ -358,14 +365,7 @@ def _rpath_linkopts(name): # ops) are picked up as long as they are in either the same or a parent # directory in the tensorflow/ tree. levels_to_root = native.package_name().count("/") + name.count("/") - return select({ - "//reverb:macos": [ - "-Wl,%s" % (_make_search_paths("$$ORIGIN", levels_to_root),), - ], - "//conditions:default": [ - "-Wl,--version-script,%s" % (_make_search_paths("$$ORIGIN", levels_to_root),), - ], - }) + return ["-Wl,%s" % (_make_search_paths("$$ORIGIN", levels_to_root),)] def reverb_pybind_extension( name, @@ -447,9 +447,15 @@ def reverb_pybind_extension( "-fexceptions", # pybind relies on exceptions, required to compile. "-fvisibility=hidden", # avoid pybind symbol clashes between DSOs. ], - linkopts = linkopts + _rpath_linkopts(module_name) + [ - "$(location %s)" % version_script_file, - ], + linkopts = linkopts + _rpath_linkopts(module_name) + + select({"//reverb:macos": [ + "-Wl,-exported_symbols_list,$(location %s)" % exported_symbols_file, + ], + "//conditions:default": [ + "-Wl,--version-script", + "$(location %s)" % version_script_file, + ], + }), deps = depset(deps + [ exported_symbols_file, version_script_file, diff --git a/reverb/cc/platform/default/repo.bzl b/reverb/cc/platform/default/repo.bzl index 62aedf76..e66d9c55 100644 --- a/reverb/cc/platform/default/repo.bzl +++ b/reverb/cc/platform/default/repo.bzl @@ -90,9 +90,6 @@ def _find_python_solib_path(repo_ctx): .format(exec_result.stderr)) version = exec_result.stdout.splitlines()[-1] basename = "lib{}.so".format(version) - if repo_ctx.os.name.lower().find("mac") != -1: - basename = "lib{}m.a".format(version) - exec_result = repo_ctx.execute( ["{}-config".format(version), "--configdir"], quiet = True, @@ -101,6 +98,10 @@ def _find_python_solib_path(repo_ctx): fail("Could not locate python shared library path:\n{}" .format(exec_result.stderr)) solib_dir = exec_result.stdout.splitlines()[-1] + if repo_ctx.os.name.lower().find("mac") != -1: + basename = "lib{}m.dylib".format(version) + solib_dir = "/".join(solib_dir.split("/")[:-2]) + full_path = repo_ctx.path("{}/{}".format(solib_dir, basename)) if not full_path.exists: fail("Unable to find python shared library file:\n{}/{}" @@ -222,7 +223,7 @@ 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 = "2.so" + suffix = "so.2" if repo_ctx.os.name.lower().find("mac") != -1: suffix = "2.dylib" diff --git a/reverb/pip_package/build_pip_package.sh b/reverb/pip_package/build_pip_package.sh index f15b83d1..200c62e5 100755 --- a/reverb/pip_package/build_pip_package.sh +++ b/reverb/pip_package/build_pip_package.sh @@ -32,7 +32,7 @@ function build_wheel() { pushd ${TMPDIR} > /dev/null echo $(date) : "=== Building wheel" - "${PYTHON_BIN_PATH}" setup.py bdist_wheel ${PKG_NAME_FLAG} ${RELEASE_FLAG} ${TF_VERSION_FLAG} --plat manylinux2010_x86_64 > /dev/null + "${PYTHON_BIN_PATH}" setup.py bdist_wheel ${PKG_NAME_FLAG} ${RELEASE_FLAG} ${TF_VERSION_FLAG} --plat $PLATFORM > /dev/null DEST=${TMPDIR}/dist/ if [[ ! "$TMPDIR" -ef "$DESTDIR" ]]; then mkdir -p ${DESTDIR} @@ -80,6 +80,7 @@ function usage() { echo " --release build a release version" echo " --dst path to copy the .whl into." echo " --tf-version tensorflow version dependency passed to setup.py." + echo " --plat platform." echo "" exit 1 } @@ -91,6 +92,8 @@ function main() { # This is where the source code is copied and where the whl will be built. DST_DIR="" + PLATFORM="manylinux2010_x86_64" + while true; do if [[ "$1" == "--help" ]]; then usage @@ -103,6 +106,9 @@ function main() { elif [[ "$1" == "--tf-version" ]]; then shift TF_VERSION_FLAG="--tf-version $1" + elif [[ "$1" == "--plat" ]]; then + shift + PLATFORM=$1 fi if [[ -z "$1" ]]; then From 32c3441d119217f2461b89b971d12acb87ee8c00 Mon Sep 17 00:00:00 2001 From: Feiteng Date: Thu, 3 Dec 2020 20:33:02 +0800 Subject: [PATCH 3/9] [3/4] Support for macOS - Fix Library not loaded ImportError --- reverb/cc/platform/default/build_rules.bzl | 27 ++++++++++++++++------ 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/reverb/cc/platform/default/build_rules.bzl b/reverb/cc/platform/default/build_rules.bzl index 487b8b17..9386f172 100644 --- a/reverb/cc/platform/default/build_rules.bzl +++ b/reverb/cc/platform/default/build_rules.bzl @@ -293,6 +293,14 @@ def reverb_gen_op_wrapper_py(name, out, kernel_lib, linkopts = [], **kwargs): fail("Argument out must end with '.py', but saw: {}".format(out)) module_name = "lib{}_gen_op".format(name) + exported_symbols_file = "%s-exported-symbols.lds" % module_name + native.genrule( + name = module_name + "_exported_symbols", + outs = [exported_symbols_file], + cmd = "echo '*ReverbDataset*\n*ReverbClient*' >$@", + output_licenses = ["unencumbered"], + visibility = ["//visibility:private"], + ) version_script_file = "%s-version-script.lds" % module_name native.genrule( name = module_name + "_version_script", @@ -303,20 +311,25 @@ def reverb_gen_op_wrapper_py(name, out, kernel_lib, linkopts = [], **kwargs): ) native.cc_binary( name = "{}.so".format(module_name), - deps = [kernel_lib] + reverb_tf_deps() + [version_script_file], + deps = [kernel_lib] + reverb_tf_deps() + [ + exported_symbols_file, + version_script_file + ], copts = tf_copts() + [ "-fno-strict-aliasing", # allow a wider range of code [aliasing] to compile. - "-fvisibility=hidden", # avoid symbol clashes between DSOs. - ], + ]+ select({ + "//reverb:macos": [], + "//conditions:default": [ + "-fvisibility=hidden", # avoid symbol clashes between DSOs. + ], + }), linkshared = 1, linkopts = linkopts + _rpath_linkopts(module_name) + select({ "//reverb:macos": [ - "-Wl", - "$(location %s)" % version_script_file, + "-Wl,-exported_symbols_list,$(location %s)" % exported_symbols_file, ], "//conditions:default": [ - "-Wl,--version-script", - "$(location %s)" % version_script_file, + "-Wl,--version-script,$(location %s)" % version_script_file, ], }), **kwargs From aed9c51364bed4e3af0fb802b8688143a5aa551b Mon Sep 17 00:00:00 2001 From: Feiteng Date: Thu, 3 Dec 2020 20:43:31 +0800 Subject: [PATCH 4/9] [3/4] Support for macOS - Format build scripts --- oss_build.sh | 12 ++++++------ reverb/cc/platform/default/build_rules.bzl | 12 +++++------- reverb/cc/platform/default/repo.bzl | 11 ++++++++--- reverb/pip_package/build_pip_package.sh | 4 ++-- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/oss_build.sh b/oss_build.sh index eeb35d81..6e1e69e5 100644 --- a/oss_build.sh +++ b/oss_build.sh @@ -102,7 +102,7 @@ for python_version in $PYTHON_VERSIONS; do fi bazel_config="" - version=`sw_vers | grep ProductVersion | awk '{print $2}' | sed 's/\./_/g' | cut -d"_" -f1,2` + version=`sw_vers -productVersion | sed 's/\./_/g' | cut -d"_" -f1,2` PLATFORM="macosx_${version}_x86_64" else if [ "$python_version" = "3.6" ]; then @@ -132,11 +132,11 @@ for python_version in $PYTHON_VERSIONS; do # someone's system unexpectedly. We are executing the python tests after # installing the final package making this approach satisfactory. # TODO(b/157223742): Execute Python tests as well. - bazel test -c opt --copt=-mavx $bazel_config --test_output=errors //reverb/cc/... + bazel test -c opt --copt=-mavx ${bazel_config} --test_output=errors //reverb/cc/... - # # Builds Reverb and creates the wheel package. - bazel build -c opt --copt=-mavx $bazel_config reverb/pip_package:build_pip_package - ./bazel-bin/reverb/pip_package/build_pip_package --dst $OUTPUT_DIR $PIP_PKG_EXTRA_ARGS --plat "$PLATFORM" + # Builds Reverb and creates the wheel package. + bazel build -c opt --copt=-mavx ${bazel_config} reverb/pip_package:build_pip_package + ./bazel-bin/reverb/pip_package/build_pip_package --dst $OUTPUT_DIR $PIP_PKG_EXTRA_ARGS --platform "${PLATFORM}" # Installs pip package. $PYTHON_BIN_PATH -mpip install ${OUTPUT_DIR}*${ABI}*.whl @@ -145,7 +145,7 @@ for python_version in $PYTHON_VERSIONS; do echo "Run Python tests..." set +e - bash run_python_tests.sh 2>&1| tee ./unittest_log.txt + bash run_python_tests.sh 2>&1 | tee ./unittest_log.txt UNIT_TEST_ERROR_CODE=$? set -e if [[ $UNIT_TEST_ERROR_CODE != 0 ]]; then diff --git a/reverb/cc/platform/default/build_rules.bzl b/reverb/cc/platform/default/build_rules.bzl index 9386f172..35c99593 100644 --- a/reverb/cc/platform/default/build_rules.bzl +++ b/reverb/cc/platform/default/build_rules.bzl @@ -294,10 +294,12 @@ def reverb_gen_op_wrapper_py(name, out, kernel_lib, linkopts = [], **kwargs): module_name = "lib{}_gen_op".format(name) exported_symbols_file = "%s-exported-symbols.lds" % module_name + # gen_client_ops -> ReverbClient + symbol = "Reverb{}".format(name.split('_')[1].capitalize()) native.genrule( name = module_name + "_exported_symbols", outs = [exported_symbols_file], - cmd = "echo '*ReverbDataset*\n*ReverbClient*' >$@", + cmd = "echo '*%s*' >$@" % symbol, output_licenses = ["unencumbered"], visibility = ["//visibility:private"], ) @@ -317,12 +319,8 @@ def reverb_gen_op_wrapper_py(name, out, kernel_lib, linkopts = [], **kwargs): ], copts = tf_copts() + [ "-fno-strict-aliasing", # allow a wider range of code [aliasing] to compile. - ]+ select({ - "//reverb:macos": [], - "//conditions:default": [ - "-fvisibility=hidden", # avoid symbol clashes between DSOs. - ], - }), + "-fvisibility=hidden", # avoid symbol clashes between DSOs. + ], linkshared = 1, linkopts = linkopts + _rpath_linkopts(module_name) + select({ "//reverb:macos": [ diff --git a/reverb/cc/platform/default/repo.bzl b/reverb/cc/platform/default/repo.bzl index e66d9c55..04b4fe46 100644 --- a/reverb/cc/platform/default/repo.bzl +++ b/reverb/cc/platform/default/repo.bzl @@ -2,6 +2,11 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") +def is_darwin(ctx): + if ctx.os.name.lower().find("mac") != -1: + return True + return False + # Sanitize a dependency so that it works correctly from code that includes # reverb as a submodule. def clean_dep(dep): @@ -98,7 +103,7 @@ def _find_python_solib_path(repo_ctx): fail("Could not locate python shared library path:\n{}" .format(exec_result.stderr)) solib_dir = exec_result.stdout.splitlines()[-1] - if repo_ctx.os.name.lower().find("mac") != -1: + if is_darwin(repo_ctx): basename = "lib{}m.dylib".format(version) solib_dir = "/".join(solib_dir.split("/")[:-2]) @@ -224,7 +229,7 @@ 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 repo_ctx.os.name.lower().find("mac") != -1: + if is_darwin(repo_ctx): suffix = "2.dylib" repo_ctx.file( @@ -339,7 +344,7 @@ def _reverb_protoc_archive(ctx): urls = [ "https://github.com/protocolbuffers/protobuf/releases/download/v%s/protoc-%s-linux-x86_64.zip" % (version, version), ] - if ctx.os.name.lower().find("mac") != -1: + if is_darwin(ctx): urls = [ "https://github.com/protocolbuffers/protobuf/releases/download/v%s/protoc-%s-osx-x86_64.zip" % (version, version), ] diff --git a/reverb/pip_package/build_pip_package.sh b/reverb/pip_package/build_pip_package.sh index 200c62e5..8bf75c74 100755 --- a/reverb/pip_package/build_pip_package.sh +++ b/reverb/pip_package/build_pip_package.sh @@ -32,7 +32,7 @@ function build_wheel() { pushd ${TMPDIR} > /dev/null echo $(date) : "=== Building wheel" - "${PYTHON_BIN_PATH}" setup.py bdist_wheel ${PKG_NAME_FLAG} ${RELEASE_FLAG} ${TF_VERSION_FLAG} --plat $PLATFORM > /dev/null + "${PYTHON_BIN_PATH}" setup.py bdist_wheel ${PKG_NAME_FLAG} ${RELEASE_FLAG} ${TF_VERSION_FLAG} --plat ${PLATFORM} > /dev/null DEST=${TMPDIR}/dist/ if [[ ! "$TMPDIR" -ef "$DESTDIR" ]]; then mkdir -p ${DESTDIR} @@ -106,7 +106,7 @@ function main() { elif [[ "$1" == "--tf-version" ]]; then shift TF_VERSION_FLAG="--tf-version $1" - elif [[ "$1" == "--plat" ]]; then + elif [[ "$1" == "--platform" ]]; then shift PLATFORM=$1 fi From c52865e0cbb223c29d282af25af88834e8aeb3da Mon Sep 17 00:00:00 2001 From: Feiteng Date: Sat, 2 Jan 2021 18:38:05 +0800 Subject: [PATCH 5/9] [3/4] Support for macOS - Fix @loader_path/../_solib_darwin image not found --- oss_build.sh | 4 +++- reverb/pip_package/build_pip_package.sh | 13 +++++++++++++ reverb/pip_package/setup.py | 13 +++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/oss_build.sh b/oss_build.sh index 6e1e69e5..b6aeba29 100644 --- a/oss_build.sh +++ b/oss_build.sh @@ -101,6 +101,8 @@ for python_version in $PYTHON_VERSIONS; do exit fi + export PYTHON_LIB_PATH=`$PYTHON_BIN_PATH -c 'import site; print("\\n".join(site.getsitepackages()))'` + bazel_config="" version=`sw_vers -productVersion | sed 's/\./_/g' | cut -d"_" -f1,2` PLATFORM="macosx_${version}_x86_64" @@ -139,7 +141,7 @@ for python_version in $PYTHON_VERSIONS; do ./bazel-bin/reverb/pip_package/build_pip_package --dst $OUTPUT_DIR $PIP_PKG_EXTRA_ARGS --platform "${PLATFORM}" # Installs pip package. - $PYTHON_BIN_PATH -mpip install ${OUTPUT_DIR}*${ABI}*.whl + $PYTHON_BIN_PATH -mpip install --force-reinstall ${OUTPUT_DIR}*${ABI}*.whl if [ "$PYTHON_TESTS" = "true" ]; then echo "Run Python tests..." diff --git a/reverb/pip_package/build_pip_package.sh b/reverb/pip_package/build_pip_package.sh index 8bf75c74..c474e252 100755 --- a/reverb/pip_package/build_pip_package.sh +++ b/reverb/pip_package/build_pip_package.sh @@ -71,6 +71,19 @@ function prepare_src() { # must remain where they are for TF to find them. find "${TMPDIR}/reverb/cc" -type d -name ops -prune -o -name '*.so' \ -exec mv {} "${TMPDIR}/reverb" \; + + # Copy darwin libs over so they can be loaded at runtime + so_lib_dir=$(ls $RUNFILES | grep solib) || true + if [ -n "${so_lib_dir}" ]; then + mkdir -p "${TMPDIR}/${so_lib_dir}" + proto_so_dir=$(ls ${RUNFILES}/${so_lib_dir} | grep proto) || true + for dir in ${proto_so_dir}; do + echo "===== DIR = $dir" + cp -R ${RUNFILES}/${so_lib_dir}/${dir} "${TMPDIR}/${so_lib_dir}" + done + + cp -r $TMPDIR/${so_lib_dir} `dirname $PYTHON_LIB_PATH` + fi } function usage() { diff --git a/reverb/pip_package/setup.py b/reverb/pip_package/setup.py index 9bebe9b2..120263f6 100644 --- a/reverb/pip_package/setup.py +++ b/reverb/pip_package/setup.py @@ -111,6 +111,16 @@ def run_setup(self): long_description = f.read() version, project_name = self._get_version() + + so_lib_paths = [ + i for i in os.listdir('.') + if os.path.isdir(i) and fnmatch.fnmatch(i, '_solib_*') + ] + + matches = [] + for path in so_lib_paths: + matches.extend(['../' + x for x in find_files('*', path) if '.py' not in x]) + setup( name=project_name, version=version, @@ -125,6 +135,9 @@ def run_setup(self): packages=find_packages(), headers=list(find_files('*.proto', 'reverb')), include_package_data=True, + package_data={ + 'reverb': matches, + }, install_requires=self._get_required_packages(), extras_require={ 'tensorflow': self._get_tensorflow_packages(), From f4e76ca972375bffe390af10f5bb508da3351838 Mon Sep 17 00:00:00 2001 From: Feiteng Date: Sat, 9 Jan 2021 13:35:59 +0800 Subject: [PATCH 6/9] [4/4] Support for macOS - Fix bazel test //reverb:*_test --- reverb/cc/platform/default/repo.bzl | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/reverb/cc/platform/default/repo.bzl b/reverb/cc/platform/default/repo.bzl index 04b4fe46..5c22669e 100644 --- a/reverb/cc/platform/default/repo.bzl +++ b/reverb/cc/platform/default/repo.bzl @@ -254,6 +254,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 = "" + # 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 # symbols at link time. @@ -263,7 +268,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"], ) @@ -273,7 +278,7 @@ cc_library( includes = ["numpy_includes"], visibility = ["//visibility:public"], ) -""".format(python_solib.basename), +""".format(python_includes_srcs), executable = False, ) From f4f9308dea9c4f62bf6f48713b0267e30412a557 Mon Sep 17 00:00:00 2001 From: Feiteng Date: Sun, 10 Jan 2021 12:40:56 +0800 Subject: [PATCH 7/9] [4/4] Support for macOS - Fix python tests --- reverb/cc/platform/default/build_rules.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/reverb/cc/platform/default/build_rules.bzl b/reverb/cc/platform/default/build_rules.bzl index 35c99593..91bc491d 100644 --- a/reverb/cc/platform/default/build_rules.bzl +++ b/reverb/cc/platform/default/build_rules.bzl @@ -107,7 +107,7 @@ def reverb_cc_proto_library(name, srcs = [], deps = [], **kwargs): name = "{}_static".format(name), srcs = gen_srcs, hdrs = gen_hdrs, - deps = depset(deps + reverb_tf_deps()), + deps = depset([dep.replace(":", ":lib") + ".so" for dep in deps] + reverb_tf_deps()), alwayslink = 1, **kwargs ) @@ -294,8 +294,8 @@ def reverb_gen_op_wrapper_py(name, out, kernel_lib, linkopts = [], **kwargs): module_name = "lib{}_gen_op".format(name) exported_symbols_file = "%s-exported-symbols.lds" % module_name - # gen_client_ops -> ReverbClient - symbol = "Reverb{}".format(name.split('_')[1].capitalize()) + # gen_client_ops -> reverb_client + symbol = "reverb_{}".format(name.split('_')[1]) native.genrule( name = module_name + "_exported_symbols", outs = [exported_symbols_file], From eef29e5dcfc38f0eaf17e0b1e3780750da644b6e Mon Sep 17 00:00:00 2001 From: Feiteng Date: Sun, 28 Feb 2021 17:11:53 +0800 Subject: [PATCH 8/9] [4/4] Support for macOS - Address review comments --- configure.py | 3 +- oss_build.sh | 43 ++++++++++------------------- reverb/cc/platform/default/repo.bzl | 36 +++++++++++++----------- 3 files changed, 36 insertions(+), 46 deletions(-) diff --git a/configure.py b/configure.py index b2ce1f2b..abe3adfa 100644 --- 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 index 84978e82..a5d63d85 100644 --- 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 index 5c22669e..0b0bd0a3 100644 --- a/reverb/cc/platform/default/repo.bzl +++ b/reverb/cc/platform/default/repo.bzl @@ -94,7 +94,6 @@ 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, @@ -102,10 +101,13 @@ def _find_python_solib_path(repo_ctx): 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, From 7f73e78103bec9dc68bb177a6ccb931df09dfd48 Mon Sep 17 00:00:00 2001 From: Cambridge Yang Date: Wed, 5 May 2021 01:02:01 -0400 Subject: [PATCH 9/9] hacks to fix macOS build --- .bazelrc | 2 ++ reverb/cc/platform/default/build_rules.bzl | 6 +++--- reverb/cc/platform/default/repo.bzl | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.bazelrc b/.bazelrc index b0f44c02..b98cf421 100644 --- a/.bazelrc +++ b/.bazelrc @@ -23,3 +23,5 @@ build --cxxopt="-DNDEBUG" # Options from ./configure try-import %workspace%/.reverb.bazelrc + +build --features=-supports_dynamic_linker diff --git a/reverb/cc/platform/default/build_rules.bzl b/reverb/cc/platform/default/build_rules.bzl index 35788e2d..d910d865 100644 --- a/reverb/cc/platform/default/build_rules.bzl +++ b/reverb/cc/platform/default/build_rules.bzl @@ -107,12 +107,12 @@ def reverb_cc_proto_library(name, srcs = [], deps = [], **kwargs): name = "{}_static".format(name), srcs = gen_srcs, hdrs = gen_hdrs, - deps = depset([dep.replace(":", ":lib") + ".so" for dep in deps] + reverb_tf_deps()), + deps = depset([dep.replace(":", ":libxxx") + ".so" for dep in deps] + reverb_tf_deps()), alwayslink = 1, **kwargs ) native.cc_binary( - name = "lib{}.so".format(name), + name = "libxxx{}.so".format(name), deps = ["{}_static".format(name)], linkshared = 1, **kwargs @@ -120,7 +120,7 @@ def reverb_cc_proto_library(name, srcs = [], deps = [], **kwargs): native.cc_library( name = name, hdrs = gen_hdrs, - srcs = ["lib{}.so".format(name)], + srcs = ["libxxx{}.so".format(name)], deps = depset(deps + reverb_tf_deps()), alwayslink = 1, **kwargs diff --git a/reverb/cc/platform/default/repo.bzl b/reverb/cc/platform/default/repo.bzl index e585556b..455307b5 100644 --- a/reverb/cc/platform/default/repo.bzl +++ b/reverb/cc/platform/default/repo.bzl @@ -103,7 +103,7 @@ def _find_python_solib_path(repo_ctx): .format(exec_result.stderr)) if is_darwin(repo_ctx): - basename = "lib{}m.dylib".format(version) + basename = "lib{}.dylib".format(version) solib_dir = "/".join(exec_result.stdout.splitlines()[-1].split("/")[:-2]) else: basename = "lib{}.so".format(version)