User-installed bundles interfering with build

[Eric Pettersen]

When doing "make install" from top ChimeraX directory:

/Applications/Xcode.app/Contents/Developer/usr/bin/make build
if [ ! -d "/Users/pett/src/chimerax/build/lib/python3.9/site-packages/chimerax" ]; then mkdir -p /Users/pett/src/chimerax/build/lib/python3.9/site-packages/chimerax; fi
/Applications/Xcode.app/Contents/Developer/usr/bin/make -C add_charge build
PYTHONNOUSERSITE=1 ../../../ChimeraX.app/Contents/bin/python3.9 -I -m chimerax.core --nogui --exit --safemode --cmd "devel build . exit true "
Unknown bundle name 'ChimeraX-MouseModes' listed in Initializations section for bundle SEQCROW
Traceback (most recent call last):

File

"/Users/pett/src/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py",
line 197, in _run_module_as_main

return _run_code(code, main_globals, None,

File

"/Users/pett/src/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py",
line 87, in _run_code

exec(code, run_globals)

File

"/Users/pett/src/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-
packages/chimerax/core/main.py", line 1043, in <module>

exit_code = init(sys.argv)

File

"/Users/pett/src/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-
packages/chimerax/core/main.py", line 642, in init

toolshed.init(sess.logger, debug=sess.debug,

File

"/Users/pett/src/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-
packages/chimerax/core/toolshed/init.py", line 1376, in init

_toolshed = Toolshed(*args, kw)

File

"/Users/pett/src/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-
packages/chimerax/core/toolshed/init.py", line 365, in init

self.reload(logger, check_remote=check_remote,

rebuild_cache=rebuild_cache, _session=session)

File

"/Users/pett/src/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-
packages/chimerax/core/toolshed/init.py", line 424, in reload

self._installed_bundle_info.load(logger, cache_file=cache_file,

File

"/Users/pett/src/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-
packages/chimerax/core/toolshed/installed.py", line 60, in load

self.extend(self._order_bundles(dist_bundle_map, logger))

File

"/Users/pett/src/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-
packages/chimerax/core/toolshed/installed.py", line 248, in _order_bundles

explicit_reverse_order.setdefault(dist_key, set()).add(d.key)

UnboundLocalError: local variable 'dist_key' referenced before assignment

UnboundLocalError: local variable 'dist_key' referenced before assignment

File
"/Users/pett/src/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-
packages/chimerax/core/toolshed/installed.py", line 248, in _order_bundles
explicit_reverse_order.setdefault(dist_key, set()).add(d.key)

_See log for complete Python traceback._

make[4]: * [wheel] Error 1
make[3]: * [add_charge.build] Error 2
make[2]: * [install] Error 2
make[1]: * [bundles.install] Error 2
make: * [install] Error 2

[Greg Couch]

This should be fixed by modifying the devel install command. We should be explicitly using "devel install user false" in our source tree.

[Greg Couch]

Zach, please take a look at this. Also check if PYTHONNOUSERSITE=1 is still needed.

[Zach Pearson]

Adding "user false" to the command does not help, but this error looks like it happens before we even get into bundle builder. Maybe when we're in safemode we shouldn't look at the user's installed bundles or try to reload the bundle cache. What do you think?

[Greg Couch]

Yes, that's a good idea. That could be part of the safemode behavior. As long as we can still install/update "user" bundles, we're good.

[Zach Pearson]

What's happening is that during main, we're initializing a Toolshed instance but not passing it safe_mode until a couple lines later in the call to ts.bootstrap_bundles()

I added safe_mode to Toolshed's initializer, and was able to build add_charge, but during our build process we have BUILD_AND_INSTALL defined, so when we try to install it we run into the next problem: if we use safe_mode as a flag for whether or not to build the bundle cache, then when we go to install a bundle it's not there for ts.find_bundle.

So we can return None if we're in safe_mode, but then there's nothing to set the install timestamp on. And if we guard that behind safemode, we're back to dist_key referenced before assignment when we go to install it except this time the traceback is in toolshed_utils:

Distribution is in ./dist/ChimeraX_AddCharge-1.5.10-py3-none-any.whl
Installing bundle
Executing: toolshed install ./dist/ChimeraX_AddCharge-1.5.10-py3-none-any.whl userOnly false noDeps true
Errors may have occurred when running pip:
pip standard error:
---

[notice] A new release of pip is available: 23.0 -> 23.0.1
[notice] To update, run: /Users/zjp/git/rbvi/ChimeraX/ChimeraX.app/Contents/bin/python3.9 -m pip install --upgrade pip
---
pip standard output:
---
Looking in indexes: https://pypi.org/simple, https://cxtoolshed.rbvi.ucsf.edu/pypi/
Processing ./dist/ChimeraX_AddCharge-1.5.10-py3-none-any.whl
ChimeraX-AddCharge is already installed with the same version as the provided wheel. Use --force-reinstall to force an installation of the wheel.
---
No bundles were installed
Unknown bundle name 'ChimeraX-MouseModes' listed in Initializations section for bundle SEQCROW
Traceback (most recent call last):
  File "/Users/zjp/git/rbvi/ChimeraX/src/bundles/core/src/commands/devel.py", line 174, in _run
    unbound_method(bb, *args, **kw)
  File "/Users/zjp/git/rbvi/ChimeraX/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/chimerax/bundle_builder/bundle_builder.py", line 154, in make_install
    run(session, cmd)
  File "/Users/zjp/git/rbvi/ChimeraX/src/bundles/core/src/commands/run.py", line 38, in run
    results = command.run(text, log=log, return_json=return_json)
  File "/Users/zjp/git/rbvi/ChimeraX/src/bundles/core/src/commands/cli.py", line 2897, in run
    result = ci.function(session, **kw_args)
  File "/Users/zjp/git/rbvi/ChimeraX/src/bundles/core/src/commands/toolshed.py", line 301, in toolshed_install
    ts.install_bundle(bundles, logger, **kw)
  File "/Users/zjp/git/rbvi/ChimeraX/src/bundles/core/src/toolshed/__init__.py", line 919, in install_bundle
    _install_bundle(self, bundle, logger, per_user=per_user, reinstall=reinstall, session=session, no_deps=no_deps)
  File "/Users/zjp/git/rbvi/ChimeraX/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/chimerax/toolshed_utils/__init__.py", line 256, in _install_bundle
    changes = toolshed.reload(logger, rebuild_cache=True, report=True)
  File "/Users/zjp/git/rbvi/ChimeraX/src/bundles/core/src/toolshed/__init__.py", line 426, in reload
    self._installed_bundle_info.load(logger, cache_file=cache_file,
  File "/Users/zjp/git/rbvi/ChimeraX/src/bundles/core/src/toolshed/installed.py", line 60, in load
    self.extend(self._order_bundles(dist_bundle_map, logger))
  File "/Users/zjp/git/rbvi/ChimeraX/src/bundles/core/src/toolshed/installed.py", line 248, in _order_bundles
    explicit_reverse_order.setdefault(dist_key, set()).add(d.key)
UnboundLocalError: local variable 'dist_key' referenced before assignment

It's honestly probably simpler to just temporarily move the user site directory before a build and restore it afterward.

[Zach Pearson]

During a programmer's meeting I said I would investigate whether this was still a problem. It is still a problem.

[Zach Pearson]

I propose this patch which appears to fix things:

diff --git a/src/bundles/core/src/__main__.py b/src/bundles/core/src/__main__.py
index 2fd520add..7b6d80a32 100644
--- a/src/bundles/core/src/__main__.py
+++ b/src/bundles/core/src/__main__.py
@@ -663,7 +663,8 @@ def init(argv, event_loop=True):
         toolshed_url = opts.toolshed
     toolshed.init(sess.logger, debug=sess.debug,
                   check_available=opts.get_available_bundles,
-                  remote_url=toolshed_url, session=sess)
+                  remote_url=toolshed_url, session=sess,
+                  safe_mode = opts.safe_mode)
     sess.toolshed = toolshed.get_toolshed()
     if opts.module != 'pip' and opts.run_path is None:
         # keep bugs in ChimeraX from preventing pip from working
diff --git a/src/bundles/core/src/toolshed/__init__.py b/src/bundles/core/src/toolshed/__init__.py
index 191509818..6fed418c5 100644
--- a/src/bundles/core/src/toolshed/__init__.py
+++ b/src/bundles/core/src/toolshed/__init__.py
@@ -287,7 +287,7 @@ class Toolshed:
     """

     def __init__(self, logger, rebuild_cache=False, check_remote=False,
-                 remote_url=None, check_available=True, session=None):
+                 remote_url=None, check_available=True, session=None, safe_mode = False):
         """Initialize Toolshed instance.

         Parameters
@@ -310,7 +310,7 @@ class Toolshed:
             self.remote_url = _DefaultRemoteURL
         else:
             self.remote_url = remote_url
-        self._safe_mode = None
+        self._safe_mode = safe_mode
         self._repo_locator = None
         self._installed_bundle_info = None
         self._available_bundle_info = None
@@ -368,7 +368,8 @@ class Toolshed:
         if (check_available and self._available_bundle_info is not None and
                                 self._available_bundle_info.toolshed_url != self.remote_url):
             self._available_bundle_info = None
-        self.reload(logger, check_remote=check_remote, rebuild_cache=rebuild_cache, _session=session)
+        if not self._safe_mode:
+            self.reload(logger, check_remote=check_remote, rebuild_cache=rebuild_cache, _session=session)
         from datetime import datetime
         from ..core_settings import settings
         now = datetime.now()
@@ -419,7 +420,6 @@ class Toolshed:
             True to check remote server for updated information;
             False to ignore remote server
         """
-
         _debug("reload", rebuild_cache, check_remote)
         changes = {}
         if reread_cache or rebuild_cache:

[Zach Pearson]

Committed the fix today.

[Zach Pearson]

Instead of simply not reloading in safemode, maybe we should ignore the user directory.

[Eric Pettersen]

Kind of how I thought this would be solved originally. :-)

[Zach Pearson]

Can you review this diff, Greg? Now instead of not reloading the cache, in safe mode I just look at the distribution's site-packages directory to see what's installed in safe mode.

diff --git a/src/bundles/core/src/toolshed/__init__.py b/src/bundles/core/src/toolshed/__init__.py
index b0f91aabc..806264210 100644
--- a/src/bundles/core/src/toolshed/__init__.py
+++ b/src/bundles/core/src/toolshed/__init__.py
@@ -373,8 +373,7 @@ class Toolshed:
         if (check_available and self._available_bundle_info is not None and
                                 self._available_bundle_info.toolshed_url != self.remote_url):
             self._available_bundle_info = None
-        if not self._safe_mode:
-            self.reload(logger, check_remote=check_remote, rebuild_cache=rebuild_cache, _session=session)
+        self.reload(logger, check_remote=check_remote, rebuild_cache=rebuild_cache, _session=session, safe_mode = safe_mode)
         from datetime import datetime
         from ..core_settings import settings
         now = datetime.now()
@@ -411,7 +410,7 @@ class Toolshed:
         _debug("finished loading bundles")

     def reload(self, logger, *, session=None, reread_cache=True, rebuild_cache=False,
-               check_remote=False, report=False, _session=None):
+               check_remote=False, report=False, _session=None, safe_mode = False):
         """Supported API. Discard and reread bundle info.

         Parameters
@@ -435,7 +434,8 @@ class Toolshed:
             cache_file = self._bundle_cache(False, logger)
             self._installed_bundle_info.load(logger, cache_file=cache_file,
                                              rebuild_cache=rebuild_cache,
-                                             write_cache=cache_file is not None)
+                                             write_cache=cache_file is not None,
+                                             safe_mode = safe_mode)
             if report:
                 if save is None:
                     logger.info("Initial installed bundles.")
diff --git a/src/bundles/core/src/toolshed/installed.py b/src/bundles/core/src/toolshed/installed.py
index d96dbe021..1ed6f030c 100644
--- a/src/bundles/core/src/toolshed/installed.py
+++ b/src/bundles/core/src/toolshed/installed.py
@@ -34,11 +34,14 @@ class InstalledBundleCache(list):
         super().__init__(*args, **kw)
         self._help_directories = None

-    def load(self, logger, cache_file=None, rebuild_cache=False, write_cache=True):
+    def load(self, logger, cache_file=None, rebuild_cache=False, write_cache=True, safe_mode=False):
         """Load list of installed bundles.

         Bundles are ordered based on dependency with dependent
-        bundles appearing later in the list."""
+        bundles appearing later in the list.
+
+        safe_mode implies write_cache=False and takes precedence
+        """

         #
         # Load bundle info.  If not rebuild_cache, try reading
@@ -47,10 +50,14 @@ class InstalledBundleCache(list):
         # try to create the cache file.
         #
         _debug("InstalledBundleCache.load: rebuild_cache", rebuild_cache)
-        if cache_file and not rebuild_cache:
-            if self._read_cache(cache_file):
-                _debug("InstalledBundleCache.load: using cached data")
-                return
+        if not safe_mode:
+            # In safe mode, don't rely on the bundle cache. Regenerate the
+            # list of bundles from the bundles installed in the distribution
+            # and nothing more
+            if cache_file and not rebuild_cache:
+                if self._read_cache(cache_file):
+                    _debug("InstalledBundleCache.load: using cached data")
+                    return
         #
         # Okay, no cache.  Go through all installed packages
         # and look for ChimeraX bundles
@@ -58,7 +65,20 @@ class InstalledBundleCache(list):
         _debug("InstalledBundleCache.load: rebuilding cache")
         from pkg_resources import WorkingSet
         dist_bundle_map = {}
-        for d in WorkingSet():
+        working_set = WorkingSet()
+        if safe_mode:
+            # Remove the user site directory from the working set search path
+            # so that we only get bundles installed into the distribution
+            import site
+            dist_site = None
+            for _site in site.getsitepackages():
+                if _site.endswith('site-packages'):
+                    dist_site = _site
+                    break
+            for entry in working_set.entries:
+                if not entry.startswith(dist_site):
+                    working_set.entries.remove(entry)
+        for d in working_set:
             _debug("InstalledBundleCache.load: package %s" % d)
             bi = _make_bundle_info(d, True, logger)
             if bi is not None:

[Zach Pearson]

With this patch I was able to get through an install. I then manually installed SEQCROW after editing its metadata to make it work on 1.8 and did another build. Then it failed, again

[Eric Pettersen]

You know, I think this was milestoned for 1.7 more so it wouldn't fall through the cracks than it absolutely needed to be in 1.7 per se. Maybe we could revert 1.7 to the way it was and put these new changes only on the 1.8 branch?

[Greg Couch]

I'll second that. Re-milestoned for 1.8.

[Zach Pearson]

OK, I'm just going to revert my last patch then because it broke the builds.

[Greg Couch]

Eric said this doesn't have to get fixed for 1.7.

[Greg Couch]

Ticket retargeted after milestone closed

[Greg Couch]

Not going to make 1.10