gnuradio / pybombs

PyBOMBS (Python Build Overlay Managed Bundle System) is the GNU Radio install management system for resolving dependencies and pulling in out-of-tree projects.
https://gnuradio.org
GNU General Public License v3.0
414 stars 189 forks source link

Pybombs breaks with ruamel.yaml 0.15.56 #517

Closed Bustel closed 6 years ago

Bustel commented 6 years ago

Tried to install the rfnoc dev environment under Ubuntu 18.04 with the following commands

$ pybombs recipes add gr-recipes git+https://github.com/gnuradio/gr-recipes.git
$ pybombs recipes add ettus git+https://github.com/EttusResearch/ettus-pybombs.git
$ pybombs prefix init ~/rfnoc_test -R rfnoc -a rfnoc_test

The last command gave me this error:

pybombs prefix init ~/rfnoc_test -R rfnoc -a rfnoc_test
PyBOMBS.ConfigManager - INFO - Prefix Python version is: 2.7.15
PyBOMBS - INFO - PyBOMBS Version 2.3.3a0
PyBOMBS.prefix - INFO - Creating directory `/home/margot/rfnoc_test'
PyBOMBS.ConfigManager - INFO - Prefix Python version is: 2.7.15
PyBOMBS.ConfigManager - INFO - Creating new config file /home/margot/rfnoc_test/.pybombs/config.yml
PyBOMBS.ConfigManager - INFO - Prefix Python version is: 2.7.15
PyBOMBS.prefix - INFO - Installing default packages for prefix...
PyBOMBS.prefix - INFO - 
  - <ruamel.yaml.comments.CommentedSeq object at 0x7f2cb77c45a0>
  - <ruamel.yaml.comments.CommentedSeq object at 0x7f2cb77c4500>
PyBOMBS.Packager.apt - INFO - Install python-apt to speed up apt processing.
PyBOMBS.install_manager - INFO - Phase 1: Creating install tree and installing binary packages:
PyBOMBS.get_recipe - ERROR - Error fetching recipe `<ruamel.yaml.comments.CommentedSeq object at 0x7f2cb77c45a0>':
Package <ruamel.yaml.comments.CommentedSeq object at 0x7f2cb77c45a0> has no recipe file!

Since I just had successfully installed this under Ubuntu 16.04 I figured this must be connected to a difference in the ruamel.yaml versions between the installations. My current machine has ruamel.yaml version 0.15.56 installed and the working machine has version 0.15.44.

Therefore, I tried downgrading ruamel.yaml to version 0.15.44, which resolved the problem. However I dont think 0.15.56 is the particular issue, since the changelog for version 0.15.55 states:

unmade CommentedSeq a subclass of list. It is now indirectly a subclass of the standard collections.abc.MutableSequence (without .abc if you are still on Python2.7). If you do isinstance(yaml.load('[1, 2]'), list)) anywhere in your code replace list with MutableSequence. Directly, CommentedSeq is a subclass of the abstract baseclass ruamel.yaml.compat.MutableScliceableSequence, with the result that (extended) slicing is supported on CommentedSeq. (reported by Stuart Berg)

I think this might be the cause for the underlying issue but I had no time to verify that.

ido1990 commented 6 years ago

Same here. @Bustel how did you downgrade ruamel.yaml?

Bustel commented 6 years ago

@ido1990 pip lets you install specific versions. The following did the trick for me:

pip uninstall ruamel.yaml
pip install ruamel.yaml==0.15.44
AvdN commented 6 years ago

Is there an easy way for me to test whether my changes broke things and I need to fix ruamel.yaml.

As indicated: CommentedSeq used to be a subclass of list. It now has a list attribute and is dependent on collections.abc.MutableSequence. That latter is an abstract class and I implemented all required classes (like getitem, setitem), but there are some method like .copy() and .sort(), that I did get automagically from list, but not from MutableSequence. I implemented those two, but there might be more missing, which should be very simple once I know what they are. From the error here I have no clue though.

If so you should probably be able to install ruamel.yaml==0.15.54 without hitting this issue.

This might however be related to the earler change in 0.15.52 from CommentedMap being subclass of dict to subclass of collections.abc.MutableMapping

AvdN commented 6 years ago

Based on myinitial investigation I suspects that this might be related to dict_merge` inpybombs/utils/utils.py``:

def dict_merge(a, b):
    """
    Recursively merge b into a. b[k] will overwrite a[k] if it exists.
    """
    if not isinstance(b, dict):
        return b
    result = deepcopy(a)
    for k, v in iteritems(b):
        if k in result and isinstance(result[k], dict):
            result[k] = dict_merge(result[k], v)
        else:
            result[k] = deepcopy(v)
    return result

That isinstance(b, dict looks fishy, and would better be isinstance(b, MutableMapping) (every dict is a MutableMapping, but not every MutableMapping is a dict). You might even use Mapping.

Take note that (Mutable)Mapping comes from collections.abc on Py3 and `collections on Py2

mbr0wn commented 6 years ago

Merged @AvdN's fix.