google-deepmind/tree

Don't use Bazel to build

Flamefire opened this issue · 4 comments

Just tried installing this via pip and got:

Building wheels for collected packages: dm-tree
  Building wheel for dm-tree (setup.py) ... error
  ERROR: Command errored out with exit status 1:
   command: /sw/installed/Python/3.7.4-GCCcore-8.3.0/bin/python -u -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-install-rnjp95y8/dm-tree/setup.py'"'"'; __file__='"'"'/tmp/pip-install-rnjp95y8/dm-tree/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' bdist_wheel -d /tmp/pip-wheel-8stvnwht --python-tag cp37
       cwd: /tmp/pip-install-rnjp95y8/dm-tree/
  Complete output (13 lines):
  running bdist_wheel
  running build
  running build_py
  creating build
  creating build/lib.linux-ppc64le-3.7
  creating build/lib.linux-ppc64le-3.7/tree
  copying tree/__init__.py -> build/lib.linux-ppc64le-3.7/tree
  copying tree/tree_test.py -> build/lib.linux-ppc64le-3.7/tree
  copying tree/tree_benchmark.py -> build/lib.linux-ppc64le-3.7/tree
  running build_ext
  bazel build //tree:_tree --symlink_prefix=build/temp.linux-ppc64le-3.7/bazel- --compilation_mode=opt
  unable to execute 'bazel': No such file or directory
  error: command 'bazel' failed with exit status 1
  ----------------------------------------
  ERROR: Failed building wheel for dm-tree

So this looks like Bazel is required to build/install this.

Looking over the sources this seems to be due to dependency on pybind11 and absl.

My suggestion:

  • pybind11 can easily be installed via pip as a regular Python dependency in a version requirement of your choosing
  • The few places where absl is used can easily be avoided:
    • C++11 provides unique_ptr
    • string_view is used for rfind and comparison which can be implemented easily with standard C++, although a bit longer
    • StrCat can similarily done in about 6 lines
  • Otherwise a header providing string_view or StrCat could be copied into this repo. They are unlikely to change or require updates

Reasoning: Bazel as a requirement to install a Python package is a lot and very uncommon. Furthermore working with it especially on HPC systems is hard as it ignores the environment set up there (e.g. loading custom compilers, MPI, etc) leading to failures later. Furthermore it is unclear which version of Bazel is required. Working with TensorFlow I've seen incompatible changes in the now 4 major versions of Bazel. So having the "right" bazel installed is not sure.

Thanks for your consideration

jt0dd commented

still an issue for me

Hey @jt0dd, we have migrated the repo to CMake a few weeks ago, but we still need to do a release for the CMake-based version to become available on PyPI.

acxz commented

Hello, Is there any timeline to make a new release with the cmake changes? I'm sure many people would appreciate a new release instead of having to build off head.

Done in 2cb667a.