Opened 7 years ago

Closed 7 years ago

#1288 closed defect (fixed)

Toolshed error

Reported by: Tristan Croll Owned by: Conrad Huang
Priority: major Milestone:
Component: Tool Shed Version:
Keywords: Cc:
Blocked By: Blocking:
Notify when closed: Platform: all
Project: ChimeraX

Description

Right at the end of make app-install, I get the below traceback, leading to a partial installation (lacking compiled libraries). Putting an if/else and a reporting print statement reveals that the offending module is chimerax.core.ui:

Module ModuleSpec(name='chimerax.core.ui', loader=<__main__.init.<locals>.CoreCompatFinder.find_spec.<locals>.FakeLoader object at 0x7f5068d1d6a0>, origin='/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/ui/__init__.py') returns None for submodule_search_locations

traceback:

adding 'ChimeraX_Clipper-0.2.1.dist-info/RECORD'
removing build/bdist.linux-x86_64/wheel
Distribution is in ./dist/ChimeraX_Clipper-0.2.1-cp36-cp36m-linux_x86_64.whl
Error processing trigger "bundle installed"
Traceback (most recent call last):
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/triggerset.py", line 126, in invoke
    return self._func(self._name, data)
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/help_viewer/__init__.py", line 40, in _update_cache
    help_directories = toolshed.get_help_directories()
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/toolshed/__init__.py", line 1308, in get_help_directories
    hd.extend(_toolshed._installed_bundle_info.help_directories)
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/toolshed/installed.py", line 209, in help_directories
    help_dir = bi.get_path('docs')
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/toolshed/info.py", line 528, in get_path
    p = self._bundle_path(subpath)
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/toolshed/info.py", line 455, in _bundle_path
    for d in s.submodule_search_locations:
TypeError: 'NoneType' object is not iterable
Executing: toolshed install './dist/ChimeraX_Clipper-0.2.1-cp36-cp36m-linux_x86_64.whl'
unloading module chimerax.clipper
Successfully installed ChimeraX-Clipper-0.2.1
Installed ChimeraX-Clipper (0.2.1)

Attachments (1)

bundle_info.xml (16.4 KB ) - added by tic20@… 7 years ago.
Added by email2trac

Download all attachments as: .zip

Change History (15)

comment:1 by Tristan Croll, 7 years ago

Actually, scratch the bit about compiled libraries. Even after bypassing this issue, I'm still not getting libraries compiled by _CLibrary installed (only _CModule).

comment:2 by Tristan Croll, 7 years ago

The _CLibrary compiled libraries not being installed because make_wheel doesn't push the new files into setup_arguments (which are set by _make_setup_arguments() during init()). The below amendment to make_wheel() is one way to address the issue:

@distlib_hack
def make_wheel(self, test=True, debug=False):

# HACK: distutils uses a cache to track created directories
# for a single setup() run. We want to run setup() multiple
# times which can remove/create the same directories.
# So we need to flush the cache before each run.
import distutils.dir_util
try:

distutils.dir_util._path_created.clear()

except AttributeError:

pass

import os.path
from collections import defaultdict
extra_libs = defaultdict(list)
for lib in self.c_libraries:

lib_path = lib.compile(self.logger, self.dependencies, debug=debug)
extra_libs[self.package].append(("file", lib_path))

self.setup_argumentspackage_data.update(self._expand_datafiles(extra_libs))
setup_args = ["--no-user-cfg", "build"]
if debug:

setup_args.append("--debug")

if test:

setup_args.append("test")

setup_args.extend(bdist_wheel)
built = self._run_setup(setup_args)
if not built or not os.path.exists(self.wheel_path):

raise RuntimeError("Building wheel failed")

else:

print("Distribution is in %s" % self.wheel_path)

comment:3 by Tristan Croll, 7 years ago

Aaah! Sorry, that attempt was wiping out all pre-existing DataFile entries, and *only* installing the compiled libs. Try this:

    @distlib_hack
    def make_wheel(self, test=True, debug=False):
        # HACK: distutils uses a cache to track created directories
        # for a single setup() run.  We want to run setup() multiple
        # times which can remove/create the same directories.
        # So we need to flush the cache before each run.
        import distutils.dir_util
        try:
            distutils.dir_util._path_created.clear()
        except AttributeError:
            pass
        import os.path
        from collections import defaultdict
        extra_libs = defaultdict(list)
        for lib in self.c_libraries:
            lib_path = lib.compile(self.logger, self.dependencies, debug=debug)
            extra_libs[self.package].append(("file", lib_path))
        elib_dict = self._expand_datafiles(extra_libs)
        for package, libs in elib_dict.items():
            self.setup_arguments["package_data"][package].extend(libs)
        setup_args = ["--no-user-cfg", "build"]
        if debug:
            setup_args.append("--debug")
        if test:
            setup_args.append("test")
        setup_args.extend(["bdist_wheel"])
        built = self._run_setup(setup_args)
        if not built or not os.path.exists(self.wheel_path):
            raise RuntimeError("Building wheel failed")
        else:
            print("Distribution is in %s" % self.wheel_path)


