symforce-org / symforce

Fast symbolic computation, code generation, and nonlinear optimization for robotics
https://symforce.org
Apache License 2.0
1.41k stars 145 forks source link

`FileNotFoundError: [Errno 2] No such file or directory: '/usr/local/lib/python*/dist-packages/build/manifest.json'` - `manifest.json` is not copied into the wheel. #336

Closed KOLANICH closed 1 year ago

KOLANICH commented 1 year ago

Describe the bug A clear and concise description of what the bug is.

To Reproduce Steps to reproduce the behavior, e.g.:

  1. python3 -m build -nwx .
  2. unpack the wheel
  3. find ./symforce-0.8.0-*-linux_x86_64 -name manifest.json

Expected behavior The code within the package expects manifest.json be found within the wheel. Also the path it searches the wheel in is incorrect. Seems like currently symforce was not made the way that it can be installed at all!

Environment (please complete the following information):

KOLANICH commented 1 year ago

It turns out this file is not needed at all. It serves the point to provide paths to internal python modules. symengine can be used from packages. .benchmarks should not be a part of the package at all, they take long to compile and bring no benefit. cc_sym.cpython-311-x86_64-linux-gnu.so is already in the location to be discovered by Python authomatically, no manipulation to sys.path is needed. The code dealing with manifest.json should be removed.

KOLANICH commented 1 year ago
From d857074e3bf9f5f046226857734d787611af4091 Mon Sep 17 00:00:00 2001
From: KOLANICH <KOLANICH@users.noreply.github.com>
Date: Mon, 22 May 2023 16:39:00 +0000
Subject: [PATCH] [SymForce] Remove `path_util` usage from `symforce`.

---
 symforce/__init__.py  | 49 +------------------------------------------
 symforce/cc_sym.py    | 11 +---------
 symforce/path_util.py | 19 ++++-------------
 3 files changed, 6 insertions(+), 73 deletions(-)

diff --git a/symforce/__init__.py b/symforce/__init__.py
index ea40ee71..b0b35317 100644
--- a/symforce/__init__.py
+++ b/symforce/__init__.py
@@ -85,54 +85,7 @@ def _find_symengine() -> ModuleType:

     Returns the imported symengine module
     """
-    if "symengine" in sys.modules:
-        return sys.modules["symengine"]
-
-    try:
-        # If symengine is available on python path, use it
-        # TODO(will, aaron): this might not be the version of symengine that we want
-        import symengine
-
-        return symengine
-    except ImportError:
-        pass
-
-    import importlib
-    import importlib.abc
-    import importlib.util
-
-    from . import path_util
-
-    try:
-        symengine_install_dir = path_util.symenginepy_install_dir()
-    except path_util.MissingManifestException as ex:
-        raise ImportError from ex
-
-    symengine_path_candidates = list(
-        symengine_install_dir.glob("lib/python3*/site-packages/symengine/__init__.py")
-    ) + list(symengine_install_dir.glob("local/lib/python3*/dist-packages/symengine/__init__.py"))
-    if len(symengine_path_candidates) != 1:
-        raise ImportError(
-            f"Should be exactly one symengine package, found candidates {symengine_path_candidates} in directory {path_util.symenginepy_install_dir()}"
-        )
-    symengine_path = symengine_path_candidates[0]
-
-    # Import symengine from the directory where we installed it.  See
-    # https://docs.python.org/3/library/importlib.html#importing-a-source-file-directly
-    spec = importlib.util.spec_from_file_location("symengine", symengine_path)
-    assert spec is not None
-    symengine = importlib.util.module_from_spec(spec)
-    sys.modules["symengine"] = symengine
-
-    # For mypy: https://github.com/python/typeshed/issues/2793
-    assert isinstance(spec.loader, importlib.abc.Loader)
-
-    try:
-        spec.loader.exec_module(symengine)
-    except:  # pylint: disable=bare-except
-        # If executing the module fails for any reason, it shouldn't be in `sys.modules`
-        del sys.modules["symengine"]
-        raise
+    import symengine

     return symengine

diff --git a/symforce/cc_sym.py b/symforce/cc_sym.py
index 808e80d8..cabf4c7b 100644
--- a/symforce/cc_sym.py
+++ b/symforce/cc_sym.py
@@ -13,16 +13,7 @@ import sys
 from symforce import logger as _logger
 from symforce import path_util

-try:
-    # If cc_sym is availble on the python path, use it
-    from cc_sym import *  # pylint: disable=wildcard-import
-except ImportError as ex:
-    try:
-        sys.path.append(os.fspath(path_util.cc_sym_install_dir()))
-    except path_util.MissingManifestException as ex2:
-        raise ImportError from ex2
-
-    from cc_sym import *  # pylint: disable=wildcard-import
+from cc_sym import *  # pylint: disable=wildcard-import

 # Set log level, in case the user has set the level in python
 set_log_level(  # pylint: disable=undefined-variable
diff --git a/symforce/path_util.py b/symforce/path_util.py
index 9d36a347..536045dc 100644
--- a/symforce/path_util.py
+++ b/symforce/path_util.py
@@ -11,6 +11,7 @@ from pathlib import Path
 class MissingManifestException(RuntimeError):
     pass

+_thisDir = Path(__file__).parent

 class _Manifest:
     """
@@ -23,7 +24,7 @@ class _Manifest:
     @classmethod
     def _ensure_loaded(cls) -> None:
         if cls._manifest is None:
-            manifest_path = Path(__file__).parent.parent / "build" / "manifest.json"
+            manifest_path = _thisDir / "manifest.json"
             try:
                 with open(manifest_path) as f:
                     cls._manifest = json.load(f)
@@ -44,26 +45,14 @@ class _Manifest:

 def symforce_dir() -> Path:
-    return Path(__file__).parent.parent
-
-
-def symenginepy_install_dir() -> Path:
-    return _Manifest.get_entry("symenginepy_install_dir")
-
-
-def cc_sym_install_dir() -> Path:
-    return _Manifest.get_entry("cc_sym_install_dir")
-
-
-def binary_output_dir() -> Path:
-    return _Manifest.get_entry("binary_output_dir")
+    return _thisDir

 def symforce_root() -> Path:
     """
     The root directory of the symforce project
     """
-    return Path(__file__).parent.parent
+    return _thisDir

 def symforce_data_root() -> Path:
-- 
2.40.1
aaron-skydio commented 1 year ago

manifest.json is required specifically to run SymForce from a build without installing, e.g. to run the tests during development.

For wheel-based installs, manifest.json is unused, and things are supposed to work without it - as you say, in that case symengine and cc_sym are installed so they're findable without the manifest.

The code within the package expects manifest.json be found within the wheel. Also the path it searches the wheel in is incorrect. Seems like currently symforce was not made the way that it can be installed at all!

Is there something specific that you ran into that's broken, i.e. something you're trying to do with an installed copy of SymForce that doesn't work because it's looking for manifest.json?

KOLANICH commented 1 year ago

I assummed that just python3 -m build -nwx . should create a working wheel I can install and just use. It can be that I have not set some env variable when building a wheel. So I just built the wheel and then just installed it with sudo pip3 install --upgrade, and then just tried to use the example from the ReadMe to test if the installation works as intended. And got the exception.

aaron-skydio commented 1 year ago

Ah, yeah what should've happened there is that symengine should've been installed in that case, and it probably wasn't importable (because of something else broken with the wheel), and you saw this exception because we use this as a fallback if it isn't importable otherwise. The actual underlying issue here is I think #330 , and this is just an error message we should clarify