Debian ruamel.yaml 0.15: Infinite Recoursion On Setup

[ax3l]

Migrated out of #2495 (comment)

Just moving it out not to clutter it further.

I currently get an infinite recursion on a fresh copy of spack develop (ccbff6e) and removed $HOME/.spack directory on . share/spack/setup_env.sh in Debian 9.5 "stretch".

Steps to reproduce the issue

See docker image below. Basically, make sure to have a recent version of ruamel.yaml on your system.

Error Message

Traceback (most recent call last):
  File "/home/axel/src/spack/bin/spack", line 54, in <module>
    sys.exit(spack.main.main())
  File "/home/axel/src/spack/lib/spack/spack/main.py", line 600, in main
    print_setup_info(*args.print_shell_vars.split(','))
  File "/home/axel/src/spack/lib/spack/spack/main.py", line 565, in print_setup_info
    module_roots = spack.config.get('config:module_roots')
  File "/home/axel/src/spack/lib/spack/spack/config.py", line 573, in get
    return config.get(path, default, scope)
  File "/home/axel/src/spack/lib/spack/llnl/util/lang.py", line 556, in __getattr__
    return getattr(self.instance, name)
  File "/home/axel/src/spack/lib/spack/llnl/util/lang.py", line 556, in __getattr__
    return getattr(self.instance, name)
  File "/home/axel/src/spack/lib/spack/llnl/util/lang.py", line 556, in __getattr__
    return getattr(self.instance, name)
  [Previous line repeated 318 more times]
  File "/home/axel/src/spack/lib/spack/llnl/util/lang.py", line 552, in instance
    self._instance = self.factory()
  File "/home/axel/src/spack/lib/spack/spack/config.py", line 547, in _config
    defaults = InternalConfigScope('_builtin', config_defaults)
  File "/home/axel/src/spack/lib/spack/spack/config.py", line 238, in __init__
    _validate_section({section: dsec}, section_schemas[section])
  File "/home/axel/src/spack/lib/spack/spack/config.py", line 611, in _validate_section
    _validate_section.validator(schema).validate(data)
  File "/home/axel/src/spack/lib/spack/external/jsonschema/validators.py", line 67, in __init__
    resolver = RefResolver.from_schema(schema)
  File "/home/axel/src/spack/lib/spack/external/jsonschema/validators.py", line 260, in from_schema
    return cls(schema.get(u"id", u""), schema, *args, **kwargs)
  File "/home/axel/src/spack/lib/spack/external/jsonschema/validators.py", line 245, in __init__
    for id, validator in iteritems(meta_schemas)
  File "/home/axel/src/spack/lib/spack/external/jsonschema/_utils.py", line 20, in __init__
    self.store.update(*args, **kwargs)
  File "/home/axel/src/spack/lib/spack/external/jsonschema/validators.py", line 244, in <genexpr>
    (id, validator.META_SCHEMA)
  File "/home/axel/miniconda3/lib/python3.6/_collections_abc.py", line 744, in __iter__
    yield (key, self._mapping[key])
  File "/home/axel/src/spack/lib/spack/external/jsonschema/_utils.py", line 23, in __getitem__
    return self.store[self.normalize(uri)]
  File "/home/axel/src/spack/lib/spack/external/jsonschema/_utils.py", line 16, in normalize
    return urlsplit(uri).geturl()
  File "/home/axel/src/spack/lib/spack/external/jsonschema/compat.py", line 37, in urlsplit
    scheme, netloc, path, query, fragment = _urlsplit(url)
  File "/home/axel/miniconda3/lib/python3.6/urllib/parse.py", line 398, in urlsplit
    url, scheme, _coerce_result = _coerce_args(url, scheme)
RecursionError: maximum recursion depth exceeded

Information on your system

Python 3.6.4 from Anaconda
ruamel.yaml 0.15.37
Debian 9.5

[ax3l]

@alalazo @healther @scheibelp @tgamblin after bisecting this problem, I found it was introduced in 63004e3 yaml: use ruamel.yaml instead of pyyaml

My system uses:

>> ruamel.yaml.version_info
(0, 15, 37)

[ax3l]

Found a Docker reproducer! :)

Python2:

$ docker run -it debian:stretch
> apt-get update
> apt-get install -y git ca-certificates curl python python-pip procps
> cd /usr/local
> git clone https://github.com/spack/spack.git
> pip install -U ruamel.yaml
> source spack/share/spack/setup-env.sh

Python3:

$ docker run -it debian:stretch
> apt-get update
> apt-get install -y git ca-certificates curl python3 python3-pip procps
> cd /usr/local
> git clone https://github.com/spack/spack.git
> pip3 install -U ruamel.yaml
> ln -s /usr/bin/python3 /usr/bin/python
> source spack/share/spack/setup-env.sh

[tgamblin]

I'm a little puzzled by this. I tried your docker image and I can reproduce the problem, but I don't know why it's not working. ruamel is in Spack's lib/spack/external location, and that path is clearly put at the front of sys.path in the spack script.