comment:4 by Conrad Huang, 7 years ago

Hi, Tristan.

I see the problem. The code fixing up the self.datafiles is getting called too late. I think the code adding CLibrary files should go into _make_setup_arguments() instead of _make_wheel() since we are setting self.setup_arguments.

Can you send me the source code that demonstrates the problem? Thanks.

Conrad

in reply to:  5 ; comment:5 by tic20@…, 7 years ago

As in: my source tree for the Clipper plugin? I can, but it's somewhat 
on the large size these days and needs pybind11 (including a small bug 
fix) to compile. I've attached a copy of my bundle_info.xml to give you 
an idea. I don't really have a *small* example at the moment.

On 2018-09-11 18:31, ChimeraX wrote:

bundle_info.xml

by tic20@…, 7 years ago

Attachment: bundle_info.xml added

Added by email2trac

comment:6 by Conrad Huang, 7 years ago

Thanks, Tristan.

As I suspected, I didn't run into this bug because I had explicitly listed the shared objects in the DataFiles section (which you obviously should not have to do). I've updated the code to add CLibrary output to DataFiles automatically and it should be in tomorrow's daily build. Can you please give it a try when you have a chance? Thanks.

Conrad

comment:7 by Tristan Croll, 7 years ago

(Error encountered while testing the new bundle_builder.py): Clang crashes when it hits a C source file in a _CLibrary module. The below modification to _CLibrary.compile() (I inserted it at line 701) takes care of it:

        # We need to manually separate out C from C++ code here, since clang
        # crashes if -std=c++11 is given as a switch while compiling C code
        c_files = []
        cpp_files = []
        for f in self.source_files:
            l = compiler.detect_language(f)
            if l == 'c':
                c_files.append(f)
            elif l == 'c++':
                cpp_files.append(f)
            else:
                raise RuntimeError('Unsupported language for {}'.format(f))

        compiler.compile(cpp_files, extra_preargs=cpp_flags,
                         macros=macros, debug=debug)
        compiler.compile(c_files, extra_preargs=[],
                         macros=macros, debug=debug)

comment:8 by Tristan Croll, 7 years ago

Change needed in _CLibrary for extra_link_args on the Mac:

        else:
            if sys.platform == "darwin":
                # On Mac, we only need the .dylib and it MUST be compiled
                # with "-dynamiclib", not "-bundle".  Hence the giant hack:
                try:
                    n = compiler.linker_so.index("-bundle")
                except ValueError:
                    pass
                else:
                    compiler.linker_so[n] = "-dynamiclib"
                lib = compiler.library_filename(lib_name, lib_type="dylib")
<              extra_link_args.append("-Wl,-install_name,@rpath/%s" % lib)
>              extra_link_args.append("-Wl,-rpath,@loader_path -Wl,-install_name,@rpath/%s" % lib)
                compiler.link_shared_object(objs, lib, output_dir=output_dir,
                                            extra_preargs=extra_link_args,
                                            debug=debug)

comment:9 by Tristan Croll, 7 years ago

The above changes get me compiling successfully on the Mac with today's daily build - but I'm still not getting the _CLibrary results installed. The original bug from the start of the thread is there too - have your changes made it through to the builds?

comment:10 by Tristan Croll, 7 years ago

One other issue (that I don't think was happening before) is that _CLibrary (but not _CModule) appears to compile from scratch every time, regardless of whether object files already exist.

comment:11 by Conrad Huang, 7 years ago

I've added your changes to the source and will commit it before tonight's build.

It's odd that your libraries are not getting installed because the Atom_Search bundle uses the same mechanism. What does version verbose report?

And the _CLibrary code is really not very smart. It ought to check for whether the library is up to date, but that's not easy (e.g., if an included header file changes). So I just rebuild every time right now. I'll look into setuptools/distlib to see if there is some existing code that does the checking.

in reply to:  13 comment:12 by tic20@…, 7 years ago

Working in Linux today (apart from the error in info.py). Haven't had a 
chance to test the Mac build.

On 2018-09-13 23:41, ChimeraX wrote:

comment:13 by Tristan Croll, 7 years ago

I'm still getting the error on for d in s.submodule_search_locations with the latest nightly build. Grepping for mentions of submodule_search_locations throughout the Python tree indicates that None is a valid value (e.g. importlib/bootstrap.py has if spec.submodule_search_locations is not None:). I'd suggest modifying toolshed/info.py at line 455:

        if s.submodule_search_locations:
            for d in s.submodule_search_locations:
                p = os.path.join(d, filename)
                if os.path.exists(p):
                    return p
        return None

comment:14 by Conrad Huang, 7 years ago

Resolution: fixed
Status: assignedclosed

Added fix in f4d609e06.

Note: See TracTickets for help on using tickets.