From ebf8369b13b43ff2d5f6a58875246218fe922c9c Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 16 Jun 2022 00:59:52 +0100 Subject: [PATCH 01/12] Add translator for config_settings to build_meta --- setuptools/build_meta.py | 157 +++++++++++++++++++++++++------- setuptools/command/dist_info.py | 16 +++- 2 files changed, 141 insertions(+), 32 deletions(-) diff --git a/setuptools/build_meta.py b/setuptools/build_meta.py index 1d67e756ac..0e5b147cec 100644 --- a/setuptools/build_meta.py +++ b/setuptools/build_meta.py @@ -28,17 +28,19 @@ import io import os +import shlex import sys import tokenize import shutil import contextlib import tempfile import warnings +from typing import Dict, List, Optional, Union import setuptools import distutils from ._reqs import parse_strings -from .extern.more_itertools import always_iterable +from distutils.util import strtobool __all__ = ['get_requires_for_build_sdist', @@ -129,33 +131,128 @@ def suppress_known_deprecation(): yield -class _BuildMetaBackend: +_ConfigSettings = Optional[Dict[str, Union[str, List[str], None]]] +""" +Currently the user can run:: + + pip install -e . --config-settings key=value + python -m build -C--key=value -C key=value + +- pip will pass both key and value as strings and overwriting repeated keys + (pypa/pip#11059). +- build will accumulate values associated with repeated keys in a list. + It will also accept keys with no associated value. + This means that an option passed by build can be ``str | list[str] | None``. +- PEP 517 specifies that ``config_settings`` is an optional dict. +""" - @staticmethod - def _fix_config(config_settings): + +class _ConfigSettingsTranslator: + """Translate ``config_settings`` into distutils-style command arguments. + Only a limited number of options is currently supported. + """ + + def _global_args(self, config_settings: _ConfigSettings) -> List[str]: + """ + If the user specify ``log-level``, it should be applied to all commands. + + >>> fn = _ConfigSettingsTranslator()._global_args + >>> fn(None) + [] + >>> fn({"log-level": "WARNING"}) + ['-q'] + >>> fn({"log-level": "DEBUG"}) + ['-vv'] + >>> fn({"log-level": None}) + Traceback (most recent call last): + ... + ValueError: Invalid value for log-level: None. + Try one of: ['WARNING', 'INFO', 'DEBUG']. """ - Ensure config settings meet certain expectations. - - >>> fc = _BuildMetaBackend._fix_config - >>> fc(None) - {'--global-option': []} - >>> fc({}) - {'--global-option': []} - >>> fc({'--global-option': 'foo'}) - {'--global-option': ['foo']} - >>> fc({'--global-option': ['foo']}) - {'--global-option': ['foo']} + log_levels = {"WARNING": "-q", "INFO": "-v", "DEBUG": "-vv"} + cfg = config_settings or {} + if "log-level" in cfg: + level = cfg["log-level"] + if level not in log_levels: + msg = f"Invalid value for log-level: {level!r}." + raise ValueError(msg + f"\nTry one of: {list(log_levels.keys())}.") + assert isinstance(level, str) + return [log_levels[level]] + return [] + + def _dist_info_args(self, config_settings: _ConfigSettings) -> List[str]: """ - config_settings = config_settings or {} - config_settings['--global-option'] = list(always_iterable( - config_settings.get('--global-option'))) - return config_settings + The ``dist_info`` command accepts ``tag-date`` and ``tag-build``. + + >>> fn = _ConfigSettingsTranslator()._dist_info_args + >>> fn(None) + [] + >>> fn({"tag-date": "False"}) + ['--no-date'] + >>> fn({"tag-date": None}) + ['--no-date'] + >>> fn({"tag-date": "true", "tag-build": ".a"}) + ['--tag-date', '--tag-build', '.a'] + """ + cfg = config_settings or {} + args: List[str] = [] + if "tag-date" in cfg: + val = strtobool(str(cfg["tag-date"] or "false")) + args.append("--tag-date" if val else "--no-date") + if "tag-build" in cfg: + args.extend(["--tag-build", str(cfg["tag-build"])]) + return args + + def _editable_args(self, config_settings: _ConfigSettings) -> List[str]: + """ + The ``editable_wheel`` command accepts ``editable-mode=strict``. + + >>> fn = _ConfigSettingsTranslator()._editable_args + >>> fn(None) + [] + >>> fn({"editable-mode": "strict"}) + ['--strict'] + >>> fn({"editable-mode": "other"}) + Traceback (most recent call last): + ... + ValueError: Invalid value for editable-mode: 'other'. Try: 'strict'. + """ + cfg = config_settings or {} + if "editable-mode" not in cfg: + return [] + mode = cfg["editable-mode"] + if mode != "strict": + msg = f"Invalid value for editable-mode: {mode!r}. Try: 'strict'." + raise ValueError(msg) + return ["--strict"] + + def _arbitrary_args(self, config_settings: _ConfigSettings) -> List[str]: + """ + Users may expect to pass arbitrary lists of arguments to a command + via "--global-option" (example provided in PEP 517 of a "escape hatch"). + + >>> fn = _ConfigSettingsTranslator()._arbitrary_args + >>> fn(None) + [] + >>> fn({}) + [] + >>> fn({'--global-option': 'foo'}) + ['foo'] + >>> fn({'--global-option': ['foo']}) + ['foo'] + >>> fn({'--global-option': 'foo'}) + ['foo'] + >>> fn({'--global-option': 'foo bar'}) + ['foo', 'bar'] + """ + cfg = config_settings or {} + opts = cfg.get("--global-option") or [] + return shlex.split(opts) if isinstance(opts, str) else opts - def _get_build_requires(self, config_settings, requirements): - config_settings = self._fix_config(config_settings) - sys.argv = sys.argv[:1] + ['egg_info'] + \ - config_settings["--global-option"] +class _BuildMetaBackend(_ConfigSettingsTranslator): + def _get_build_requires(self, config_settings, requirements): + sys.argv = [*sys.argv[:1], "egg_info", *self._arbitrary_args(config_settings)] try: with Distribution.patch(): self.run_setup() @@ -176,16 +273,14 @@ def run_setup(self, setup_script='setup.py'): exec(compile(code, __file__, 'exec'), locals()) def get_requires_for_build_wheel(self, config_settings=None): - return self._get_build_requires( - config_settings, requirements=['wheel']) + return self._get_build_requires(config_settings, requirements=['wheel']) def get_requires_for_build_sdist(self, config_settings=None): return self._get_build_requires(config_settings, requirements=[]) def prepare_metadata_for_build_wheel(self, metadata_directory, config_settings=None): - sys.argv = sys.argv[:1] + [ - 'dist_info', '--output-dir', metadata_directory] + sys.argv = [*sys.argv[:1], 'dist_info', '--output-dir', metadata_directory] with no_install_setup_requires(): self.run_setup() @@ -218,15 +313,15 @@ def prepare_metadata_for_build_wheel(self, metadata_directory, def _build_with_temp_dir(self, setup_command, result_extension, result_directory, config_settings): - config_settings = self._fix_config(config_settings) + args = self._arbitrary_args(config_settings) result_directory = os.path.abspath(result_directory) # Build in a temporary directory, then copy to the target. os.makedirs(result_directory, exist_ok=True) with tempfile.TemporaryDirectory(dir=result_directory) as tmp_dist_dir: - sys.argv = (sys.argv[:1] + setup_command + - ['--dist-dir', tmp_dist_dir] + - config_settings["--global-option"]) + sys.argv = [ + *sys.argv[:1], *setup_command, "--dist-dir", tmp_dist_dir, *args + ] with no_install_setup_requires(): self.run_setup() diff --git a/setuptools/command/dist_info.py b/setuptools/command/dist_info.py index aa7af48c1a..323dbefc94 100644 --- a/setuptools/command/dist_info.py +++ b/setuptools/command/dist_info.py @@ -25,13 +25,21 @@ class dist_info(Command): " DEPRECATED: use --output-dir."), ('output-dir=', 'o', "directory inside of which the .dist-info will be" "created (default: top of the source tree)"), + ('tag-date', 'd', "Add date stamp (e.g. 20050528) to version number"), + ('tag-build=', 'b', "Specify explicit tag to add to version number"), + ('no-date', 'D', "Don't include date stamp [default]"), ] + boolean_options = ['tag-date'] + negative_opt = {'no-date': 'tag-date'} + def initialize_options(self): self.egg_base = None self.output_dir = None self.name = None self.dist_info_dir = None + self.tag_date = None + self.tag_build = None def finalize_options(self): if self.egg_base: @@ -43,8 +51,14 @@ def finalize_options(self): project_dir = dist.src_root or os.curdir self.output_dir = Path(self.output_dir or project_dir) - egg_info = self.reinitialize_command('egg_info') + self.set_undefined_options( + "egg_info", ("tag_date", "tag_date"), ("tag_build", "tag_build") + ) + + egg_info = self.reinitialize_command("egg_info") egg_info.egg_base = str(self.output_dir) + egg_info.tag_date = self.tag_date + egg_info.tag_build = self.tag_build egg_info.finalize_options() self.egg_info = egg_info From 6f680c986759cb03922df9e2b275efbb5a17f796 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 16 Jun 2022 01:27:47 +0100 Subject: [PATCH 02/12] Ensure new options for dist-info work --- setuptools/command/dist_info.py | 17 +++++++++++------ setuptools/tests/test_dist_info.py | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/setuptools/command/dist_info.py b/setuptools/command/dist_info.py index 323dbefc94..39a74e1e1e 100644 --- a/setuptools/command/dist_info.py +++ b/setuptools/command/dist_info.py @@ -51,14 +51,19 @@ def finalize_options(self): project_dir = dist.src_root or os.curdir self.output_dir = Path(self.output_dir or project_dir) - self.set_undefined_options( - "egg_info", ("tag_date", "tag_date"), ("tag_build", "tag_build") - ) - egg_info = self.reinitialize_command("egg_info") egg_info.egg_base = str(self.output_dir) - egg_info.tag_date = self.tag_date - egg_info.tag_build = self.tag_build + + if self.tag_date: + egg_info.tag_date = self.tag_date + else: + self.tag_date = egg_info.tag_date + + if self.tag_build: + egg_info.tag_build = self.tag_build + else: + self.tag_build = egg_info.tag_build + egg_info.finalize_options() self.egg_info = egg_info diff --git a/setuptools/tests/test_dist_info.py b/setuptools/tests/test_dist_info.py index eb41a66775..5cd1a3428d 100644 --- a/setuptools/tests/test_dist_info.py +++ b/setuptools/tests/test_dist_info.py @@ -2,6 +2,7 @@ """ import pathlib import re +import shutil import subprocess import sys from functools import partial @@ -91,6 +92,26 @@ def test_invalid_version(self, tmp_path): dist_info = next(tmp_path.glob("*.dist-info")) assert dist_info.name.startswith("proj-42") + def test_tag_arguments(self, tmp_path): + config = """ + [metadata] + name=proj + version=42 + [egg_info] + tag_date=1 + tag_build=.post + """ + (tmp_path / "setup.cfg").write_text(config, encoding="utf-8") + + print(run_command("dist_info", "--no-date", cwd=tmp_path)) + dist_info = next(tmp_path.glob("*.dist-info")) + assert dist_info.name.startswith("proj-42") + shutil.rmtree(dist_info) + + print(run_command("dist_info", "--tag-build", ".a", cwd=tmp_path)) + dist_info = next(tmp_path.glob("*.dist-info")) + assert dist_info.name.startswith("proj-42a") + def test_output_dir(self, tmp_path): config = "[metadata]\nname=proj\nversion=42\n" (tmp_path / "setup.cfg").write_text(config, encoding="utf-8") From da33c520ef3500052ea54ac55675ab37c5e98725 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 16 Jun 2022 10:39:30 +0100 Subject: [PATCH 03/12] Make the dist info args translation private It requires changes in other parts of the build system/commands to be able to be used. --- setuptools/build_meta.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/setuptools/build_meta.py b/setuptools/build_meta.py index 0e5b147cec..d02d656506 100644 --- a/setuptools/build_meta.py +++ b/setuptools/build_meta.py @@ -180,11 +180,16 @@ def _global_args(self, config_settings: _ConfigSettings) -> List[str]: return [log_levels[level]] return [] - def _dist_info_args(self, config_settings: _ConfigSettings) -> List[str]: + def __dist_info_args(self, config_settings: _ConfigSettings) -> List[str]: """ The ``dist_info`` command accepts ``tag-date`` and ``tag-build``. - >>> fn = _ConfigSettingsTranslator()._dist_info_args + .. warning:: + We cannot use this yet as it requires the ``sdist`` and ``bdist_wheel`` + commands run in ``build_sdist`` and ``build_wheel`` to re-use the egg-info + directory created in ``prepare_metadata_for_build_wheel``. + + >>> fn = _ConfigSettingsTranslator()._ConfigSettingsTranslator__dist_info_args >>> fn(None) [] >>> fn({"tag-date": "False"}) From 26f38a25dbed69fd55901b9a1aebfc38631504da Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 16 Jun 2022 12:05:23 +0100 Subject: [PATCH 04/12] build_meta: Consider --global-option and --build-option --- setuptools/build_meta.py | 116 +++++++++++++++++++++++++++------------ 1 file changed, 82 insertions(+), 34 deletions(-) diff --git a/setuptools/build_meta.py b/setuptools/build_meta.py index d02d656506..e2a0f7f7f1 100644 --- a/setuptools/build_meta.py +++ b/setuptools/build_meta.py @@ -35,7 +35,7 @@ import contextlib import tempfile import warnings -from typing import Dict, List, Optional, Union +from typing import Dict, Iterator, List, Optional, Union import setuptools import distutils @@ -151,23 +151,52 @@ class _ConfigSettingsTranslator: """Translate ``config_settings`` into distutils-style command arguments. Only a limited number of options is currently supported. """ + # See pypa/setuptools#1928 - def _global_args(self, config_settings: _ConfigSettings) -> List[str]: + def _get_config(self, key: str, config_settings: _ConfigSettings) -> List[str]: + """ + Get the value of a specific key in ``config_settings`` as a list of strings. + + >>> fn = _ConfigSettingsTranslator()._get_config + >>> fn("--global-option", None) + [] + >>> fn("--global-option", {}) + [] + >>> fn("--global-option", {'--global-option': 'foo'}) + ['foo'] + >>> fn("--global-option", {'--global-option': ['foo']}) + ['foo'] + >>> fn("--global-option", {'--global-option': 'foo'}) + ['foo'] + >>> fn("--global-option", {'--global-option': 'foo bar'}) + ['foo', 'bar'] + """ + cfg = config_settings or {} + opts = cfg.get(key) or [] + return shlex.split(opts) if isinstance(opts, str) else opts + + def _valid_global_options(self): + options = (opt[:2] for opt in setuptools.dist.Distribution.global_options) + return {flag for long_and_short in options for flag in long_and_short if flag} + + def _global_args(self, config_settings: _ConfigSettings) -> Iterator[str]: """ If the user specify ``log-level``, it should be applied to all commands. >>> fn = _ConfigSettingsTranslator()._global_args - >>> fn(None) + >>> list(fn(None)) [] - >>> fn({"log-level": "WARNING"}) + >>> list(fn({"log-level": "WARNING"})) ['-q'] - >>> fn({"log-level": "DEBUG"}) + >>> list(fn({"log-level": "DEBUG"})) ['-vv'] - >>> fn({"log-level": None}) + >>> list(fn({"log-level": None})) Traceback (most recent call last): ... ValueError: Invalid value for log-level: None. Try one of: ['WARNING', 'INFO', 'DEBUG']. + >>> list(fn({"log-level": "DEBUG", "--global-option": "-q --no-user-cfg"})) + ['-vv', '-q', '--no-user-cfg'] """ log_levels = {"WARNING": "-q", "INFO": "-v", "DEBUG": "-vv"} cfg = config_settings or {} @@ -177,10 +206,13 @@ def _global_args(self, config_settings: _ConfigSettings) -> List[str]: msg = f"Invalid value for log-level: {level!r}." raise ValueError(msg + f"\nTry one of: {list(log_levels.keys())}.") assert isinstance(level, str) - return [log_levels[level]] - return [] + yield log_levels[level] - def __dist_info_args(self, config_settings: _ConfigSettings) -> List[str]: + valid = self._valid_global_options() + args = self._get_config("--global-option", config_settings) + yield from (arg for arg in args if arg.strip("-") in valid) + + def __dist_info_args(self, config_settings: _ConfigSettings) -> Iterator[str]: """ The ``dist_info`` command accepts ``tag-date`` and ``tag-build``. @@ -190,74 +222,90 @@ def __dist_info_args(self, config_settings: _ConfigSettings) -> List[str]: directory created in ``prepare_metadata_for_build_wheel``. >>> fn = _ConfigSettingsTranslator()._ConfigSettingsTranslator__dist_info_args - >>> fn(None) + >>> list(fn(None)) [] - >>> fn({"tag-date": "False"}) + >>> list(fn({"tag-date": "False"})) ['--no-date'] - >>> fn({"tag-date": None}) + >>> list(fn({"tag-date": None})) ['--no-date'] - >>> fn({"tag-date": "true", "tag-build": ".a"}) + >>> list(fn({"tag-date": "true", "tag-build": ".a"})) ['--tag-date', '--tag-build', '.a'] """ cfg = config_settings or {} - args: List[str] = [] if "tag-date" in cfg: val = strtobool(str(cfg["tag-date"] or "false")) - args.append("--tag-date" if val else "--no-date") + yield ("--tag-date" if val else "--no-date") if "tag-build" in cfg: - args.extend(["--tag-build", str(cfg["tag-build"])]) - return args + yield from ["--tag-build", str(cfg["tag-build"])] - def _editable_args(self, config_settings: _ConfigSettings) -> List[str]: + def _editable_args(self, config_settings: _ConfigSettings) -> Iterator[str]: """ The ``editable_wheel`` command accepts ``editable-mode=strict``. >>> fn = _ConfigSettingsTranslator()._editable_args - >>> fn(None) + >>> list(fn(None)) [] - >>> fn({"editable-mode": "strict"}) + >>> list(fn({"editable-mode": "strict"})) ['--strict'] - >>> fn({"editable-mode": "other"}) + >>> list(fn({"editable-mode": "other"})) Traceback (most recent call last): ... ValueError: Invalid value for editable-mode: 'other'. Try: 'strict'. """ cfg = config_settings or {} if "editable-mode" not in cfg: - return [] + return mode = cfg["editable-mode"] if mode != "strict": msg = f"Invalid value for editable-mode: {mode!r}. Try: 'strict'." raise ValueError(msg) - return ["--strict"] + yield "--strict" - def _arbitrary_args(self, config_settings: _ConfigSettings) -> List[str]: + def _arbitrary_args(self, config_settings: _ConfigSettings) -> Iterator[str]: """ Users may expect to pass arbitrary lists of arguments to a command via "--global-option" (example provided in PEP 517 of a "escape hatch"). >>> fn = _ConfigSettingsTranslator()._arbitrary_args - >>> fn(None) + >>> list(fn(None)) [] - >>> fn({}) + >>> list(fn({})) [] - >>> fn({'--global-option': 'foo'}) + >>> list(fn({'--build-option': 'foo'})) ['foo'] - >>> fn({'--global-option': ['foo']}) + >>> list(fn({'--build-option': ['foo']})) ['foo'] - >>> fn({'--global-option': 'foo'}) + >>> list(fn({'--build-option': 'foo'})) ['foo'] - >>> fn({'--global-option': 'foo bar'}) + >>> list(fn({'--build-option': 'foo bar'})) ['foo', 'bar'] """ - cfg = config_settings or {} - opts = cfg.get("--global-option") or [] - return shlex.split(opts) if isinstance(opts, str) else opts + args = self._get_config("--global-option", config_settings) + global_opts = self._valid_global_options() + warn = [] + + for arg in args: + if arg.strip("-") not in global_opts: + warn.append(arg) + yield arg + + yield from self._get_config("--build-option", config_settings) + + if warn: + msg = f""" + The arguments {warn!r} were given via `--global-option`. + Please use `--build-option` instead, + `--global-option` is reserved to flags like `--verbose` or `--quiet`. + """ + warnings.warn(msg, setuptools.SetuptoolsDeprecationWarning) class _BuildMetaBackend(_ConfigSettingsTranslator): def _get_build_requires(self, config_settings, requirements): - sys.argv = [*sys.argv[:1], "egg_info", *self._arbitrary_args(config_settings)] + sys.argv = [ + *sys.argv[:1], *self._global_args(config_settings), + "egg_info", *self._arbitrary_args(config_settings) + ] try: with Distribution.patch(): self.run_setup() From c338f92eadd16791428e0e6bebf467e5f4024e71 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 16 Jun 2022 12:34:46 +0100 Subject: [PATCH 05/12] Support --build-option alongside --global-option `--build-option` should be used as a escape hatch to pass command specific options, while `--global-option` can be used to pass flags like `--quiet` or `--verbose`. --- setuptools/build_meta.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/setuptools/build_meta.py b/setuptools/build_meta.py index e2a0f7f7f1..9863c9558c 100644 --- a/setuptools/build_meta.py +++ b/setuptools/build_meta.py @@ -151,7 +151,7 @@ class _ConfigSettingsTranslator: """Translate ``config_settings`` into distutils-style command arguments. Only a limited number of options is currently supported. """ - # See pypa/setuptools#1928 + # See pypa/setuptools#1928 pypa/setuptools#2491 def _get_config(self, key: str, config_settings: _ConfigSettings) -> List[str]: """ @@ -303,8 +303,10 @@ def _arbitrary_args(self, config_settings: _ConfigSettings) -> Iterator[str]: class _BuildMetaBackend(_ConfigSettingsTranslator): def _get_build_requires(self, config_settings, requirements): sys.argv = [ - *sys.argv[:1], *self._global_args(config_settings), - "egg_info", *self._arbitrary_args(config_settings) + *sys.argv[:1], + *self._global_args(config_settings), + "egg_info", + *self._arbitrary_args(config_settings), ] try: with Distribution.patch(): @@ -333,7 +335,13 @@ def get_requires_for_build_sdist(self, config_settings=None): def prepare_metadata_for_build_wheel(self, metadata_directory, config_settings=None): - sys.argv = [*sys.argv[:1], 'dist_info', '--output-dir', metadata_directory] + sys.argv = [ + *sys.argv[:1], + *self._global_args(config_settings), + "dist_info", + "--output-dir", metadata_directory, + *self._arbitrary_args(config_settings), + ] with no_install_setup_requires(): self.run_setup() @@ -366,14 +374,17 @@ def prepare_metadata_for_build_wheel(self, metadata_directory, def _build_with_temp_dir(self, setup_command, result_extension, result_directory, config_settings): - args = self._arbitrary_args(config_settings) result_directory = os.path.abspath(result_directory) # Build in a temporary directory, then copy to the target. os.makedirs(result_directory, exist_ok=True) with tempfile.TemporaryDirectory(dir=result_directory) as tmp_dist_dir: sys.argv = [ - *sys.argv[:1], *setup_command, "--dist-dir", tmp_dist_dir, *args + *sys.argv[:1], + *self._global_args(config_settings), + *setup_command, + "--dist-dir", tmp_dist_dir, + *self._arbitrary_args(config_settings), ] with no_install_setup_requires(): self.run_setup() From 4092e3b2db8f709c4bc91399ac3383b8be234307 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 16 Jun 2022 13:17:44 +0100 Subject: [PATCH 06/12] Prefer 'verbose' and 'quiet' instead of 'log-level' --- setuptools/build_meta.py | 44 +++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/setuptools/build_meta.py b/setuptools/build_meta.py index 9863c9558c..041d21d3f7 100644 --- a/setuptools/build_meta.py +++ b/setuptools/build_meta.py @@ -176,37 +176,39 @@ def _get_config(self, key: str, config_settings: _ConfigSettings) -> List[str]: return shlex.split(opts) if isinstance(opts, str) else opts def _valid_global_options(self): + """Global options accepted by setuptools (e.g. quiet or verbose).""" options = (opt[:2] for opt in setuptools.dist.Distribution.global_options) return {flag for long_and_short in options for flag in long_and_short if flag} def _global_args(self, config_settings: _ConfigSettings) -> Iterator[str]: """ - If the user specify ``log-level``, it should be applied to all commands. + Let the user specify ``verbose`` or ``quiet`` + escape hatch via + ``--global-option``. + Note: ``-v``, ``-vv``, ``-vvv`` have similar effects in setuptools, + so we just have to cover the basic scenario ``-v``. >>> fn = _ConfigSettingsTranslator()._global_args >>> list(fn(None)) [] - >>> list(fn({"log-level": "WARNING"})) + >>> list(fn({"verbose": "False"})) + ['-q'] + >>> list(fn({"verbose": "1"})) + ['-v'] + >>> list(fn({"--verbose": None})) + ['-v'] + >>> list(fn({"verbose": "true", "--global-option": "-q --no-user-cfg"})) + ['-v', '-q', '--no-user-cfg'] + >>> list(fn({"--quiet": None})) ['-q'] - >>> list(fn({"log-level": "DEBUG"})) - ['-vv'] - >>> list(fn({"log-level": None})) - Traceback (most recent call last): - ... - ValueError: Invalid value for log-level: None. - Try one of: ['WARNING', 'INFO', 'DEBUG']. - >>> list(fn({"log-level": "DEBUG", "--global-option": "-q --no-user-cfg"})) - ['-vv', '-q', '--no-user-cfg'] """ - log_levels = {"WARNING": "-q", "INFO": "-v", "DEBUG": "-vv"} cfg = config_settings or {} - if "log-level" in cfg: - level = cfg["log-level"] - if level not in log_levels: - msg = f"Invalid value for log-level: {level!r}." - raise ValueError(msg + f"\nTry one of: {list(log_levels.keys())}.") - assert isinstance(level, str) - yield log_levels[level] + falsey = {"false", "no", "0", "off"} + if "verbose" in cfg or "--verbose" in cfg: + level = str(cfg.get("verbose") or cfg.get("--verbose") or "1") + yield ("-q" if level.lower() in falsey else "-v") + if "quiet" in cfg or "--quiet" in cfg: + level = str(cfg.get("quiet") or cfg.get("--quiet") or "1") + yield ("-v" if level.lower() in falsey else "-q") valid = self._valid_global_options() args = self._get_config("--global-option", config_settings) @@ -250,14 +252,14 @@ def _editable_args(self, config_settings: _ConfigSettings) -> Iterator[str]: >>> list(fn({"editable-mode": "other"})) Traceback (most recent call last): ... - ValueError: Invalid value for editable-mode: 'other'. Try: 'strict'. + ValueError: Invalid value for `editable-mode`: 'other'. Try: 'strict'. """ cfg = config_settings or {} if "editable-mode" not in cfg: return mode = cfg["editable-mode"] if mode != "strict": - msg = f"Invalid value for editable-mode: {mode!r}. Try: 'strict'." + msg = f"Invalid value for `editable-mode`: {mode!r}. Try: 'strict'." raise ValueError(msg) yield "--strict" From 46c16363c7c90dfcb2d3e5883ad6c7a449f4c5ad Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 16 Jun 2022 14:23:39 +0100 Subject: [PATCH 07/12] Consider config_settings in build_editable --- setuptools/build_meta.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/setuptools/build_meta.py b/setuptools/build_meta.py index 041d21d3f7..70cc6475b0 100644 --- a/setuptools/build_meta.py +++ b/setuptools/build_meta.py @@ -255,9 +255,9 @@ def _editable_args(self, config_settings: _ConfigSettings) -> Iterator[str]: ValueError: Invalid value for `editable-mode`: 'other'. Try: 'strict'. """ cfg = config_settings or {} - if "editable-mode" not in cfg: + if "editable-mode" not in cfg and "editable_mode" not in cfg: return - mode = cfg["editable-mode"] + mode = cfg.get("editable-mode") or cfg.get("editable_mode") if mode != "strict": msg = f"Invalid value for `editable-mode`: {mode!r}. Try: 'strict'." raise ValueError(msg) @@ -420,10 +420,14 @@ def build_editable( self, wheel_directory, config_settings=None, metadata_directory=None ): # XXX can or should we hide our editable_wheel command normally? - return self._build_with_temp_dir( - ["editable_wheel", "--dist-info-dir", metadata_directory], - ".whl", wheel_directory, config_settings - ) + print(f"{self._editable_args(config_settings)=}") + cmd = [ + "editable_wheel", + "--dist-info-dir", + metadata_directory, + *self._editable_args(config_settings), + ] + return self._build_with_temp_dir(cmd, ".whl", wheel_directory, config_settings) def get_requires_for_build_editable(self, config_settings=None): return self.get_requires_for_build_wheel(config_settings) From df4419251fa5742fd5b2b4d5a9f11a3915be19ba Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 16 Jun 2022 17:24:03 +0100 Subject: [PATCH 08/12] build_meta.build_editable: Add tests for config_settings --- setuptools/build_meta.py | 26 ++++++++----- setuptools/tests/test_build_meta.py | 58 +++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 9 deletions(-) diff --git a/setuptools/build_meta.py b/setuptools/build_meta.py index 70cc6475b0..1010dba465 100644 --- a/setuptools/build_meta.py +++ b/setuptools/build_meta.py @@ -35,11 +35,13 @@ import contextlib import tempfile import warnings +from pathlib import Path from typing import Dict, Iterator, List, Optional, Union import setuptools import distutils from ._reqs import parse_strings +from ._deprecation_warning import SetuptoolsDeprecationWarning from distutils.util import strtobool @@ -281,6 +283,10 @@ def _arbitrary_args(self, config_settings: _ConfigSettings) -> Iterator[str]: ['foo'] >>> list(fn({'--build-option': 'foo bar'})) ['foo', 'bar'] + >>> warnings.simplefilter('error', SetuptoolsDeprecationWarning) + >>> list(fn({'--global-option': 'foo'})) # doctest: +IGNORE_EXCEPTION_DETAIL + Traceback (most recent call last): + SetuptoolsDeprecationWarning: ...arguments given via `--global-option`... """ args = self._get_config("--global-option", config_settings) global_opts = self._valid_global_options() @@ -299,7 +305,7 @@ def _arbitrary_args(self, config_settings: _ConfigSettings) -> Iterator[str]: Please use `--build-option` instead, `--global-option` is reserved to flags like `--verbose` or `--quiet`. """ - warnings.warn(msg, setuptools.SetuptoolsDeprecationWarning) + warnings.warn(msg, SetuptoolsDeprecationWarning) class _BuildMetaBackend(_ConfigSettingsTranslator): @@ -342,7 +348,6 @@ def prepare_metadata_for_build_wheel(self, metadata_directory, *self._global_args(config_settings), "dist_info", "--output-dir", metadata_directory, - *self._arbitrary_args(config_settings), ] with no_install_setup_requires(): self.run_setup() @@ -412,6 +417,13 @@ def build_sdist(self, sdist_directory, config_settings=None): '.tar.gz', sdist_directory, config_settings) + def _get_dist_info_dir(self, metadata_directory: Optional[str]) -> Optional[str]: + if not metadata_directory: + return None + dist_info_candidates = list(Path(metadata_directory).glob("*.dist-info")) + assert len(dist_info_candidates) <= 1 + return str(dist_info_candidates[0]) if dist_info_candidates else None + # PEP660 hooks: # build_editable # get_requires_for_build_editable @@ -420,13 +432,9 @@ def build_editable( self, wheel_directory, config_settings=None, metadata_directory=None ): # XXX can or should we hide our editable_wheel command normally? - print(f"{self._editable_args(config_settings)=}") - cmd = [ - "editable_wheel", - "--dist-info-dir", - metadata_directory, - *self._editable_args(config_settings), - ] + info_dir = self._get_dist_info_dir(metadata_directory) + opts = ["--dist-info-dir", info_dir] if info_dir else [] + cmd = ["editable_wheel", *opts, *self._editable_args(config_settings)] return self._build_with_temp_dir(cmd, ".whl", wheel_directory, config_settings) def get_requires_for_build_editable(self, config_settings=None): diff --git a/setuptools/tests/test_build_meta.py b/setuptools/tests/test_build_meta.py index 36940e768f..b4051582e1 100644 --- a/setuptools/tests/test_build_meta.py +++ b/setuptools/tests/test_build_meta.py @@ -8,9 +8,11 @@ from concurrent import futures import re from zipfile import ZipFile +from pathlib import Path import pytest from jaraco import path +from setuptools._deprecation_warning import SetuptoolsDeprecationWarning from .textwrap import DALS @@ -611,6 +613,62 @@ def test_build_sdist_relative_path_import(self, tmpdir_cwd): with pytest.raises(ImportError, match="^No module named 'hello'$"): build_backend.build_sdist("temp") + _simple_pyproject_example = { + "pyproject.toml": DALS(""" + [project] + name = "proj" + version = "42" + """), + "src": { + "proj": {"__init__.py": ""} + } + } + + @pytest.mark.filterwarnings("error::setuptools.SetuptoolsDeprecationWarning") + # For some reason `pytest.warns` does no seem to work here + # but `pytest.raises` does works, together with an error filter. + def test_editable_with_config_settings_warning(self, tmpdir_cwd): + path.build({**self._simple_pyproject_example, '_meta': {}}) + build_backend = self.get_build_backend() + + msg = "The arguments .'--strict'. were given via .--global-option." + with pytest.raises(SetuptoolsDeprecationWarning, match=msg): + cfg = {"--global-option": "--strict"} + build_backend.prepare_metadata_for_build_editable("_meta", cfg) + build_backend.build_editable("temp", cfg, "_meta") + + def test_editable_without_config_settings(self, tmpdir_cwd): + """ + Sanity check to ensure tests with --strict are different from the ones + without --strict. + + --strict should create a local directory with a package tree. + The directory should not get created otherwise. + """ + path.build(self._simple_pyproject_example) + build_backend = self.get_build_backend() + assert not Path("build").exists() + build_backend.build_editable("temp") + assert not Path("build").exists() + + @pytest.mark.parametrize( + "config_settings", [ + {"--build-option": "--strict"}, + {"editable-mode": "strict"}, + ] + ) + def test_editable_with_config_settings(self, tmpdir_cwd, config_settings): + path.build({**self._simple_pyproject_example, '_meta': {}}) + assert not Path("build").exists() + build_backend = self.get_build_backend() + build_backend.prepare_metadata_for_build_editable("_meta", config_settings) + build_backend.build_editable("temp", config_settings, "_meta") + files = list(Path("build").glob("__editable__.*/**/*")) + assert files + for file in files: + # All files should either links or hard links + assert file.is_symlink() or os.stat(file).st_nlink > 0 + @pytest.mark.parametrize('setup_literal, requirements', [ ("'foo'", ['foo']), ("['foo']", ['foo']), From 01d961a8b990d50e326ea4759dd62dd215ad2b4d Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 16 Jun 2022 18:16:55 +0100 Subject: [PATCH 09/12] Add warning with information for the user about link tree --- pytest.ini | 1 + setuptools/command/editable_wheel.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/pytest.ini b/pytest.ini index 14c7e94c27..f171d853af 100644 --- a/pytest.ini +++ b/pytest.ini @@ -60,3 +60,4 @@ filterwarnings= ignore:Setuptools is replacing distutils ignore:Support for project metadata in .pyproject.toml. is still experimental + ignore::setuptools.command.editable_wheel.InformationOnly diff --git a/setuptools/command/editable_wheel.py b/setuptools/command/editable_wheel.py index 482029905b..2776577f44 100644 --- a/setuptools/command/editable_wheel.py +++ b/setuptools/command/editable_wheel.py @@ -15,6 +15,7 @@ import shutil import sys import logging +import warnings from itertools import chain from pathlib import Path from tempfile import TemporaryDirectory @@ -166,6 +167,15 @@ def _populate_link_tree( populate = _LinkTree(self.distribution, name, auxiliary_build_dir, tmp) populate(unpacked_dir) + msg = f"""\n + Strict editable installation performed using the auxiliary directory: + {auxiliary_build_dir} + + Please be careful to not remove this directory, otherwise you might not be able + to import/use your package. + """ + warnings.warn(msg, InformationOnly) + def _populate_static_pth(self, name: str, project_dir: Path, unpacked_dir: Path): """Populate wheel using the "lax" ``.pth`` file strategy, for ``src-layout``.""" src_dir = self.package_dir[""] @@ -583,3 +593,11 @@ def _finder_template( """ mapping = dict(sorted(mapping.items(), key=lambda p: p[0])) return _FINDER_TEMPLATE.format(name=name, mapping=mapping, namespaces=namespaces) + + +class InformationOnly(UserWarning): + """Currently there is no clear way of displaying messages to the users + that use the setuptools backend directly via ``pip``. + The only thing that might work is a warning, although it is not the + most appropriate tool for the job... + """ From 57c863a288e1916619e0c4a7b66c3050bd38ca09 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 16 Jun 2022 18:54:44 +0100 Subject: [PATCH 10/12] Add news fragment --- changelog.d/3380.change.rst | 10 ++++++++++ changelog.d/3380.deprecation.rst | 7 +++++++ 2 files changed, 17 insertions(+) create mode 100644 changelog.d/3380.change.rst create mode 100644 changelog.d/3380.deprecation.rst diff --git a/changelog.d/3380.change.rst b/changelog.d/3380.change.rst new file mode 100644 index 0000000000..9622417a07 --- /dev/null +++ b/changelog.d/3380.change.rst @@ -0,0 +1,10 @@ +Improved the handling of the ``config_settings`` parameter in both PEP 517 and +PEP 660 interfaces: + +- It is possible now to pass both ``--global-option`` and ``--build-option``. + As discussed in #1928, arbitrary arguments passed via ``--global-option`` + should be placed before the name of the setuptools' internal command, while + ``--build-option`` should come after. + +- Users can pass ``editable-mode=strict`` to select a strict behaviour for the + editable installation. diff --git a/changelog.d/3380.deprecation.rst b/changelog.d/3380.deprecation.rst new file mode 100644 index 0000000000..54d3c4c37a --- /dev/null +++ b/changelog.d/3380.deprecation.rst @@ -0,0 +1,7 @@ +Passing some types of parameters via ``--global-option`` to setuptools PEP 517/PEP 660 backend +is now considered deprecated. The user can pass the same arbitrary parameter +via ``--build-option`` (``--global-option`` is now reserved for flags like +``--verbose`` or ``--quiet``). + +Both ``--build-option`` and ``--global-option`` are supported as a **transitional** effort (a.k.a. "escape hatch"). +In the future a proper list of allowed ``config_settings`` may be created. From 8db675d5bd5972d00ebd49d09c7dabab87a5d43a Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 16 Jun 2022 21:15:36 +0100 Subject: [PATCH 11/12] Skip problematic test on macOS --- setuptools/tests/test_build_meta.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/setuptools/tests/test_build_meta.py b/setuptools/tests/test_build_meta.py index b4051582e1..a669a3acda 100644 --- a/setuptools/tests/test_build_meta.py +++ b/setuptools/tests/test_build_meta.py @@ -624,9 +624,14 @@ def test_build_sdist_relative_path_import(self, tmpdir_cwd): } } - @pytest.mark.filterwarnings("error::setuptools.SetuptoolsDeprecationWarning") # For some reason `pytest.warns` does no seem to work here - # but `pytest.raises` does works, together with an error filter. + # but `pytest.raises` does works (in systems other then macOS), + # together with an error filter. + @pytest.mark.xfail( + sys.platform == "darwin", + reason="inconsistent behaviour in macOS" + ) + @pytest.mark.filterwarnings("error::setuptools.SetuptoolsDeprecationWarning") def test_editable_with_config_settings_warning(self, tmpdir_cwd): path.build({**self._simple_pyproject_example, '_meta': {}}) build_backend = self.get_build_backend() From a19c4dfb8f7fda86a755b181bbf5819fc4139ab7 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Fri, 17 Jun 2022 10:48:45 +0100 Subject: [PATCH 12/12] Address errors in tests for Windows --- setuptools/build_meta.py | 8 +++--- setuptools/tests/test_build_meta.py | 43 +++++++++++++++-------------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/setuptools/build_meta.py b/setuptools/build_meta.py index 1010dba465..f39a5a626a 100644 --- a/setuptools/build_meta.py +++ b/setuptools/build_meta.py @@ -290,18 +290,18 @@ def _arbitrary_args(self, config_settings: _ConfigSettings) -> Iterator[str]: """ args = self._get_config("--global-option", config_settings) global_opts = self._valid_global_options() - warn = [] + bad_args = [] for arg in args: if arg.strip("-") not in global_opts: - warn.append(arg) + bad_args.append(arg) yield arg yield from self._get_config("--build-option", config_settings) - if warn: + if bad_args: msg = f""" - The arguments {warn!r} were given via `--global-option`. + The arguments {bad_args!r} were given via `--global-option`. Please use `--build-option` instead, `--global-option` is reserved to flags like `--verbose` or `--quiet`. """ diff --git a/setuptools/tests/test_build_meta.py b/setuptools/tests/test_build_meta.py index a669a3acda..7337ef4d1e 100644 --- a/setuptools/tests/test_build_meta.py +++ b/setuptools/tests/test_build_meta.py @@ -12,7 +12,6 @@ import pytest from jaraco import path -from setuptools._deprecation_warning import SetuptoolsDeprecationWarning from .textwrap import DALS @@ -624,23 +623,31 @@ def test_build_sdist_relative_path_import(self, tmpdir_cwd): } } - # For some reason `pytest.warns` does no seem to work here - # but `pytest.raises` does works (in systems other then macOS), - # together with an error filter. - @pytest.mark.xfail( - sys.platform == "darwin", - reason="inconsistent behaviour in macOS" - ) - @pytest.mark.filterwarnings("error::setuptools.SetuptoolsDeprecationWarning") - def test_editable_with_config_settings_warning(self, tmpdir_cwd): + def _assert_link_tree(self, parent_dir): + """All files in the directory should be either links or hard links""" + files = list(Path(parent_dir).glob("**/*")) + assert files # Should not be empty + for file in files: + assert file.is_symlink() or os.stat(file).st_nlink > 0 + + @pytest.mark.filterwarnings("ignore::setuptools.SetuptoolsDeprecationWarning") + # Since the backend is running via a process pool, in some operating systems + # we may have problems to make assertions based on warnings/stdout/stderr... + # So the best is to ignore them for the time being. + def test_editable_with_global_option_still_works(self, tmpdir_cwd): + """The usage of --global-option is now discouraged in favour of --build-option. + This is required to make more sense of the provided scape hatch and align with + previous pip behaviour. See pypa/setuptools#1928. + """ path.build({**self._simple_pyproject_example, '_meta': {}}) build_backend = self.get_build_backend() + assert not Path("build").exists() + + cfg = {"--global-option": "--strict"} + build_backend.prepare_metadata_for_build_editable("_meta", cfg) + build_backend.build_editable("temp", cfg, "_meta") - msg = "The arguments .'--strict'. were given via .--global-option." - with pytest.raises(SetuptoolsDeprecationWarning, match=msg): - cfg = {"--global-option": "--strict"} - build_backend.prepare_metadata_for_build_editable("_meta", cfg) - build_backend.build_editable("temp", cfg, "_meta") + self._assert_link_tree(next(Path("build").glob("__editable__.*"))) def test_editable_without_config_settings(self, tmpdir_cwd): """ @@ -668,11 +675,7 @@ def test_editable_with_config_settings(self, tmpdir_cwd, config_settings): build_backend = self.get_build_backend() build_backend.prepare_metadata_for_build_editable("_meta", config_settings) build_backend.build_editable("temp", config_settings, "_meta") - files = list(Path("build").glob("__editable__.*/**/*")) - assert files - for file in files: - # All files should either links or hard links - assert file.is_symlink() or os.stat(file).st_nlink > 0 + self._assert_link_tree(next(Path("build").glob("__editable__.*"))) @pytest.mark.parametrize('setup_literal, requirements', [ ("'foo'", ['foo']),