Spack: ROOT + Python/libxml2: Maximum Recursion Depth

[ax3l]

Trying to install root via

$ spack spec root

leads to

Input spec
--------------------------------
root

Concretized
--------------------------------
==> Error: maximum recursion depth exceeded

The problem is gone when using root ~python.

---

Steps to reproduce the issue

$ spack install root

Platform and user environment

Please report your OS here:

$ uname -a 
Linux axel-work 5.0.0-29-generic #31~18.04.1-Ubuntu SMP Thu Sep 12 18:29:21 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
$ lsb_release -d
Description:    Ubuntu 18.04.3 LTS

Additional information

Spack version: develop as of ae6229dee20da04b2120dc97cdfae800c136e42a

Note 1: root +x (default) does not build (seen via root ~xml).
Note 2: root ~x does not build unless I also deactivate ~opengl (default: +opengl).

[ax3l]

The problem is rooted, :trumpet: in

    depends_on('libxml2+python',   when='+xml+python')
    depends_on('libxml2~python',   when='+xml~python')

work-around: root~xml

[scheibelp]

EDIT: I can reproduce this myself with on MacOS and a nearly-clean environment (I can try with a completely clean environment tomorrow).

[chissg]

@scheibelp If it starts to look like a problem with the recipe, or you need help isolating a regression, tag me in.

[chissg]

@adamjstewart @tgamblin @scheibelp Could this be related to root cause of https://github.com/spack/spack/issues/13555#issuecomment-549430024?

[scheibelp]

Nominally I can trace this issue back to https://github.com/spack/spack/pull/10944 (this commit fails and the prior one succeeds). That being said I don't specifically blame that PR: Spack shouldn't fail like this regardless of what constraints exist. I'll be looking into why that is causing an issue today.

[scheibelp]

