diff --git a/.bazelrc b/.bazelrc index b066fd0..8e74164 100644 --- a/.bazelrc +++ b/.bazelrc @@ -17,7 +17,7 @@ build --cxxopt="-std=c++17" build --cxxopt="-D_GLIBCXX_USE_CXX11_ABI=1" build --auto_output_filter=subpackages build --copt="-Wall" --copt="-Wno-sign-compare" -build --linkopt="-lrt -lm" + # We build with AVX and eigen byte alignment to match tensorflow's (and Eigen) # pip package byte alignment. See b/186669968 for more details. build --copt=-mavx --copt=-DEIGEN_MAX_ALIGN_BYTES=64 diff --git a/configure.py b/configure.py index 15ddcc9..675e303 100644 --- a/configure.py +++ b/configure.py @@ -74,6 +74,13 @@ def main(): reset_configure_bazelrc() setup_python(environ_cp, args.force_defaults) + write_to_bazelrc('') + 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: + 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/oss_build.sh b/oss_build.sh index ff74872..1554c56 100644 --- a/oss_build.sh +++ b/oss_build.sh @@ -95,22 +95,29 @@ for python_version in $PYTHON_VERSIONS; do fi if [ "$python_version" = "3.7" ]; then - export PYTHON_BIN_PATH=/usr/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 elif [ "$python_version" = "3.9" ]; then - export PYTHON_BIN_PATH=/usr/bin/python3.9 && export PYTHON_LIB_PATH=/usr/local/lib/python3.9/dist-packages ABI=cp39 elif [ "$python_version" = "3.10" ]; then - export PYTHON_BIN_PATH=/usr/bin/python3.10 && export PYTHON_LIB_PATH=/usr/local/lib/python3.10/dist-packages ABI=cp310 else echo "Error unknown --python. Only [3.7|3.8|3.9|3.10]" exit 1 fi + 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="" + PLATFORM=`${PYTHON_BIN_PATH} -c "from distutils import util; print(util.get_platform())"` + else + bazel_config="--config=manylinux2014" + PLATFORM="manylinux2014_x86_64" + fi + # Configures Bazel environment for selected Python version. $PYTHON_BIN_PATH configure.py @@ -121,24 +128,25 @@ 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=manylinux2014 --test_output=errors //reverb/cc/... + bazel test -c opt --copt=-mavx ${bazel_config} --test_output=errors //reverb/cc/... EXTRA_OPT="" if [ "$DEBUG_BUILD" = "true" ]; then EXTRA_OPT="--copt=-g2" fi + # Builds Reverb and creates the wheel package. - bazel build -c opt --copt=-mavx $EXTRA_OPT --config=manylinux2014 reverb/pip_package:build_pip_package - ./bazel-bin/reverb/pip_package/build_pip_package --dst $OUTPUT_DIR $PIP_PKG_EXTRA_ARGS + bazel build -c opt --copt=-mavx $EXTRA_OPT $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 + $PYTHON_BIN_PATH -m pip install --force-reinstall ${OUTPUT_DIR}*${ABI}*.whl if [ "$PYTHON_TESTS" = "true" ]; then 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/BUILD b/reverb/BUILD index c373075..2d1b798 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 c154ff3..c8826b8 100644 --- a/reverb/cc/platform/default/build_rules.bzl +++ b/reverb/cc/platform/default/build_rules.bzl @@ -110,7 +110,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 ) @@ -297,6 +297,16 @@ 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 + # gen_client_ops -> reverb_client + symbol = "reverb_{}".format(name.split('_')[1]) + native.genrule( + name = module_name + "_exported_symbols", + outs = [exported_symbols_file], + cmd = "echo '*%s*' >$@" % symbol, + output_licenses = ["unencumbered"], + visibility = ["//visibility:private"], + ) version_script_file = "%s-version-script.lds" % module_name native.genrule( name = module_name + "_version_script", @@ -307,16 +317,23 @@ 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. ], linkshared = 1, - linkopts = linkopts + _rpath_linkopts(module_name) + [ - "-Wl,--version-script", - "$(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, + ], + }), **kwargs ) native.genrule( @@ -455,10 +472,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) + [ - "-Wl,--version-script", - "$(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 19ea78e..c53697d 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 # codebase as a submodule. def clean_dep(dep): @@ -89,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, @@ -97,11 +101,21 @@ 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(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: - fail("Unable to find python shared library file:\n{}/{}" - .format(solib_dir, basename)) + basename = basename.replace('m.dylib', '.dylib') + full_path = repo_ctx.path("{}/{}".format(solib_dir, basename)) + if not full_path.exists: + fail("Unable to find python shared library file:\n{}/{}" + .format(solib_dir, basename)) return struct(dir = solib_dir, basename = basename) def _eigen_archive_repo_impl(repo_ctx): @@ -220,17 +234,24 @@ 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") + if is_darwin(repo_ctx): + tensorflow_solib = "libtensorflow_cc.2.dylib" + full_path = repo_ctx.path("tensorflow_solib/{}".format(tensorflow_solib)) + if not full_path.exists: + tensorflow_solib = "libtensorflow_framework.2.dylib" + else: + tensorflow_solib = "libtensorflow_framework.so.2" + repo_ctx.file( "BUILD", content = """ cc_library( name = "framework_lib", - srcs = ["tensorflow_solib/libtensorflow_framework.so.2"], + srcs = ["tensorflow_solib/{tensorflow_solib}"], deps = ["@python_includes", "@python_includes//:numpy_includes"], visibility = ["//visibility:public"], ) -""", - ) +""".format(tensorflow_solib=tensorflow_solib)) def _python_includes_repo_impl(repo_ctx): python_include_path = _find_python_include_path(repo_ctx) @@ -243,12 +264,19 @@ def _python_includes_repo_impl(repo_ctx): 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 + repo_ctx.file( "BUILD", content = """ cc_library( name = "python_includes", hdrs = glob(["python_includes/**/*.h"]), + {srcs} includes = ["python_includes"], visibility = ["//visibility:public"], ) @@ -258,7 +286,7 @@ cc_library( includes = ["numpy_includes"], visibility = ["//visibility:public"], ) -""".format(python_solib.basename), +""".format(srcs=python_includes_srcs), executable = False, ) @@ -359,8 +387,19 @@ def _protoc_archive(ctx): version = ctx.attr.version sha256 = ctx.attr.sha256 + override_version = ctx.os.environ.get("REVERB_PROTOC_VERSION") + if override_version: + sha256 = "" + version = override_version + + if is_darwin(ctx): + platform = "osx" + sha256 = "99729771ccb2f70621ac20f241f6ab1c70271f2c6bd2ea1ddbd9c2f7ae08d316" + else: + platform = "linux" + urls = [ - "https://github.com/protocolbuffers/protobuf/releases/download/v%s/protoc-%s-linux-x86_64.zip" % (version, version), + "https://github.com/protocolbuffers/protobuf/releases/download/v%s/protoc-%s-%s-x86_64.zip" % (version, version, platform), ] ctx.download_and_extract( url = urls, diff --git a/reverb/cc/table.cc b/reverb/cc/table.cc index 07b53b5..cd7460d 100644 --- a/reverb/cc/table.cc +++ b/reverb/cc/table.cc @@ -154,10 +154,12 @@ Table::Table(std::string name, std::shared_ptr sampler, num_unique_samples_(0), max_size_(max_size), max_enqueued_inserts_( - std::max(1L, std::min(max_size * kMaxEnqueuedInsertsPerc, + std::max(static_cast(1), + std::min(max_size * kMaxEnqueuedInsertsPerc, kMaxEnqueuedInserts))), max_enqueued_extension_ops_( - std::max(1L, std::min(max_size * kMaxPendingExtensionOpsPerc, + std::max(static_cast(1), + std::min(max_size * kMaxPendingExtensionOpsPerc, kMaxPendingExtensionOps))), max_times_sampled_(max_times_sampled), name_(std::move(name)), diff --git a/reverb/pip_package/build_pip_package.sh b/reverb/pip_package/build_pip_package.sh index f6a1fd9..45bb576 100755 --- a/reverb/pip_package/build_pip_package.sh +++ b/reverb/pip_package/build_pip_package.sh @@ -20,6 +20,7 @@ function build_wheel() { DESTDIR="$2" RELEASE_FLAG="$3" TF_VERSION_FLAG="$4" + PLATFORM="$5" # Before we leave the top-level directory, make sure we know how to # call python. @@ -32,7 +33,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 manylinux2014_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} @@ -71,6 +72,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() { @@ -80,6 +94,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 " --platform platform." echo "" exit 1 } @@ -91,6 +106,8 @@ function main() { # This is where the source code is copied and where the whl will be built. DST_DIR="" + PLATFORM="manylinux2014_x86_64" + while true; do if [[ "$1" == "--help" ]]; then usage @@ -103,6 +120,9 @@ function main() { elif [[ "$1" == "--tf-version" ]]; then shift TF_VERSION_FLAG="--tf-version $1" + elif [[ "$1" == "--platform" ]]; then + shift + PLATFORM=$1 fi if [[ -z "$1" ]]; then @@ -117,7 +137,7 @@ function main() { fi prepare_src "$TMPDIR" - build_wheel "$TMPDIR" "$DST_DIR" "$RELEASE_FLAG" "$TF_VERSION_FLAG" + build_wheel "$TMPDIR" "$DST_DIR" "$RELEASE_FLAG" "$TF_VERSION_FLAG" "$PLATFORM" } main "$@" diff --git a/reverb/pip_package/setup.py b/reverb/pip_package/setup.py index b7c878d..c4d2ba0 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(), diff --git a/run_python_tests.sh b/run_python_tests.sh index 088ce6c..1ba369f 100644 --- a/run_python_tests.sh +++ b/run_python_tests.sh @@ -29,7 +29,8 @@ py_test() { echo "===========Running Python tests============" - for test_file in `find reverb/ -name '*_test.py' -print` + cd reverb/ # Fix OSX circular import error + for test_file in `find ./ -name '*_test.py' -print` do echo "####=======Testing ${test_file}=======####" ${PYTHON_BIN_PATH} "${test_file}"