Skip to content

ENH: no output in editable verbose mode when there is no work to do #594

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

Merged
merged 1 commit into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions mesonpy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,7 @@ def build(self, directory: Path, source_dir: pathlib.Path, build_dir: pathlib.Pa
f'{loader_module_name}.py',
read_binary('mesonpy', '_editable.py') + textwrap.dedent(f'''
install(
{self._metadata.name!r},
{self._top_level_modules!r},
{os.fspath(build_dir)!r},
{build_command!r},
Expand Down
36 changes: 27 additions & 9 deletions mesonpy/_editable.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from __future__ import annotations

import ast
import functools
import importlib.abc
import importlib.machinery
Expand Down Expand Up @@ -285,15 +286,16 @@ def find_spec(fullname: str, tree: Node) -> Optional[importlib.machinery.ModuleS


class MesonpyMetaFinder(importlib.abc.MetaPathFinder):
def __init__(self, names: Set[str], path: str, cmd: List[str], verbose: bool = False):
def __init__(self, package: str, names: Set[str], path: str, cmd: List[str], verbose: bool = False):
self._name = package
self._top_level_modules = names
self._build_path = path
self._build_cmd = cmd
self._verbose = verbose
self._loaders: List[Tuple[type, str]] = []

def __repr__(self) -> str:
return f'{self.__class__.__name__}({self._build_path!r})'
return f'{self.__class__.__name__}({self._name!r}, {self._build_path!r})'

def find_spec(
self,
Expand All @@ -308,6 +310,21 @@ def find_spec(
tree = self._rebuild()
return find_spec(fullname, tree)

def _work_to_do(self, env: dict[str, str]) -> bool:
if sys.platform == 'win32':
# On Windows the build command is 'meson compile' eventually with a --ninja-args= option.
if self._build_cmd[-1].startswith('--ninja-args='):
ninja_args = ast.literal_eval(self._build_cmd[-1].split('=', 1)[1]) + ['-n']
dry_run_build_cmd = self._build_cmd[:-1] + [f'--ninja-args={ninja_args!r}']
else:
dry_run_build_cmd = self._build_cmd + ['--ninja-args=-n']
else:
dry_run_build_cmd = self._build_cmd + ['-n']
# Check adapted from
# https://github.com/mesonbuild/meson/blob/a35d4d368a21f4b70afa3195da4d6292a649cb4c/mesonbuild/mtest.py#L1635-L1636
p = subprocess.run(dry_run_build_cmd, cwd=self._build_path, env=env, capture_output=True)
return b'ninja: no work to do.' not in p.stdout and b'samu: nothing to do' not in p.stdout

@functools.lru_cache(maxsize=1)
def _rebuild(self) -> Node:
# skip editable wheel lookup during rebuild: during the build
Expand All @@ -317,12 +334,13 @@ def _rebuild(self) -> Node:
env[MARKER] = os.pathsep.join((env.get(MARKER, ''), self._build_path))

if self._verbose or bool(env.get(VERBOSE, '')):
print('+ ' + ' '.join(self._build_cmd))
stdout = None
# We want to show some output only if there is some work to do
if self._work_to_do(env):
build_command = ' '.join(self._build_cmd)
print(f'meson-python: building {self._name}: {build_command}', flush=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that this line can get very long, with a longer package name plus a long absolute path. I think it's fine though; the absolute path is the least important part here. Only with something like Nix which has hashes in the path this may look a bit off. But let's leave it as is for now and see if anyone notices.

subprocess.run(self._build_cmd, cwd=self._build_path, env=env)
else:
stdout = subprocess.DEVNULL

subprocess.run(self._build_cmd, cwd=self._build_path, env=env, stdout=stdout, check=True)
subprocess.run(self._build_cmd, cwd=self._build_path, env=env, stdout=subprocess.DEVNULL)

install_plan_path = os.path.join(self._build_path, 'meson-info', 'intro-install_plan.json')
with open(install_plan_path, 'r', encoding='utf8') as f:
Expand Down Expand Up @@ -366,7 +384,7 @@ def iter_modules(self, prefix: str) -> Iterator[Tuple[str, bool]]:
yield prefix + modname, False


def install(names: Set[str], path: str, cmd: List[str], verbose: bool) -> None:
finder = MesonpyMetaFinder(names, path, cmd, verbose)
def install(package: str, names: Set[str], path: str, cmd: List[str], verbose: bool) -> None:
finder = MesonpyMetaFinder(package, names, path, cmd, verbose)
sys.meta_path.insert(0, finder)
sys.path_hooks.insert(0, finder._path_hook)
69 changes: 63 additions & 6 deletions tests/test_editable.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@
#
# SPDX-License-Identifier: MIT

import io
import os
import pathlib
import pkgutil
import sys

from contextlib import redirect_stdout

import pytest

import mesonpy
Expand Down Expand Up @@ -66,10 +69,10 @@ def test_mesonpy_meta_finder(package_complex, tmp_path):
mesonpy.Project(package_complex, tmp_path)

# point the meta finder to the build directory
finder = _editable.MesonpyMetaFinder({'complex'}, os.fspath(tmp_path), ['ninja'])
finder = _editable.MesonpyMetaFinder('complex', {'complex'}, os.fspath(tmp_path), ['ninja'])

# check repr
assert repr(finder) == f'MesonpyMetaFinder({str(tmp_path)!r})'
assert repr(finder) == f'MesonpyMetaFinder(\'complex\', {str(tmp_path)!r})'

# verify that we can look up a pure module in the source directory
spec = finder.find_spec('complex')
Expand Down Expand Up @@ -130,7 +133,7 @@ def test_resources(tmp_path):
mesonpy.Project(package_path, tmp_path)

# point the meta finder to the build directory
finder = _editable.MesonpyMetaFinder({'simple'}, os.fspath(tmp_path), ['ninja'])
finder = _editable.MesonpyMetaFinder('simple', {'simple'}, os.fspath(tmp_path), ['ninja'])

# verify that we can look up resources
spec = finder.find_spec('simple')
Expand All @@ -149,7 +152,7 @@ def test_importlib_resources(tmp_path):
mesonpy.Project(package_path, tmp_path)

# point the meta finder to the build directory
finder = _editable.MesonpyMetaFinder({'simple'}, os.fspath(tmp_path), ['ninja'])
finder = _editable.MesonpyMetaFinder('simple', {'simple'}, os.fspath(tmp_path), ['ninja'])

try:
# install the finder in the meta path
Expand Down Expand Up @@ -198,7 +201,7 @@ def test_editable_pkgutils_walk_packages(package_complex, tmp_path):
# build a package in a temporary directory
mesonpy.Project(package_complex, tmp_path)

finder = _editable.MesonpyMetaFinder({'complex'}, os.fspath(tmp_path), ['ninja'])
finder = _editable.MesonpyMetaFinder('complex', {'complex'}, os.fspath(tmp_path), ['ninja'])

try:
# install editable hooks
Expand Down Expand Up @@ -230,10 +233,64 @@ def test_editable_pkgutils_walk_packages(package_complex, tmp_path):

def test_custom_target_install_dir(package_custom_target_dir, tmp_path):
mesonpy.Project(package_custom_target_dir, tmp_path)
finder = _editable.MesonpyMetaFinder({'package'}, os.fspath(tmp_path), ['ninja'])
finder = _editable.MesonpyMetaFinder('package', {'package'}, os.fspath(tmp_path), ['ninja'])
try:
sys.meta_path.insert(0, finder)
import package.generated.one
import package.generated.two # noqa: F401
finally:
del sys.meta_path[0]


@pytest.mark.parametrize('verbose', [False, True], ids=('', 'verbose'))
@pytest.mark.parametrize('args', [[], ['-j1']], ids=('', '-Ccompile-args=-j1'))
def test_editable_rebuild(package_purelib_and_platlib, tmp_path, verbose, args):
with mesonpy._project({'builddir': os.fspath(tmp_path), 'compile-args': args}) as project:

finder = _editable.MesonpyMetaFinder(
project._metadata.name, {'plat', 'pure'},
os.fspath(tmp_path), project._build_command,
verbose=verbose,
)

try:
# Install editable hooks
sys.meta_path.insert(0, finder)

# Import module and trigger rebuild. Importing any module in the
# Python package triggers the build. Use the the pure Python one as
# Cygwin is not happy when reloading an extension module.
stdout = io.StringIO()
with redirect_stdout(stdout):
import pure
assert not verbose or stdout.getvalue().startswith('meson-python: building ')

# Reset state.
del sys.modules['pure']
finder._rebuild.cache_clear()

# Importing again should result in no output.
stdout = io.StringIO()
with redirect_stdout(stdout):
import pure # noqa: F401, F811
assert stdout.getvalue() == ''

finally:
del sys.meta_path[0]
sys.modules.pop('pure', None)


def test_editable_verbose(venv, package_complex, editable_complex, monkeypatch):
monkeypatch.setenv('MESONPY_EDITABLE_VERBOSE', '1')
venv.pip('install', os.fspath(editable_complex))

# Importing the module should not result in any output since the project has already been built
assert venv.python('-c', 'import complex').strip() == ''

# Touch a compiled source file and make sure that the build info is output on import
package_complex.joinpath('test.pyx').touch()
output = venv.python('-c', 'import complex').strip()
assert output.startswith('meson-python: building complex: ')

# Another import without file changes should not show any output
assert venv.python('-c', 'import complex') == ''