Skip to content

Commit

Permalink
Remove builder from setup.py (#654)
Browse files Browse the repository at this point in the history
The builder module was included in `setup.py` to allow us to ship the
main Redis binaries (not RedisAI) with installs from PyPI. The changes
in this PR remove our ability to do this and requires users to build
Redis as part of the `smart build`. This change in behaviour was
deemed reasonable to allow for easier maintenance and extension
of the Builder class as well as simplify the deployment of wheels.

[ committed by @ashao ]
[ reviewed by @MattToast ]
  • Loading branch information
ashao authored Aug 6, 2024
1 parent 6f6722c commit fde9f2e
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 53 deletions.
6 changes: 6 additions & 0 deletions doc/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ To be released at some future point in time

Description

- Remove build of Redis from setup.py
- Mitigate dependency installation issues
- Fix internal host name representation for Dragon backend
- Make dependencies more discoverable in setup.py
Expand All @@ -28,6 +29,11 @@ Description

Detailed Notes

- The builder module was included in setup.py to allow us to ship the
main Redis binaries (not RedisAI) with installs from PyPI. To
allow easier maintenance of this file and enable future complexity
this has been removed. The Redis binaries will thus be built
by users during the `smart build` step
- Installation of mypy or dragon in separate build actions caused
some dependencies (typing_extensions, numpy) to be upgraded and
caused runtime failures. The build actions were tweaked to include
Expand Down
53 changes: 0 additions & 53 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,6 @@
from pathlib import Path

from setuptools import setup
from setuptools.command.build_py import build_py
from setuptools.command.install import install
from setuptools.dist import Distribution

# Some necessary evils we have to do to be able to use
# the _install tools in smartsim/smartsim/_core/_install
Expand All @@ -95,12 +92,6 @@
buildenv = importlib.util.module_from_spec(buildenv_spec)
buildenv_spec.loader.exec_module(buildenv)

# import builder module
builder_path = _install_dir.joinpath("builder.py")
builder_spec = importlib.util.spec_from_file_location("builder", str(builder_path))
builder = importlib.util.module_from_spec(builder_spec)
builder_spec.loader.exec_module(builder)

# helper classes for building dependencies that are
# also utilized by the Smart CLI
build_env = buildenv.BuildEnv(checks=False)
Expand Down Expand Up @@ -128,47 +119,8 @@
class BuildError(Exception):
pass


# Hacky workaround for solving CI build "purelib" issue
# see https://github.com/google/or-tools/issues/616
class InstallPlatlib(install):
def finalize_options(self):
super().finalize_options()
if self.distribution.has_ext_modules():
self.install_lib = self.install_platlib


class SmartSimBuild(build_py):
def run(self):
database_builder = builder.DatabaseBuilder(
build_env(), build_env.MALLOC, build_env.JOBS
)
if not database_builder.is_built:
database_builder.build_from_git(versions.REDIS_URL, versions.REDIS)

database_builder.cleanup()

# run original build_py command
super().run()


# Tested with wheel v0.29.0
class BinaryDistribution(Distribution):
"""Distribution which always forces a binary package with platform name
We use this because we want to pre-package Redis for certain
platforms to use.
"""

def has_ext_modules(_placeholder):
return True


# Define needed dependencies for the installation

# Add SmartRedis at specific version
# install_requires.append("smartredis>={}".format(versions.SMARTREDIS))

extras_require = {
"dev": [
"black==24.1a1",
Expand Down Expand Up @@ -232,13 +184,8 @@ def has_ext_modules(_placeholder):
"numpy<2",
"smartredis>=0.5,<0.6",
],
cmdclass={
"build_py": SmartSimBuild,
"install": InstallPlatlib,
},
zip_safe=False,
extras_require=extras_require,
distclass=BinaryDistribution,
entry_points={
"console_scripts": [
"smart = smartsim._core._cli.__main__:main",
Expand Down

0 comments on commit fde9f2e

Please sign in to comment.