mesonbuild / meson-python

Meson PEP 517 Python build backend
https://mesonbuild.com/meson-python/
MIT License
120 stars 59 forks source link

Better error message for editable installs with missing --no-build-isolation #512

Open timhoffm opened 9 months ago

timhoffm commented 9 months ago

Meson needs the --no-build-isolation flag for editable installs. I've stumbled over a missing --no-build-isolation flag multiple times when a project adopted meson but I followed old install instructions.

This results in a quite cryptic stack trace. Could meson detect an editable install with missing --no-build-isolation and issue a more informative warning?

dnicolodi commented 9 months ago

Meson needs the --no-build-isolation flag for editable installs.

This is not strictly true. It is required only when ninja is not provided by the system or the package being installed has build dependencies which are not also runtime dependencies.

This results in a quite cryptic stack trace.

The stack trace you obtain most likely is informing you that ninja could not be executed.

Could meson detect an editable install with missing --no-build-isolation and issue a more informative warning?

Meson definitely cannot do it. meson-python is the component that sits between the build front-end (most likely pip) receiving the --no-build-isolation command line option and the Meson build system. However, meson-python does not have access to the configuration used to run the build front-end. And --no-build-isolation is not required, anyway.

We could issue a warning if we detect that ninja is not a binary provided by the system when building an editable wheel. However, this would not catch the case when ninja is installed on the system but build dependencies are not installed in the Python environment. I'm also not so sure that users would actually pay any attention to the warning.

I think that using build isolation for editable wheels is a misfeature of build front-ends.

rgommers commented 9 months ago

We could issue a warning if we detect that ninja is not a binary provided by the system when building an editable wheel.

That does seem reasonable to me. It's likely (but not certain) that this was user error and rebuilds are going to break.

I'm also not so sure that users would actually pay any attention to the warning.

Perhaps indeed. However, the users here are typically more advanced, and may actually pay attention to such warnings. The warning must be visible without the -v flag to pip though, which may be an issue - the typical output is:

pip install -e .
Obtaining file:///Users/rgommers/code/numpy
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build editable ... done
  Preparing editable metadata (pyproject.toml) ... /

with / the rotating spinner until the build finishes. So if that first build succeeds, all build warnings remain invisible.

I think that using build isolation for editable wheels is a misfeature of build front-ends.

Agreed. It's unlikely to change unfortunately.

dnicolodi commented 9 months ago

The warning must be visible without the -v flag to pip though, which may be an issue

AFAIK, this is not something we can control. I don't think emitting a warning that no one is going to see is very useful.

dnicolodi commented 9 months ago

The only option that remains is to return an error if ninja is not present when building an editable wheel. If ninja is not present the resulting wheel is unusable anyhow, thus it makes sense, IMO.

rgommers commented 9 months ago

That does sound right - and erroring out there does allow us to clearly explain the problem to the user.

timhoffm commented 9 months ago

Thanks for the explanations. In case it helps, I provide the exact case that triggered me opening this issue. In the example in https://github.com/matplotlib/matplotlib/issues/27005 the build runs without error. The stacktrace happens during import of the library at runtime:

n [1]: import matplotlib
---------------------------------------------------------------------------
CalledProcessError                        Traceback (most recent call last)
Cell In[1], line 1
----> 1 import matplotlib

File <frozen importlib._bootstrap>:1176, in _find_and_load(name, import_)

File <frozen importlib._bootstrap>:1138, in _find_and_load_unlocked(name, import_)

File <frozen importlib._bootstrap>:1078, in _find_spec(name, path, target)

File ~/anaconda3/envs/mpl-dev/lib/python3.11/site-packages/_matplotlib_editable_loader.py:271, in MesonpyMetaFinder.find_spec(self, fullname, path, target)
    269     return None
    270 namespace = False
--> 271 tree = self.rebuild()
    272 parts = fullname.split('.')
    274 # look for a package

File ~/anaconda3/envs/mpl-dev/lib/python3.11/site-packages/_matplotlib_editable_loader.py:312, in MesonpyMetaFinder.rebuild(self)
    309 else:
    310     stdout = subprocess.DEVNULL
--> 312 subprocess.run(self._build_cmd, cwd=self._build_path, env=env, stdout=stdout, check=True)
    314 install_plan_path = os.path.join(self._build_path, 'meson-info', 'intro-install_plan.json')
    315 with open(install_plan_path, 'r', encoding='utf8') as f:

File ~/anaconda3/envs/mpl-dev/lib/python3.11/subprocess.py:571, in run(input, capture_output, timeout, check, *popenargs, **kwargs)
    569     retcode = process.poll()
    570     if check and retcode:
--> 571         raise CalledProcessError(retcode, process.args,
    572                                  output=stdout, stderr=stderr)
    573 return CompletedProcess(process.args, retcode, stdout, stderr)

CalledProcessError: Command '['/home/tim/anaconda3/envs/mpl-dev/bin/ninja']' returned non-zero exit status 1.

The mentioned ninja binary exists.

If one cannot detect the/all issues during the build, that's ok. But turning the above runtime exception into a useful message would be a big help.

dnicolodi commented 9 months ago

This is the kind of error that is not possible to detect at wheel build time. The package is failing to compile and the exception is telling you just that: the ninja command returned an error. This may have nothing to do with build isolation. However, we could get the output from ninja and add it to the exception in case of error. I'll look into this.