I am seeing something kind of like this, where I can't seem to get it to import the ruamel from Spack's vendor directory. I wonder if there are weird settings about the system path on Debian. I'll keep looking.

[alalazo]

@tgamblin Still digging. Right now I can confirm what you say. The code fails here:

spack/lib/spack/llnl/util/lang.py

Line 552 in ff13f39

self._instance = self.factory()

Because of:

>>> import sys
>>> sys.exc_info()
(<class 'AttributeError'>, AttributeError("module 'ruamel.yaml' has no attribute 'Mark'",), <traceback object at 0x7f67edb20e08>)

If I look at the ruamel.yaml module:

>>> import ruamel.yaml
>>> import inspect
>>> inspect.getsourcefile(ruamel.yaml)
'/home/mculpo/venvs/spack/py36/lib/python3.6/site-packages/ruamel/yaml/__init__.py'

I wonder if there are weird settings about the system path on Debian.

I could reproduce on Ubuntu, the trigger seems to be having a site ruamel.yaml package installed.

[ax3l]

I also found this SO but it does not look to me that easy-install.pth was used (at least in the docker above).

sys.path:

['/usr/local/spack/lib/spack/external',
 '/usr/local/spack/lib/spack',
 '/usr/local/spack/bin',
 '/usr/lib/python2.7',
 '/usr/lib/python2.7/plat-x86_64-linux-gnu',
 '/usr/lib/python2.7/lib-tk',
 '/usr/lib/python2.7/lib-old',
 '/usr/lib/python2.7/lib-dynload',
 '/usr/local/lib/python2.7/dist-packages',
 '/usr/lib/python2.7/dist-packages'
]

Also only a single site.py on the system at /usr/lib/python2.7/site.py.

[alalazo]

Fact: when it is installed ruamel.yaml uses a namespace package. What we vendor does not (we have a __init__.py in the ruamel directory). I'll look into the rules for namespace packages + the setup.py of ruamel.yaml, and see if I can find something.

[alalazo]

By the way, the problem can be trimmed down to something very simple:

import inspect
import sys

sys.path.insert(0, '<SpackRoot>/lib/spack/external/')

import ruamel.yaml
print(inspect.getsourcefile(ruamel.yaml))
import six
print(inspect.getsourcefile(six))

Here six will be loaded from the spack/external folder, ruamel.yaml from site-packages.

[alalazo]

😱 I think I got it. Installing ruamel.yaml (via pip for instance) creates this nasty ruamel.yaml-0.15.66-py2.7-nspkg.pth file in site_packages or dist-packages:

import sys, types, os;has_mfs = sys.version_info > (3, 5);p = os.path.join(sys._getframe(1).f_locals['sitedir'], *('ruamel',));importlib = has_mfs and __import__('importlib.util');has_mfs and __import__('importlib.machinery');m = has_mfs and sys.modules.setdefault('ruamel', importlib.util.module_from_spec(importlib.machinery.PathFinder.find_spec('ruamel', [os.path.dirname(p)])));m = m or sys.modules.setdefault('ruamel', types.ModuleType('ruamel'));mp = (m or []) and m.__dict__.setdefault('__path__',[]);(p not in mp) and mp.append(p)

According to the docs (emphasis mine):

A path configuration file is a file whose name has the form name.pth and exists in one of the four directories mentioned above; its contents are additional items (one per line) to be added to sys.path. Non-existing items are never added to sys.path, and no check is made that the item refers to a directory rather than a file. No item is added to sys.path more than once. Blank lines and lines beginning with # are skipped. Lines starting with import (followed by space or tab) are executed.

The creator of ruamel.yaml seems to be exploiting the mechanism in bold characters to execute code that imports ruamel.yaml from site-packages / dist-packages no matter what.

I don't really understand the purpose of that but found a couple of posts here and here. Also, there's a comment here that suggests the deletion of the infamous issue 28, where the intentions were clearly explained?

To end this: it's by design and I see no simple way out (rather than hacking our way through sys.modules).

[alalazo]

To finish the diagnosis:

$ docker run -it debian:stretch
> apt-get update
> apt-get install -y git ca-certificates curl python3 python3-pip procps
> cd /usr/local
> git clone https://github.com/spack/spack.git
> pip3 install -U ruamel.yaml

> rm /usr/local/lib/python2.7/dist-packages/ruamel.yaml-0.15.66-py2.7-nspkg.pth
> rm /usr/local/lib/python2.7/dist-packages/ruamel.ordereddict-0.4.13-py2.7-nspkg.pth

> ln -s /usr/bin/python3 /usr/bin/python
> source spack/share/spack/setup-env.sh

works as expected. Probably it's worth opening an issue upstream saying that the package can't be vendored because of the way it is installed.

[alalazo]

It's worth looking into Python's site package. If we can make python inspect our spack/external folder before site-packages / dist-packages we can try to employ the same .pth trick to import our stuff regardless of what's in site-packages / dist-packages.

[tgamblin]

@alalazo: do you want to do a hot fix to "undo" the sys.modules modification the pth file makes? Seems like that is the only option for us at the moment... I think that and contacting the upstream developers are the best bet...