Of all the changes in that PR, the one that appears to cause this issue is adding gettext as a dependency to Python (I can get root to concretize if I remove that dependency). I need to investigate why that is (and similar to what I mentioned above, I don't consider this the core cause of the issue).

[scheibelp]

There appears to be a cyclic dependency:

libxml2+python -> python -> gettext -> libxml2

root requests libxml2+python by default, which is why https://github.com/spack/spack/issues/13637#issuecomment-551356181 works. Spack should do a better job of reporting this cyclic dependency. Figuring out exactly why it doesn't detect and report this better requires more work.

That being said, overall Spack can't actually resolve the constraints as they are currently specified across the involved packages: one of them is going to have to be more specific (e.g. perhaps Python should explicitly request gettext without libxml2 support).

[chissg]

It would appear that in RHEL-7 and friends, system gettextis compiled _with_ libxml2 support, and system python is compiled without explicit gettext support, but on GNU systems there's a good chance that gettext() is pulled straight from glibc. It would seem to be sensible to follow the same prescription in the python build recipe as the respective systems do. I would note that neither homebrew's (macOS), our own (Fermilab's) build recipe (macOS, RHEL-ish, Ubuntu LTS), nor RedHat's own SPEC file refer to the gettext package at all (although RedHat's python package does provide a gettext.py).

TL;DR: IMO the dependency of python on gettext should be removed in order to avoid this circular dependency. The non-detection of same by Spack's concretizer is a completely different problem that I am glad is not mine 😉.

[chissg]

As a minor point, there is no reason I can think of for the root recipe to require a dependency on libxml2~python when ~python: it should just depend upon libxml2 without qualification, and take what it gets.

[scheibelp]

TL;DR: IMO the dependency of python on gettext should be removed in order to avoid this circular dependency.

(EDIT: PR is planned for Monday 11/11)

I agree. I'll create a PR to remove it. That being said I think I'll also want to investigate it more to understand the details.

It would appear that in RHEL-7 and friends, system gettext is compiled with libxml2 support, and system python is compiled without explicit gettext support, but on GNU systems there's a good chance that gettext() is pulled straight from glibc

10944 added gettext to python on account of https://github.com/spack/spack/pull/10944#discussion_r270608085: the author noticed a configure check in the python package which referred to it. However, the author also noted that they originally added this because they encountered a build failure that they could not reproduce later, so currently I don't think there's evidence that removing the gettext dependency from python will cause problems.

Do you know if Python can be compiled with explicit gettext support? There didn't appear to be a flag to enable/disable it, so it seemed mandatory.

[ax3l]

Usage of libintl.h (gettext) in CPython:
https://github.com/python/cpython/search?q=libintl&unscoped_q=libintl

and warnings in its README on Mac

This almost always means you are trying to build a universal binary for Python and have libraries in /usr/local that don't contain the required architectures. Temporarily move /usr/local aside to finish the build.

configure scripts.

Seams to be used for i18n of CPython itself, e.g. in Modules/_localemodule.c.

[ax3l]

Now hold your horses, gettext can itself also provide a python module(?)/dependency :laughing:
(v0.11.1+)
(But we are not pulling that in our package.)

Update: checking the gettext sources this looks more like an example to me, not a full Python module build.

[scheibelp]

That might mean it could require python as a build dependency, and once we can support multiple instances of packages as build dependencies I don't think there would be an issue with cyclic dependencies. Although for now luckily no one has needed that functionality for gettext in Spack.

It's unfortunate if the only currently-supported mechanism for avoiding libintl warnings is to move /usr/local/. Perhaps long-term (if the temporary issue reported in #10944 crops up again) the solution is to patch the build system to remove the libintl check.

[chissg]

After doing a full build of python with the depends_on('gettext') removed, I can confirm that the only mention of "gettext" in the build log is the provided gettext.py. We do have:

checking libintl.h usability... no
checking libintl.h presence... no
checking for libintl.h... no

but that appears to have no discernible effect on the build.

[ax3l]

Okay, just be aware that gettext provides libintl.h and disabling it might affect internationalization (multi-language) support of the cpython build for some users.

@chissg I guess you build on osx with clang as in the original comment?
In my defaults (%[email protected] arch=linux-ubuntu18.04-skylake), with gettext removed, Python 2.7.15 and 3.8.0 writes:

checking libintl.h usability... yes
checking libintl.h presence... yes
checking for libintl.h... yes

but who knows from where this is picked up, I have one in /usr/include/libintl.h by default from libc6-dev. Maybe we can set a --without-gettext/--without-libintl flag or something to configure to have control (at least on osx).

[scheibelp]

I'll be adding a PR for this shortly but anticipate merging it after 11/25 to help our Spack tutorial at SC run smoothly (removing gettext from python will change the hashes of quite a few packages including those that are installed at the tutorial).

[ax3l]

I just tried a spack spec root on the latest develop and this still fails:

python requires gettext variant ~libxml2, but spec asked for +libxml2

The previously reported work-around root~xml does not work anymore (and root~xml~python fails as well).

cc @scheibelp @becker33 @tgamblin @chissg

[chissg]

@ax3l With a packages.yaml with the following clause for gettext on scientific7:
```.yaml
gettext:
paths:
[email protected]+libunistring+xz+bzip2+libxml2+curses+tar+git: /usr

I get successful executions of:
```Console
spack spec -It root  # Builds latest gettext.
spack spec -It root ^gettext+libunistring  # Builds latest gettext.
spack spec -It root ^python+libxml2 ^gettext+libunistring  # Builds against system gettext.

Without any packages.yaml at all, it is necessary to specify _either_ ^python+libxml2 or ^gettext~libxml2 as their defaults are currently incompatible. It is unclear to me why this is not resolvable, since AFAICT no-one is specifying an incompatible variant, but the default appears to be getting treated as specified at some point.

[ax3l]

Thanks for the work-around!

We will keep this open since this is probably the concretizer not trying hard enough. It's a good test-case for @tgamblin 's upcoming concretizer.

[scheibelp]

@chissg your suggestion in https://github.com/spack/spack/issues/13637#issuecomment-580929596 was the approach of https://github.com/spack/spack/pull/14795, which is now merged.

It is unclear to me why this is not resolvable, since AFAICT no-one is specifying an incompatible variant, but the default appears to be getting treated as specified at some point.

That's part of the current greedy concretization approach: it paints itself into a corner sometimes. I think this can be closed with the temporary fix because the general issue is documented in e.g. https://github.com/spack/spack/issues/5465#issuecomment-332027738

See also: item (1) of https://github.com/spack/spack/wiki/Issue-Disambiguation

[ax3l]

Re-opening with new concretizer issue: spack spec root:

Input spec
--------------------------------
root

Concretized
--------------------------------
==> Error: An unsatisfiable variant constraint has been detected for spec:

    [email protected]%[email protected]~symlinks~termlib arch=linux-ubuntu18.04-skylake
        ^[email protected]%[email protected] arch=linux-ubuntu18.04-skylake


while trying to concretize the partial spec:

    llvm@6:
        ^[email protected]%[email protected]~doc+ncurses+openssl+ownlibs~qt arch=linux-ubuntu18.04-skylake
            ^[email protected]%[email protected]~symlinks~termlib arch=linux-ubuntu18.04-skylake
                ^[email protected]%[email protected] arch=linux-ubuntu18.04-skylake
            ^[email protected]%[email protected]+systemcerts arch=linux-ubuntu18.04-skylake
                ^[email protected]%[email protected]+cpanm+shared+threads arch=linux-ubuntu18.04-skylake
                    ^[email protected]%[email protected] arch=linux-ubuntu18.04-skylake
                        ^[email protected]%[email protected] arch=linux-ubuntu18.04-skylake
                ^[email protected]%[email protected]+optimize+pic+shared arch=linux-ubuntu18.04-skylake
        ^perl-data-dumper


llvm requires ncurses variant +termlib, but spec asked for ~termlib

[scheibelp]

spack spec root ^ncurses+termlib will avoid this issue for now

[glennpj]

@scheibelp We should decide on either PR #14853 or #14862 for llvm.

[adamjstewart]

Has #14862 been submitted upstream? If it's likely to be merged upstream, I would lean in that direction.

[glennpj]

Ha, I was kind of waiting to see if we thought it was a good idea for us (spack) first. I did find the following where it looks like NetBSD did something similar:

https://reviews.llvm.org/D54650

[glennpj]

I just committed an update to #14862 that addresses the remaining issue of potentially picking up a system library depending on whether the system splits tinfo out or not. Essentially, I created a new cmake variable that llvm can use, and that spack can set, to make the library to use for tinfo symbols explicit. That variable will always be first in the search list, ensuring that llvm will pick up the correct library. For building outside of spack the variable could be used for non-standard curses, or left unset to pick up the system libraries.

Unless there is something that I am missing, I will propose that to the llvm project. I feel more comfortable with the latest iteration. I think it covers all cases now.

[alalazo]

This seems to be working (again) with:

• Spack: 0.15.4-1865-61f7625028
• Python: 3.8.5
• Platform: linux-ubuntu18.04-broadwell

Closing for now.