Skip to content

BLD: remove /usr/include from default include dirs#18658

Merged
charris merged 1 commit intonumpy:mainfrom
rgommers:fix-xlocale
Mar 21, 2021
Merged

BLD: remove /usr/include from default include dirs#18658
charris merged 1 commit intonumpy:mainfrom
rgommers:fix-xlocale

Conversation

@rgommers
Copy link
Copy Markdown
Member

@rgommers rgommers commented Mar 21, 2021 •
edited
Loading

Including this directory is painful for cross-compiling, see gh-14980 and gh-13280. Removing this directory fixes the following build failure:

gcc: numpy/core/src/common/numpyos.c
In file included from numpy/core/src/common/numpyos.c:23:
/usr/include/xlocale.h:27:16: error: redefinition of ‘struct __locale_struct’
   27 | typedef struct __locale_struct

This error also shows up in various other build issues outside of the NumPy issue tracker.

Compilers normally always include this path, so this shouldn't break anything. The default include paths for the compiler can be checked, e.g. for gcc with cpp -v. That will typically have /usr/include last.

In case this breaks something for a nonstandard compiler, that can be worked around via a site.cfg file in the root of the repo (or equivalently, ~/numpy-site.cfg) containing:

[DEFAULT]
include_dirs = /usr/include

The same principle should apply to /usr/lib. The /usr/lib change is made in a separate commit, because the failure mode for that will be different (may be useful in case this breaks something else).

EDIT: to clarify: it is not possible to fix this with an extra ifdef in numpyos.c, the problem is that there's a mix between locale.h and xlocale.h that both exist but are not compatible. For example, a really new locale.h in /usr/include on Arch Linux, combined with an old xlocale.h shipped with conda.

EDIT2: currently it's really easy to reproduce the xlocale.h failure:

@rgommers
BLD: remove /usr/include from default include dirs
6115fc5 
Including this directory is painful for cross-compiling, see
numpygh-14980 and numpygh-13280. Removing this directory fixes the following
build failure:
```
gcc: numpy/core/src/common/numpyos.c
In file included from numpy/core/src/common/numpyos.c:23:
/usr/include/xlocale.h:27:16: error: redefinition of ‘struct __locale_struct’
   27 | typedef struct __locale_struct
```
This error also shows up in various other build issues outside of the
NumPy issue tracker.

Compilers normally always include this path, so this shouldn't break
anything. The default include paths for the compiler can be checked,
e.g. for gcc with `cpp -v`. That will typically have /usr/include last.

In case this breaks something for a nonstandard compiler, that can be
worked around via a site.cfg file in the root of the repo (or
equivalently, `~/numpy-site.cfg`)  containing:
```
[DEFAULT]
include_dirs = /usr/include
```

The same principle should apply to `/usr/lib`. I will make that change
in a separate commit, because the failure mode for that will be
different (and I'm not running into it right now).
@rgommers
Copy link
Copy Markdown
Member Author

rgommers commented Mar 21, 2021

The blas64 job ends with:

  numpy.distutils.system_info.BlasILP64NotFoundError:
      64-bit Blas libraries not found.
      Known libraries in numpy/distutils/site.cfg file are:
      openblas64_, openblas_ilp64

and on Shippable with:

  python tools/openblas_support.py --check_version
Traceback (most recent call last):
  File "tools/openblas_support.py", line 331, in <module>
    test_version(args.check_version)
  File "tools/openblas_support.py", line 303, in test_version
    get_config = dll.openblas_get_config
  File "/usr/lib/python3.7/ctypes/__init__.py", line 369, in __getattr__
    func = self.__getitem__(name)
  File "/usr/lib/python3.7/ctypes/__init__.py", line 374, in __getitem__
    func = self._FuncPtr((name_or_ordinal, self))
AttributeError: /root/venv/3.7/lib/python3.7/site-packages/numpy/core/_multiarray_umath.cpython-37m-aarch64-linux-gnu.so: undefined symbol: openblas_get_config

So that seems to need /usr/lib

@charris
Copy link
Copy Markdown
Member

charris commented Mar 21, 2021

The two github actions failure both use blas64.

@rgommers
Copy link
Copy Markdown
Member Author

rgommers commented Mar 21, 2021

The design of tools/openblas_support.py is a bit clumsy - it unpacks the openblas libraries to a directory that is encoded within the .tar.gz itself. Some debug printing says it ends up in /tmp/openblas/lib/libopenblas64_.a, but somehow removing /usr/lib is problematic.

I'll just remove the second commit, we can deal with /usr/lib later if needed. SciPy also has the same openblas_support.py, and it's not worth the effort to change it right now.

@rgommers rgommers changed the title BLD: remove /usr/include and /usr/lib from default include and library dirs BLD: remove /usr/include from default include dirs Mar 21, 2021
@charris charris merged commit 2bd4836 into numpy:main Mar 21, 2021
@charris
Copy link
Copy Markdown
Member

charris commented Mar 21, 2021

Thanks Ralf.

@rgommers rgommers deleted the fix-xlocale branch March 21, 2021 19:50
rgommers added a commit to rgommers/numpy that referenced this pull request Mar 21, 2021
@rgommers
DOC: add release note for removal of /usr/include from include paths
a2f7cd1 
Follow-up to numpygh-18658
@rgommers
Copy link
Copy Markdown
Member Author

rgommers commented Mar 21, 2021

Thanks Chuck. This needs a release note too, sending a follow-up PR now.

@rgommers rgommers mentioned this pull request Mar 21, 2021
Merged
@QuLogic QuLogic mentioned this pull request Jan 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

36 - Build Build related PR component: numpy.distutils

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rgommers @charris @strawpants