Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meson complains about an absolute include path #16312

Closed
Tracked by #16293
rgommers opened this issue May 29, 2022 · 14 comments · Fixed by #18006
Closed
Tracked by #16293

Meson complains about an absolute include path #16312

rgommers opened this issue May 29, 2022 · 14 comments · Fixed by #18006
Labels
Build issues Issues with building from source, including different choices of architecture, compilers and OS Meson Items related to the introduction of Meson as the new build system for SciPy
Milestone

Comments

@rgommers
Copy link
Member

This is a fairly annoying issue, and I've seen it before but somehow am only now running into it again - probably because I'm working in a Docker container.

Trying to run pip install . --no-build-isolation inside a Docker container on current main results in:

      ../../scipy/meson.build:48:0: ERROR: Tried to form an absolute path to a source dir.
      You should not do that but use relative paths instead.

Outside of a Docker container there's never a complaint - and yes, we are feeding include_directories an absolute path.

find_libraries, which we also use, does require an absolute path. So it looks like we need both; this diff gets us past errors in scipy/meson.build:

diff --git a/scipy/meson.build b/scipy/meson.build
index a1e97b2dd..26f0af439 100644
--- a/scipy/meson.build
+++ b/scipy/meson.build
@@ -37,7 +37,7 @@ if is_windows
 endif
 
 # NumPy include directory - needed in all submodules