[alalazo]

@tgamblin I'll give it a shot tomorrow. I think we have 2 possible ways to solve the issue:

1. undo the sys.modules modification
2. try to employ the same trick as ruamel.yaml and put a .pth file in our spack/external folder (and register it with higher priority than site and dist folders)

[healther]

We had a discussion around something similar regarding the installation of python namespace packages (namely backports, but that's just the example). I'll try tomorrow if I can dig it up, I know that at least @adamjstewart @kljohann, myself and probably @scheibelp were involved in this. I don't remember the details anymore but basically it was that a .pth file needs to be created with all backports installations otherwise imports will fail. Maybe there was some logic that can be useful here

[alalazo]

To add on this. A possible path to explore is to rename bin/spack into bin/spack.py (or any other name) and have a bash script that invokes it:

#!/usr/bin/env bash
exec /usr/bin/env python -S $0.py "$@"

This is needed to invoke python with the -S option that, according to docs:

Disable the import of the module site and the site-dependent manipulations of sys.path that it entails.

Because of portability issues, this can't be done simply with:

#!/usr/bin/env python -S

as the shebang argument is "correctly" interpreted on BSD and MacOS, but not on Linux. See here and here for more details.

Going this route though we need to solve other issues, like vendoring setuptools to make spack test work (as reported in #9034) and modify the lines with eval in setup_env.sh:

eval `spack --print-shell-vars sh`

as they seem to print to user stdout with this extra indirection.

[ax3l]

Could we just rename the included dirs to make sure they have a distinct name, such as ruamel_spack/ ?
Or make our usage of ruamel.yaml compatible with newer versions?

[alalazo]

@ax3l Changing name to the module would work, if we are fine with this hack. In any case, the entire Python initialization process seems like an hack, so fine with me 😄

[tgamblin]

Will it work? Would we not also have to change the name ruamel.yaml throughput the package? I’d rather not do that if we do not have to...

[alalazo]

Would we not also have to change the name ruamel.yaml throughput the package?

That's for sure. 63 matches in 17 files.

[alalazo]

For the record: I have a possible solution in #9261 - I just need to make it work correctly with virtualenv and Python 2.7 to pass the tests in Travis.

[alalazo]

Just to state explicitly another solution that is similar to @ax3l proposal, but doesn't require changes in vendored packages: if we had a different directory layout for dependencies, we could have embedded them into a namespace. At that point we would use, for instance, something like:

import spack.externals.six

instead of

import six

in Spack's code. On the plus side we can refer to our vendored packages unambiguously, on the minus side a typo (from six import ... instead of from spack.externals.six import ...) could make us use both the system package and the vendored one in different parts of Spack. As a note: this is how setuptools and other similar packages vendor their dependencies.

[ibaned]

This is an issue for me as well on Ubuntu, and it has been two weeks since fixes were proposed with no action... I would recommend just picking one fix and implementing it, or reverting the change that introduced this bug.

[alalazo]

I would recommend just picking one fix and implementing it, or reverting the change that introduced this bug.

@ibaned If you read the thread you'll see that #9261 has been implemented already, and it's waiting for review. As a temporary solution, you can work-around the issue using a virtualenv without ruamel.yaml installed (in that way Spack will use the one that is vendored).

[alalazo]

@scheibelp @tgamblin (Replying to a comment from Peter in #9261)

If you don't see issues with your suggestion in #9206 (comment), and if you are time-constrained, I could add that to my plate. If You prefer this approach, I'd be interested to hear about the downsides of the alternative.

I'm half-way through having all vendored packages used under the external.<pkg> namespace in a local branch. From the work involved I see a few relevant downsides to this approach:

1. It requires extensive modifications to vendored packages. This is due to two main reasons: we have packages with transitive dependencies (which also need to be moved to the external namespace) and packages that import part of themselves with absolute imports, rather than relative imports. jinja2 is also kind of peculiar, as it generates valid python code that contains imports of the package that probably need to be modified.
2. The modifications to our vendored dependencies to tweak their import statements must be done every time we update them. This opens for more maintenance work once we'll deprecate python 2.6 and could therefore bump the versions of our deps.
3. A typo could make two versions of the same package co-exist with each other. That's what I reported in Debian ruamel.yaml 0.15: Infinite Recoursion On Setup #9206 (comment) at the end.

As a sidenote, while I started with the idea of having everything under the spack.external.<pkg> namespace that was not possible because we have already 2 namespaces at the same level that make use of the vendored dependencies: llnl and spack. This is probably not the most flexible choice to allow changing the file layout in the future.

EDIT: going this route consistently is really cumbersome, but if we accept to use namespace external just for ruamel.yaml the changes are not that many. I'll submit a PR soon.

[alalazo]

I just submitted a PR that tweaks sys.modules (#9725). The downside of that approach is that previous references to ruamel.yaml, if present, will still point to the site module - I don't change the object in sys.modules (as importlib.reload does) but just delete an object from cache and replace it. On the other hand it's the approach which minimizes the lines that have to be changed in core.