diff --git a/CHANGES/577.packaging.rst b/CHANGES/577.packaging.rst new file mode 100644 index 00000000..aecc0c39 --- /dev/null +++ b/CHANGES/577.packaging.rst @@ -0,0 +1,23 @@ +The :pep:`517` build backend now supports a new ``build-inplace`` +config setting (and ``FROZENLIST_BUILD_INPLACE`` environment variable) +for controlling whether to build the project in-tree or in a +temporary directory. It only affects wheels and is set up to build +in a temporary directory by default. It does not affect editable +wheel builds; they will keep being built in-tree regardless. + +Here's an example of using this setting: + +.. code-block:: console + + $ python -m build --config-setting=build-inplace=true + +Additionally, when building wheels in an automatically created +temporary directory, the build backend now normalizes the +respective file system path to a deterministic source checkout +directory by injecting the ``-ffile-prefix-map`` compiler option +into the ``CFLAGS`` environment variable, as suggested by known +`reproducible build practices +`__. + +The effect is that downstreams will get reproducible build results +-- by :user:`bdraco`. diff --git a/packaging/pep517_backend/_backend.py b/packaging/pep517_backend/_backend.py index 1e5cbfcc..7bfd6dc4 100644 --- a/packaging/pep517_backend/_backend.py +++ b/packaging/pep517_backend/_backend.py @@ -79,6 +79,12 @@ PURE_PYTHON_ENV_VAR = 'FROZENLIST_NO_EXTENSIONS' """Environment variable name toggle used to opt out of making C-exts.""" +BUILD_INPLACE_CONFIG_SETTING = 'build-inplace' +"""Config setting name toggle for building C-exts in-place.""" + +BUILD_INPLACE_ENV_VAR = 'FROZENLIST_BUILD_INPLACE' +"""Environment variable name toggle for building C-exts in-place.""" + IS_CPYTHON = _system_implementation.name == "cpython" """A flag meaning that the current interpreter implementation is CPython.""" @@ -134,6 +140,19 @@ def _include_cython_line_tracing( ) +def _build_inplace( + config_settings: _ConfigDict | None = None, + *, + default: bool = False, +) -> bool: + return _get_setting_value( + config_settings, + BUILD_INPLACE_CONFIG_SETTING, + BUILD_INPLACE_ENV_VAR, + default=default, + ) + + @contextmanager def patched_distutils_cmd_install() -> Iterator[None]: """Make `install_lib` of `install` cmd always use `platlib`. @@ -216,7 +235,7 @@ def _exclude_dir_path( @contextmanager -def _in_temporary_directory(src_dir: Path) -> Iterator[None]: +def _in_temporary_directory(src_dir: Path) -> Iterator[Path]: with TemporaryDirectory(prefix='.tmp-frozenlist-pep517-') as tmp_dir: tmp_dir_path = Path(tmp_dir) root_tmp_dir_path = tmp_dir_path.parent @@ -231,7 +250,7 @@ def _in_temporary_directory(src_dir: Path) -> Iterator[None]: symlinks=True, ) os.chdir(tmp_src_dir) - yield + yield tmp_src_dir @contextmanager @@ -284,15 +303,21 @@ def maybe_prebuild_c_extensions( stacklevel=999, ) + original_src_dir = Path.cwd().resolve() build_dir_ctx = ( nullcontext() if build_inplace - else _in_temporary_directory(src_dir=Path.cwd().resolve()) + else _in_temporary_directory(src_dir=original_src_dir) ) - with build_dir_ctx: + with build_dir_ctx as tmp_build_dir: config = _get_local_cython_config() cythonize_args = _make_cythonize_cli_args_from_config(config, cython_line_tracing_requested) - with _patched_cython_env(config['env'], cython_line_tracing_requested): + with _patched_cython_env( + config['env'], + cython_line_tracing_requested, + original_source_directory=original_src_dir, + temporary_build_directory=tmp_build_dir, + ): _cythonize_cli_cmd(cythonize_args) with patched_distutils_cmd_install(): with patched_dist_has_ext_modules(): @@ -316,7 +341,7 @@ def build_wheel( """ with maybe_prebuild_c_extensions( line_trace_cython_when_unset=False, - build_inplace=False, + build_inplace=_build_inplace(config_settings, default=False), config_settings=config_settings, ): return _setuptools_build_wheel( @@ -341,9 +366,17 @@ def build_editable( :param metadata_directory: :file:`.dist-info` directory path. """ + mandatory_build_inplace = True + if not _build_inplace(config_settings, default=mandatory_build_inplace): + _warn_that( + 'Editable builds require C-extensions to be produced in-tree', + RuntimeWarning, + stacklevel=999, + ) + with maybe_prebuild_c_extensions( line_trace_cython_when_unset=True, - build_inplace=True, + build_inplace=mandatory_build_inplace, config_settings=config_settings, ): return _setuptools_build_editable( diff --git a/packaging/pep517_backend/_cython_configuration.py b/packaging/pep517_backend/_cython_configuration.py index f58d52b3..de0ba3e2 100644 --- a/packaging/pep517_backend/_cython_configuration.py +++ b/packaging/pep517_backend/_cython_configuration.py @@ -3,6 +3,7 @@ from __future__ import annotations import os +import sys from collections.abc import Iterator from contextlib import contextmanager from pathlib import Path @@ -103,7 +104,13 @@ def make_cythonize_cli_args_from_config(config: Config, cython_line_tracing_requ @contextmanager -def patched_env(env: dict[str, str], cython_line_tracing_requested: bool) -> Iterator[None]: +def patched_env( + env: dict[str, str], + cython_line_tracing_requested: bool, + *, + original_source_directory: Path | None = None, + temporary_build_directory: Path | None = None, +) -> Iterator[None]: """Temporary set given env vars. :param env: tmp env vars to set @@ -111,15 +118,49 @@ def patched_env(env: dict[str, str], cython_line_tracing_requested: bool) -> Ite :yields: None """ + extra_cflags: list[str] = [] + if cython_line_tracing_requested: + extra_cflags.append('-DCYTHON_TRACE_NOGIL=1') # Implies CYTHON_TRACE=1 + # When building in a temporary directory, rewrite the random tmp dir + # path back to the original source directory so the compiled artifacts + # are reproducible. `-ffile-prefix-map` is a GCC/Clang flag and is not + # understood by MSVC, so skip it on Windows. Validation runs before we + # touch ``os.environ`` so a configuration error cannot leave the process + # environment half-mutated. + # Ref: https://github.com/aio-libs/frozenlist/issues/577 + if temporary_build_directory is not None and sys.platform != 'win32': + if original_source_directory is None: + raise ValueError( + 'original_source_directory is required ' + 'when temporary_build_directory is set', + ) + tmp_path = str(temporary_build_directory) + src_path = str(original_source_directory) + # `CFLAGS`/`CXXFLAGS` are split on whitespace by the compiler driver, + # so a build path containing a space would silently tokenise into + # multiple arguments and break the mapping. Fail loudly instead of + # producing a broken-but-quiet reproducibility result. + if ' ' in tmp_path or ' ' in src_path: + raise ValueError( + 'Build paths must not contain whitespace for ' + '`-ffile-prefix-map` to apply cleanly; got ' + f'temporary_build_directory={tmp_path!r}, ' + f'original_source_directory={src_path!r}', + ) + extra_cflags.append(f'-ffile-prefix-map={tmp_path}={src_path}') + orig_env = os.environ.copy() expanded_env = {name: expandvars(var_val) for name, var_val in env.items()} os.environ.update(expanded_env) - - if cython_line_tracing_requested: - os.environ['CFLAGS'] = ' '.join(( - os.getenv('CFLAGS', ''), - '-DCYTHON_TRACE_NOGIL=1', # Implies CYTHON_TRACE=1 - )).strip() + if extra_cflags: + # The Cython extension compiles as C++ (``# distutils: language = c++``), + # so setuptools' ``customize_compiler`` uses ``CXXFLAGS`` rather than + # ``CFLAGS`` for the compile step. Set both so the flags also apply + # when downstream forks switch the language. + for env_var in ('CFLAGS', 'CXXFLAGS'): + os.environ[env_var] = ' '.join( + (os.getenv(env_var, ''), *extra_cflags), + ).strip() try: yield finally: diff --git a/pytest.ini b/pytest.ini index d6b39673..365b28ff 100644 --- a/pytest.ini +++ b/pytest.ini @@ -63,6 +63,11 @@ markers = minversion = 3.8.2 +# Make the in-tree PEP 517 backend importable for tests without a per-test +# ``sys.path`` hack. ``testpaths = tests/`` keeps ``--doctest-modules`` from +# collecting modules under ``packaging/``. +pythonpath = packaging + # Optimize pytest's lookup by restricting potentially deep dir tree scan: norecursedirs = build diff --git a/tests/conftest.py b/tests/conftest.py deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/test_pep517_backend.py b/tests/test_pep517_backend.py new file mode 100644 index 00000000..3deb5e0a --- /dev/null +++ b/tests/test_pep517_backend.py @@ -0,0 +1,372 @@ +"""Tests for the in-tree PEP 517 build backend. + +These exercise the bits of ``packaging/pep517_backend/`` that drive the +reproducibility behaviour added for issue #577: the ``build-inplace`` +config-setting / ``FROZENLIST_BUILD_INPLACE`` env-var precedence ladder, the +``-ffile-prefix-map`` injection into ``CFLAGS`` / ``CXXFLAGS`` from +``patched_env``, and the way ``maybe_prebuild_c_extensions`` / +``build_editable`` thread those paths through to the Cython env hook. +""" + +from __future__ import annotations + +import os +import sys +from collections.abc import Iterator +from contextlib import contextmanager +from pathlib import Path + +import pytest + +# The build backend transitively imports ``expandvars``, which is a build-time +# dependency declared in ``pyproject.toml`` rather than a test requirement. CI +# only installs it on the non-``no-extensions`` matrix cells where the wheel +# is built; on the ``FROZENLIST_NO_EXTENSIONS=Y`` cells it is absent, so skip +# this module there instead of failing to collect. +pytest.importorskip("expandvars") + +# ``pytest.ini`` sets ``pythonpath = packaging`` so the in-tree PEP 517 backend +# under ``packaging/`` becomes importable for these tests. +from pep517_backend import _backend # noqa: E402 +from pep517_backend._backend import ( # noqa: E402 + BUILD_INPLACE_CONFIG_SETTING, + BUILD_INPLACE_ENV_VAR, + CYTHON_TRACING_ENV_VAR, + PURE_PYTHON_ENV_VAR, + _build_inplace, + build_editable, + maybe_prebuild_c_extensions, +) +from pep517_backend._cython_configuration import patched_env # noqa: E402 + + +@pytest.fixture +def clean_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Strip env vars that could leak into the build-inplace lookup. + + ``FROZENLIST_NO_EXTENSIONS`` is wiped because CI exports it (set to the + matrix's ``no-extensions`` value, which may be the empty string) when + running the ``Test`` job; the backend's ``_is_truthy_setting_value`` + treats an empty string as truthy and would otherwise force + ``maybe_prebuild_c_extensions`` into the pure-Python early-return, + skipping the stub-calling code these tests exercise. + """ + monkeypatch.delenv(BUILD_INPLACE_ENV_VAR, raising=False) + monkeypatch.delenv(PURE_PYTHON_ENV_VAR, raising=False) + monkeypatch.delenv(CYTHON_TRACING_ENV_VAR, raising=False) + monkeypatch.delenv("CFLAGS", raising=False) + monkeypatch.delenv("CXXFLAGS", raising=False) + + +def test_build_inplace_default_false(clean_env: None) -> None: + assert _build_inplace() is False + + +def test_build_inplace_default_true(clean_env: None) -> None: + assert _build_inplace(default=True) is True + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + ("true", True), + ("1", True), + ("on", True), + ("", True), + ("false", False), + ("0", False), + ("off", False), + ], +) +def test_build_inplace_config_setting( + clean_env: None, + value: str, + expected: bool, +) -> None: + assert ( + _build_inplace( + {BUILD_INPLACE_CONFIG_SETTING: value}, + ) + is expected + ) + + +def test_build_inplace_env_var( + clean_env: None, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv(BUILD_INPLACE_ENV_VAR, "true") + assert _build_inplace() is True + + +def test_build_inplace_config_setting_beats_env_var( + clean_env: None, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv(BUILD_INPLACE_ENV_VAR, "true") + assert ( + _build_inplace( + {BUILD_INPLACE_CONFIG_SETTING: "false"}, + ) + is False + ) + + +def test_patched_env_injects_flag_when_tmp_dir_set( + clean_env: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + monkeypatch.setattr(sys, "platform", "linux") + src_dir = tmp_path / "src" + build_dir = tmp_path / "build" + src_dir.mkdir() + build_dir.mkdir() + expected = f"-ffile-prefix-map={build_dir!s}={src_dir!s}" + + with patched_env( + env={}, + cython_line_tracing_requested=False, + original_source_directory=src_dir, + temporary_build_directory=build_dir, + ): + assert expected in os.environ["CFLAGS"].split() + assert expected in os.environ["CXXFLAGS"].split() + + +def test_patched_env_skipped_when_no_tmp_dir( + clean_env: None, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(sys, "platform", "linux") + + with patched_env( + env={}, + cython_line_tracing_requested=False, + ): + assert "CFLAGS" not in os.environ + assert "CXXFLAGS" not in os.environ + + +def test_patched_env_skipped_on_windows( + clean_env: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + monkeypatch.setattr(sys, "platform", "win32") + + with patched_env( + env={}, + cython_line_tracing_requested=False, + original_source_directory=tmp_path / "src", + temporary_build_directory=tmp_path / "build", + ): + assert "CFLAGS" not in os.environ + assert "CXXFLAGS" not in os.environ + + +def test_patched_env_line_tracing_still_applied_on_windows( + clean_env: None, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(sys, "platform", "win32") + + with patched_env( + env={}, + cython_line_tracing_requested=True, + ): + assert "-DCYTHON_TRACE_NOGIL=1" in os.environ["CFLAGS"].split() + assert "-DCYTHON_TRACE_NOGIL=1" in os.environ["CXXFLAGS"].split() + + +def test_patched_env_raises_when_source_dir_missing( + clean_env: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + monkeypatch.setattr(sys, "platform", "linux") + with pytest.raises(ValueError, match="original_source_directory"): + with patched_env( + env={}, + cython_line_tracing_requested=False, + original_source_directory=None, + temporary_build_directory=tmp_path / "build", + ): + pass + + +@pytest.mark.parametrize( + ("src_name", "build_name"), + [ + ("src with space", "build"), + ("src", "build with space"), + ], +) +def test_patched_env_raises_on_whitespace_in_paths( + clean_env: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + src_name: str, + build_name: str, +) -> None: + monkeypatch.setattr(sys, "platform", "linux") + src_dir = tmp_path / src_name + build_dir = tmp_path / build_name + with pytest.raises(ValueError, match="whitespace"): + with patched_env( + env={}, + cython_line_tracing_requested=False, + original_source_directory=src_dir, + temporary_build_directory=build_dir, + ): + pass + + +def test_patched_env_appends_to_existing_cflags( + clean_env: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + monkeypatch.setattr(sys, "platform", "linux") + monkeypatch.setenv("CFLAGS", "-O2") + monkeypatch.setenv("CXXFLAGS", "-O3") + src_dir = tmp_path / "src" + build_dir = tmp_path / "build" + src_dir.mkdir() + build_dir.mkdir() + + with patched_env( + env={}, + cython_line_tracing_requested=False, + original_source_directory=src_dir, + temporary_build_directory=build_dir, + ): + assert "-O2" in os.environ["CFLAGS"].split() + assert "-O3" in os.environ["CXXFLAGS"].split() + assert any( + tok.startswith("-ffile-prefix-map=") for tok in os.environ["CFLAGS"].split() + ) + + +def _install_backend_stubs( + monkeypatch: pytest.MonkeyPatch, +) -> dict[str, object]: + """Replace the heavy bits of the build pipeline with cheap recorders. + + Returns a dict the caller can inspect to assert on observed arguments. + Used by ``maybe_prebuild_c_extensions`` / ``build_editable`` tests so the + contexts can be entered without invoking ``cythonize`` or ``setuptools``. + """ + observed: dict[str, object] = {} + + def fake_get_config() -> dict[str, object]: + return {"env": {}, "flags": {}, "kwargs": {}, "src": []} + + def fake_make_args(config: object, line_tracing: bool) -> list[str]: + return [] + + def fake_cythonize(args: list[str]) -> None: + observed["cythonize_called"] = True + + def fake_build_editable( + wheel_directory: str, + config_settings: object = None, + metadata_directory: str | None = None, + ) -> str: + observed["editable_directory"] = wheel_directory + return "stub-editable" + + @contextmanager + def fake_patched_env( + env: dict[str, str], + cython_line_tracing_requested: bool, + *, + original_source_directory: Path | None = None, + temporary_build_directory: Path | None = None, + ) -> Iterator[None]: + observed["original_source_directory"] = original_source_directory + observed["temporary_build_directory"] = temporary_build_directory + yield + + monkeypatch.setattr(_backend, "_get_local_cython_config", fake_get_config) + monkeypatch.setattr( + _backend, + "_make_cythonize_cli_args_from_config", + fake_make_args, + ) + monkeypatch.setattr(_backend, "_cythonize_cli_cmd", fake_cythonize) + monkeypatch.setattr(_backend, "_setuptools_build_editable", fake_build_editable) + monkeypatch.setattr(_backend, "_patched_cython_env", fake_patched_env) + return observed + + +def test_maybe_prebuild_c_extensions_inplace_passes_no_tmp_dir( + clean_env: None, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``build_inplace=True`` takes the ``nullcontext`` branch. + + ``patched_env`` should receive ``temporary_build_directory=None`` so the + ``-ffile-prefix-map`` flag is *not* injected when the build runs in-tree. + """ + observed = _install_backend_stubs(monkeypatch) + + with maybe_prebuild_c_extensions(build_inplace=True): + pass + + assert observed["temporary_build_directory"] is None + assert isinstance(observed["original_source_directory"], Path) + assert observed["cythonize_called"] is True + + +def test_maybe_prebuild_c_extensions_tmp_dir_threads_paths( + clean_env: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """``build_inplace=False`` enters ``_in_temporary_directory``. + + Verify both ``original_source_directory`` and ``temporary_build_directory`` + are forwarded to ``patched_env`` so the reproducibility flag fires. + """ + observed = _install_backend_stubs(monkeypatch) + fake_tmp_dir = tmp_path / "fake-tmp" + fake_tmp_dir.mkdir() + + @contextmanager + def fake_in_temporary_directory(src_dir: Path) -> Iterator[Path]: + observed["in_tmp_src_dir"] = src_dir + yield fake_tmp_dir + + monkeypatch.setattr( + _backend, + "_in_temporary_directory", + fake_in_temporary_directory, + ) + + with maybe_prebuild_c_extensions(build_inplace=False): + pass + + assert observed["temporary_build_directory"] == fake_tmp_dir + assert observed["original_source_directory"] == observed["in_tmp_src_dir"] + + +def test_build_editable_warns_when_user_disables_inplace( + clean_env: None, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """``build_editable`` rejects ``build-inplace=false`` with a warning. + + Editable installs require in-tree artifacts; downstream packagers passing + the opt-out get a ``RuntimeWarning`` explaining why their override was + ignored. Covers the ``build_editable`` warning branch. + """ + _install_backend_stubs(monkeypatch) + + with pytest.warns(RuntimeWarning, match="in-tree"): + result = build_editable( + str(tmp_path), + config_settings={BUILD_INPLACE_CONFIG_SETTING: "false"}, + ) + assert result == "stub-editable"