-incdir_numpy = run_command(py3,
+incdir_numpy_abs = run_command(py3,
   [
     '-c',
     'import os; os.chdir(".."); import numpy; print(numpy.get_include())'
@@ -45,29 +45,37 @@ incdir_numpy = run_command(py3,
   check: true
 ).stdout().strip()
 
-inc_np = include_directories(incdir_numpy)
+incdir_numpy_rel = run_command(py3,
+  [
+    '-c',
+    'import os; os.chdir(".."); import numpy; print(os.path.join("..", os.path.relpath(numpy.get_include())))'
+  ],
+  check: true
+).stdout().strip()
+
+inc_np = include_directories(incdir_numpy_rel)
 
-incdir_f2py = incdir_numpy / '..' / '..' / 'f2py' / 'src'
+incdir_f2py = incdir_numpy_rel / '..' / '..' / 'f2py' / 'src'
 inc_f2py = include_directories(incdir_f2py)
 fortranobject_c = incdir_f2py / 'fortranobject.c'
 
 cc = meson.get_compiler('c')
-npymath_path = incdir_numpy / '..' / 'lib'
+npymath_path = incdir_numpy_abs / '..' / 'lib'
 npymath_lib = cc.find_library('npymath', dirs: npymath_path)
-npyrandom_path = incdir_numpy / '..' / '..' / 'random' / 'lib'
+npyrandom_path = incdir_numpy_abs / '..' / '..' / 'random' / 'lib'
 # Note: `required: false` can be removed once numpy 1.19 is the minimum version
 npyrandom_lib = cc.find_library('npyrandom', dirs: npyrandom_path, required: false)
 
 # pybind11 include directory - needed in several submodules
-incdir_pybind11 = run_command(py3,
+incdir_pybind11_rel = run_command(py3,
   [
     '-c',
-    'import pybind11; print(pybind11.get_include())'
+    'import os; import pybind11; print(os.path.relpath(pybind11.get_include()))'
   ],
   check: true
 ).stdout().strip()
 
-inc_pybind11 = include_directories(incdir_pybind11)
+inc_pybind11 = include_directories(incdir_pybind11_rel)
 
 # Pythran include directory and build flags
 use_pythran = run_command(py3,
@@ -82,7 +90,7 @@ if use_pythran
   incdir_pythran = run_command(py3,
     [
       '-c',
-      'import os; os.chdir(".."); import pythran; print(os.path.dirname(pythran.__file__));'
+      'import os; os.chdir(".."); import pythran; print(os.path.join("..", os.path.relpath(os.path.dirname(pythran.__file__))));'
     ],
     check: true
   ).stdout().strip()

However, now inc_np is a relative path so it cannot be reused as easily in subdirectories. The next error that pops up is:

      ../../scipy/special/meson.build:320:4: ERROR: File ../env/lib/python3.10/site-packages/numpy/core/include/../../f2py/src/fortranobject.c does not exist.

We'd have to go through every subdir where we use inc_np (and there's 67 usages). An absolute path should be fine (and is nicer to use), and it's not clear to me why it doesn't work inside Docker.

@eli-schwartz any thoughts?

@rgommers rgommers added Build issues Issues with building from source, including different choices of architecture, compilers and OS Meson Items related to the introduction of Meson as the new build system for SciPy labels May 29, 2022
@rgommers rgommers added this to the 1.9.0 milestone May 29, 2022
@rgommers
Copy link
Member Author

Similar bug report (but no follow-up, does say it's happening on Ubuntu, and I was using an Ubuntu Docker container with system Python): mesonbuild/meson#8640

@rgommers
Copy link
Member Author

Ah, the code looks like this:

src_root = self.environment.get_source_dir()

for a in incdir_strings:
    if a.startswith(src_root):
        raise InvalidArguments(textwrap.dedent('''\
            Tried to form an absolute path to a source dir.

So what's happening is that the check goes off only if your include dir is inside your source tree. In this case I was creating a Python virtual environment within the scipy source tree. So the error message is highly misleading.

I think it's actually not good practice to create venv's within the source tree, but it's also common (I don't use venv's or Docker much, so I just copy-pasted a tutorial which is why I ran into it now).

@rgommers
Copy link
Member Author

I have a vague memory of bringing this up before and the initial answer being "don't do that". We need to fix this somehow though, because it's common practice. Here's a few tools that do create venv's inside the local tree:

@ev-br
Copy link
Member

ev-br commented May 29, 2022

rgommers#82 (comment)

@rgommers
Copy link
Member Author

Argh yes, thanks. That increases the importance of making dependency('numpy') work natively in Meson. And until we do that, I guess the answer remains "don't do that".

I'll submit a PR to Meson to clarify the error message though.

@ev-br
Copy link
Member

ev-br commented May 29, 2022

And until we do that, I guess the answer remains "don't do that".

And it will remain the same after that, too.

One thing that an old-fashioned virtualenv package is better than venv, is you create a ~/.virtualenvs directory, set a $WORKON_HOME env var in .bashrc, once. and that's it.

@rgommers
Copy link
Member Author

rgommers commented Jul 5, 2022

Not considering this a blocker for 1.9.0, so bumping the milestone. If someone runs into this when building from source in a venv that's installed inside the source tree: see the four last comments above.

@andyfaff
Copy link
Contributor

andyfaff commented Oct 5, 2022

I think I had this issue in #17151. I've been trying to use meson to build in an ubuntu and a quay.io/pypa/manylinux2014_x86_64 docker image. I get the error reported in #17151, which is a lengthier version of the first comment in this issue. I have trying to build within a venv, and see this error regardless of whether I make the venv inside or outside of the scipy source tree. i.e. if the scipy source tree is in /io/scipy it doesn't matter whether myvenv is in /io/myvenv or /io/scipy/mvenv`, both don't work.

I do not see the error if I try to build without a venv.

@rgommers
Copy link
Member Author

rgommers commented Oct 5, 2022

We do have a CI job specifically testing a venv on Debian: https://github.com/scipy/scipy/blob/main/.github/workflows/linux_meson.yml#L125. So it's a little odd that that works, except when you're in Docker.

So what's happening is that the check goes off only if your include dir is inside your source tree.

I am fairly sure that that is the only thing that triggers this check. It would be helpful to to confirm that by debugging in the Docker container if you can @andyfaff, and checking the paths. The exact line of code is https://github.com/mesonbuild/meson/blob/b8e53ed5ea9c2a3eec19396233e646d4c1f667ae/mesonbuild/interpreter/interpreter.py#L2657.

@astrojuanlu
Copy link
Contributor

We are observing this issue during the PyData NYC mini sprints outside a Docker container. System is a Macbook Pro with macOS Big Sur from @kejsitake. Creating the virtual environment outside the SciPy source directory didn't help.

@rgommers
Copy link
Member Author

rgommers commented Nov 8, 2022

Hmm, very odd and not good. If you are able to post a reproducer for that, I'll investigate. I have a macbook; it's on macOS 12, but it's unlikely that the OS version matters here.

@eli-schwartz
Copy link
Contributor

I think the exact details should be available in builddir/meson-logs/meson-log.txt so if that log file could be provided it would help to debug this.

@astrojuanlu
Copy link
Contributor

After switching to a conda/mamba environment @kejsitake managed to successfully build SciPy. In parallel she made some global software changes with Homebrew so it's unclear whether we will be able to reproduce the issue again or recover the logs, but at least knowing that it was a venv thing might help.

@rgommers rgommers modified the milestones: 1.10.0, 1.11.0 Nov 27, 2022
@rgommers
Copy link
Member Author

This is important, but not as high prio as working on the BLAS/LAPACK support in Meson. We won't get around to this for 1.10.0, so I have bumped the milestone.

lagru added a commit to lagru/scikit-image that referenced this issue Dec 11, 2022
It seems that the combination venv and docker may be a problem for
meson currently [1].

[1] scipy/scipy#16312
rgommers added a commit to rgommers/scipy that referenced this issue Feb 20, 2023
This should make things work with in-tree virtual envs, as well as some
situations with Docker that had issues before.

The approach to determine absolute/relative paths unfortunately has to
be a bit convoluted, but I believe this should work in all
circumstances. A future improvement is still to make this work:
```
np_dep = dependency('numpy')
```
That way it won't be needed to run any Python code for the host
interpreter at build time (this is bad for cross-compiling).

Closes scipygh-16312
rgommers added a commit to rgommers/scipy that referenced this issue Feb 22, 2023
This should make things work with in-tree virtual envs, as well as some
situations with Docker that had issues before.

The approach to determine absolute/relative paths unfortunately has to
be a bit convoluted, but I believe this should work in all
circumstances. A future improvement is still to make this work:
```
np_dep = dependency('numpy')
```
That way it won't be needed to run any Python code for the host
interpreter at build time (this is bad for cross-compiling).

Closes scipygh-16312
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build issues Issues with building from source, including different choices of architecture, compilers and OS Meson Items related to the introduction of Meson as the new build system for SciPy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants