-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update python installation method #255
Comments
So I had forgotten about all the other potential stuff in the install dir (blas, tblis, thrust...) I am really not sure what to do about those. Presumably they need to be in a fixed location where when all the CLLR I started a topic on the Python packaging discourse regarding Bokeh, but a lot of the discussion seems relevant here as well: https://discuss.python.org/t/custom-build-steps-moving-bokeh-off-setup-py/16128/3 One immediate thing we can do just to get out from that deprecation warning, is simply to get rid of this: And per some of that discussion, an easy way to do that is probably just to rely on environment variables, rather than custom command line options, to condition the code in But that small step does not get |
Yeah, that seems to work fine, here is a quick and dirty POC diff diff --git a/install.py b/install.py
index aa0c405..d4210cc 100755
--- a/install.py
+++ b/install.py
@@ -537,21 +537,20 @@ def build_legate_core(
f.write(content.encode("utf-8"))
cmd = ["cp", "config.mk", os.path.join(install_dir, "share", "legate")]
verbose_check_call(cmd, cwd=src_dir)
- # Then run setup.py
- cmd = [
- sys.executable,
- "setup.py",
- "install",
- "--recurse",
- ] + setup_py_flags
+ prefix = str(install_dir)
if unknown is not None:
try:
prefix_loc = unknown.index("--prefix")
- cmd.extend(unknown[prefix_loc : prefix_loc + 2])
+ prefix = unknown[prefix_loc + 1]
except ValueError:
- cmd += ["--prefix", str(install_dir)]
- else:
- cmd += ["--prefix", str(install_dir)]
+ pass
+ cmd = [ sys.executable, "-m", "pip", "install", ".", "--prefix", prefix]
+ os.environ["LEGATE_INSTALL_PREFIX"] = prefix
+ print(f"""
+
+ ****************************** {prefix}
+
+ """)
verbose_check_call(cmd, cwd=legate_core_dir)
diff --git a/setup.py b/setup.py
index 5759076..27c6da5 100755
--- a/setup.py
+++ b/setup.py
@@ -15,24 +15,14 @@
# limitations under the License.
#
-import argparse
import os
import subprocess
-import sys
from distutils.command.build_py import build_py
from distutils.core import setup
from setuptools import find_packages
-# We need to know the prefix for the installation
-# so we can know where to get the library
-parser = argparse.ArgumentParser()
-parser.add_argument("--prefix", required=False)
-parser.add_argument(
- "--recurse", required=False, default=False, action="store_true"
-)
-args, _ = parser.parse_known_args()
-
+prefix = os.environ.get("LEGATE_INSTALL_PREFIX", None)
class my_build_py(build_py):
def run(self):
@@ -44,7 +34,7 @@ class my_build_py(build_py):
root_dir = os.path.dirname(os.path.realpath(__file__))
header_src = os.path.join(root_dir, "src", "core", "legate_c.h")
output_dir = os.path.join(root_dir, "legate", "core")
- include_dir = os.path.join(args.prefix, "include")
+ include_dir = os.path.join(prefix, "include")
header = subprocess.check_output(
[
os.getenv("CC", "gcc"),
@@ -55,7 +45,7 @@ class my_build_py(build_py):
header_src,
]
).decode("utf-8")
- libpath = os.path.join(args.prefix, "lib")
+ libpath = os.path.join(prefix, "lib")
with open(os.path.join(output_dir, "install_info.py.in")) as f:
content = f.read()
content = content.format(
@@ -66,20 +56,9 @@ class my_build_py(build_py):
build_py.run(self)
-# If we haven't been called from install.py then do that first
-if args.recurse:
- # Remove the recurse argument from the list
- sys.argv.remove("--recurse")
- setup(
- name="legate.core",
- version="22.03",
- packages=find_packages(
- where=".",
- include=["legate*"],
- ),
- cmdclass={"build_py": my_build_py},
- )
-else:
- with open("install.py") as f:
- code = compile(f.read(), "install.py", "exec")
- exec(code)
+setup(
+ name="legate.core",
+ version="22.03",
+ packages=find_packages(where=".", include=["legate*"]),
+ cmdclass={"build_py": my_build_py},
+)
As an added bonus, since we would never be calling |
I guess another incremental step might be to continue installing all the other shared libraries, etc. under the install dir in the repo, but let the pure python package part install in the "standard" python env. dev38 ❯ python
Python 3.8.13 | packaged by conda-forge | (default, Mar 25 2022, 06:04:10)
[GCC 10.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import legate
>>> legate
<module 'legate' from '/home/bryan/anaconda3/envs/dev38/lib/python3.8/site-packages/legate/__init__.py'> But we would also need to get the verbose_check_call(
["cp", "legate.py", os.path.join(install_dir, "bin", "legate")],
cwd=legate_core_dir,
) |
#198 passes |
@trxcllnt that still appears to be calling Edit: or, just let the python bits get installed into the standard python env, which is I guess the ultimate goal. |
Here is a list of dependencies that we normally don't get from conda:
Overall, we will likely shift to getting most of our dependencies from conda in the medium term, but certain libraries we will still want to build from source (legion, realm and gasnet). I think we could handle these as separate packages, and have a separate install process for each. Legion has a minimal python component (called pygion), that should easily fit within a standard |
OK, back from being out last week. I would propose this as a very rough work plan:
I could imagine a similar path for |
@manopapad This can probably be closed now with the CMake changes? |
I think so. There are still some deprecation warnings printed out, but these might be benign? What do you think @bryevdv ?
|
AFAIK our |
Sounds good, closing |
Legion and legate are using
easy_install
to install the python packages to an arbitrary location. This mode is deprecated and will soon be removed. We should switch to following standard conventions (e.g. pip-based), which install directly into the currently active virtual environment (e.g. the one managed by conda).Here are some relevant messages we get during installation:
One thing I would like to know is how easy it would be to "uninstall" our packages in this mode, in case we want to do a clean build.
Copying from my discussion with @bryevdv on alternatives:
The issue probably comes down to whether "build cmd + package/install cmd" is acceptable or whether you want some one-shot to do everything. One option might be to have cmake build the binary artifacts and install them in the source tree (or somewhere), and then a
pip install .
just does a fairly normal "python package install". Maybe the cmake process even does thepip install .
for you as the last step. If you want things "driven" from python then you still have asetup.py
that invokes the C++ build. Since we can't callsetup.py
as a script anymore (that is deprecated) we'd have to rely on only env vars to control any conditional logic inside it when you callpip install .
or build. There are evidently other tools that support custom steps, like thehatch
tool mentioned in use by Jupyter.CC @trxcllnt for input
The text was updated successfully, but these errors were encountered: