rpms / pyproject-rpm-macros

Clone
Source Code
GIT

#385 Allow passing config_settings to the build backend
Merged a year ago by churchyard. Opened a year ago by gotmax23.
rpms/ gotmax23/pyproject-rpm-macros config-settings  into  rawhide

file modified
+37 
@@ -163,6 +163,43 @@ 
 

  it means "include runtime dependencies" which has been the default since version 0-53.
 

  

  

+ Passing config settings to build backends
 

+ -----------------------------------------
 

+ 

+ The `%pyproject_buildrequires` and `%pyproject_wheel` macros accept a `-C` flag
 

+ to pass [configuration settings][config_settings] to the build backend.
 

+ Options take the form of `-C KEY`, `-C KEY=VALUE`, or `-C--option-with-dashes`.
 

+ Pass `-C` multiple times to specify multiple options.
 

+ This option is equivalent to pip's `--config-settings` flag.
 

+ These are passed on to PEP 517 hooks' `config_settings` argument as a Python
 

+ dictionary.
 

+ 

+ The `%pyproject_buildrequires` macro passes these options to the
 

+ `get_requires_for_build_wheel` and `prepare_metadata_for_build_wheel` hooks.
 

+ Passing `-C` to `%pyproject_buildrequires` is incompatible with `-N` which does
 

+ not call these hooks at all.
 

+ 

+ The `%pyproject_wheel` macro passes these options to the `build_wheel` hook.
 

+ 

+ Consult the project's upstream documentation and/or the corresponding build
 

+ backend's documentation for more information.
 

+ Note that some projects don't use config settings at all
 

+ and other projects may only accept config settings for one of the two steps.
 

+ 

+ Note that the current implementation of the macros uses `pip` to build wheels.
 

+ On some systems (notably on RHEL 9 with Python 3.9),
 

+ `pip` is too old to understand `--config-settings`.
 

+ Using the `-C` option for `%pyproject_wheel` (or `%pyproject_buildrequires -w`)
 

+ is not supported there and will result to an error like:
 

+ 

+     Usage:   

+       /usr/bin/python3 -m pip wheel [options] <requirement specifier> ...
 

+       ...
 

+     no such option: --config-settings
 

+ 

+ [config_settings]: https://peps.python.org/pep-0517/#config-settings
 

+ 

+ 

  Running tox based tests
 

  -----------------------

file modified
+1 -1 
@@ -4,4 +4,4 @@ 
 

  # this macro will cause the package with the real macro to be installed.
 

  # When macros.pyproject is installed, it overrides this macro.
 

  # Note: This needs to maintain the same set of options as the real macro.
 

- %pyproject_buildrequires(rRxtNwe:) echo 'pyproject-rpm-macros' && exit 0
 

+ %pyproject_buildrequires(rRxtNwe:C:) echo 'pyproject-rpm-macros' && exit 0

Thanks for updating pyproject-srpm-macros. Note to self: Add a test that ensures the two declarations are in sync.

Edited a year ago by churchyard

file modified
+4 -3 
@@ -26,11 +26,11 @@ 
 

  # The value is read and used by the %%pytest and %%tox macros:
 

  %_set_pytest_addopts %global __pytest_addopts --ignore=%{_pyproject_builddir}
 

  

- %pyproject_wheel() %{expand:\\\
 

+ %pyproject_wheel(C:) %{expand:\\\
 

  %_set_pytest_addopts
 

  mkdir -p "%{_pyproject_builddir}"
 

  CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}" TMPDIR="%{_pyproject_builddir}" \\\
 

- %{__python3} -Bs %{_rpmconfigdir}/redhat/pyproject_wheel.py %{_pyproject_wheeldir}
 

+ %{__python3} -Bs %{_rpmconfigdir}/redhat/pyproject_wheel.py %{?**} %{_pyproject_wheeldir}
 

  }
 

  

  
@@ -140,7 +140,7 @@ 
 

  

  

  # Note: Keep the options in sync with this macro from macros.aaa-pyproject-srpm
 

- %pyproject_buildrequires(rRxtNwe:) %{expand:\\\
 

+ %pyproject_buildrequires(rRxtNwe:C:) %{expand:\\\
 

  %_set_pytest_addopts
 

  # The _auto_set_build_flags feature does not do this in %%generate_buildrequires section,
 

  # but we want to get an environment consistent with %%build:
 
@@ -158,6 +158,7 @@ 
 

  %{-e:%{error:The -N and -e options are mutually exclusive}}
 

  %{-t:%{error:The -N and -t options are mutually exclusive}}
 

  %{-w:%{error:The -N and -w options are mutually exclusive}}
 

+ %{-C:%{error:The -N and -C options are mutually exclusive}}
 

  }
 

  %{-e:%{expand:%global toxenv %(%{__python3} -s %{_rpmconfigdir}/redhat/pyproject_construct_toxenv.py %{?**})}}
 

  echo 'pyproject-rpm-macros'  # first stdout line matches the implementation in macros.aaa-pyproject-srpm

file modified
+5 -1 
@@ -13,7 +13,7 @@ 
 

  #   Increment Y and reset Z when new macros or features are added
 

  #   Increment Z when this is a bugfix or a cosmetic change
 

  # Dropping support for EOL Fedoras is *not* considered a breaking change
 

- Version:        1.8.1
 

+ Version:        1.9.0
 

  Release:        1%{?dist}
 

  

  # Macro files
 
@@ -161,6 +161,10 @@ 
 

  

  

  %changelog
 

+ * Wed May 31 2023 Maxwell G <maxwell@gtmx.me> - 1.9.0-1
 

+ - Allow passing config_settings to the build backend.
 

+ - Resolves: rhbz#2192581
 

+ 

  * Wed May 31 2023 Miro Hrončok <mhroncok@redhat.com> - 1.8.1-1
 

  - On Python older than 3.11, use tomli instead of deprecated toml
 

  - Fix literal %% handling in %%{pyproject_files} on RPM 4.19

file modified
+20 -6 
@@ -14,6 +14,7 @@ 
 

  import zipfile
 

  

  from pyproject_requirements_txt import convert_requirements_txt
 

+ from pyproject_wheel import parse_config_settings_args
 

  

  

  # Some valid Python version specifiers are not supported.
 
@@ -67,7 +68,7 @@ 
 

  class Requirements:
 

      """Requirement gatherer. The macro will eventually print out output_lines."""
 

      def __init__(self, get_installed_version, extras=None,
 

-                  generate_extras=False, python3_pkgversion='3'):
 

+                  generate_extras=False, python3_pkgversion='3', config_settings=None):
 

          self.get_installed_version = get_installed_version
 

          self.output_lines = []
 

          self.extras = set()
 
@@ -81,6 +82,7 @@ 
 

  

          self.generate_extras = generate_extras
 

          self.python3_pkgversion = python3_pkgversion
 

+         self.config_settings = config_settings
 

  

      def add_extras(self, *extras):
 

          self.extras |= set(e.strip() for e in extras)
 
@@ -269,7 +271,7 @@ 
 

  def generate_build_requirements(backend, requirements):
 

      get_requires = getattr(backend, 'get_requires_for_build_wheel', None)
 

      if get_requires:
 

-         new_reqs = get_requires()
 

+         new_reqs = get_requires(config_settings=requirements.config_settings)
 

          requirements.extend(new_reqs, source='get_requires_for_build_wheel')
 

          requirements.check(source='get_requires_for_build_wheel')
 

  
@@ -303,7 +305,7 @@ 
 

              'Use the provisional -w flag to build the wheel and parse the metadata from it, '
 

              'or use the -R flag not to generate runtime dependencies.'
 

          )
 

-     dir_basename = prepare_metadata('.')
 

+     dir_basename = prepare_metadata('.', config_settings=requirements.config_settings)
 

      with open(dir_basename + '/METADATA') as metadata_file:
 

          name, requires = package_name_and_requires_from_metadata_file(metadata_file)
 

          for key, req in requires.items():
 
@@ -327,7 +329,11 @@ 
 

      wheel = find_built_wheel(wheeldir)
 

      if not wheel:
 

          import pyproject_wheel
 

-         returncode = pyproject_wheel.build_wheel(wheeldir=wheeldir, stdout=sys.stderr)
 

+         returncode = pyproject_wheel.build_wheel(
 

+             wheeldir=wheeldir,
 

+             stdout=sys.stderr,
 

+             config_settings=requirements.config_settings,
 

+         )
 

          if returncode != 0:
 

              raise RuntimeError('Failed to build the wheel for %pyproject_buildrequires -w.')
 

          wheel = find_built_wheel(wheeldir)
 
@@ -415,7 +421,7 @@ 
 

      *, include_runtime=False, build_wheel=False, wheeldir=None, toxenv=None, extras=None,
 

      get_installed_version=importlib.metadata.version,  # for dep injection
 

      generate_extras=False, python3_pkgversion="3", requirement_files=None, use_build_system=True,
 

-     output,
 

+     output, config_settings=None,
 

  ):
 

      """Generate the BuildRequires for the project in the current directory
 

  
@@ -426,7 +432,8 @@ 
 

      requirements = Requirements(
 

          get_installed_version, extras=extras or [],
 

          generate_extras=generate_extras,
 

-         python3_pkgversion=python3_pkgversion
 

+         python3_pkgversion=python3_pkgversion,
 

+         config_settings=config_settings,
 

      )
 

  

      try:
 
@@ -516,6 +523,12 @@ 
 

          metavar='REQUIREMENTS.TXT',
 

          help=('Add buildrequires from file'),
 

      )
 

+     parser.add_argument(
 

+         '-C',
 

+         dest='config_settings',
 

+         action='append',
 

+         help='Configuration settings to pass to the PEP 517 backend',
 

+     )
 

  

      args = parser.parse_args(argv)
 

  
@@ -550,6 +563,7 @@ 
 

              requirement_files=args.requirement_files,
 

              use_build_system=args.use_build_system,
 

              output=args.output,
 

+             config_settings=parse_config_settings_args(args.config_settings),
 

          )
 

      except Exception:
 

          # Log the traceback explicitly (it's useful debug info)

file modified
+37 
@@ -986,3 +986,40 @@ 
 

      python3dist(rightdep)
 

      python3dist(startdep)
 

    result: 0
 

+ 

+ config_settings_control:
 

+   include_runtime: false
 

+   config_settings:
 

+   pyproject.toml: |
 

+     [build-system]
 

+     build-backend = "test_backend"
 

+     backend-path = ["."]
 

+   test_backend.py: |
 

+     def get_requires_for_build_wheel(config_settings=None):
 

+         if not (config_settings is None or isinstance(config_settings, dict)):
 

+             raise TypeError
 

+         if config_settings and "test-config-setting" in config_settings:
 

+             return ["test-config-setting"]
 

+         return ["test-no-config-setting"]
 

+   expected: |
 

+     python3dist(test-no-config-setting)
 

+   result: 0
 

+ 

+ config_settings:
 

+   include_runtime: false
 

+   config_settings:
 

+     test-config-setting: ""
 

+   pyproject.toml: |
 

+     [build-system]
 

+     build-backend = "test_backend"
 

+     backend-path = ["."]
 

+   test_backend.py: |
 

+     def get_requires_for_build_wheel(config_settings=None):
 

+         if not (config_settings is None or isinstance(config_settings, dict)):
 

+             raise TypeError
 

+         if config_settings and "test-config-setting" in config_settings:
 

+             return ["test-config-setting"]
 

+         return ["test-no-config-setting"]
 

+   expected: |
 

+     python3dist(test-config-setting)
 

+   result: 0

file modified
+55 -2 
@@ -1,8 +1,46 @@ 
 

+ import argparse
 

  import sys
 

  import subprocess
 

  

  

- def build_wheel(*, wheeldir, stdout=None):
 

+ def parse_config_settings_args(config_settings):
 

+     """
 

+     Given a list of config `KEY=VALUE` formatted config settings,
 

+     return a dictionary that can be passed to PEP 517 hook functions.
 

+     """
 

+     if not config_settings:
 

+         return config_settings
 

+     new_config_settings = {}
 

+     for arg in config_settings:
 

+         key, _, value = arg.partition('=')
 

+         if key in new_config_settings:
 

+             if not isinstance(new_config_settings[key], list):
 

+                 # convert the existing value to a list
 

+                 new_config_settings[key] = [new_config_settings[key]]
 

+             new_config_settings[key].append(value)
 

+         else:
 

+             new_config_settings[key] = value
 

+     return new_config_settings
 

+ 

+ 

+ def get_config_settings_args(config_settings):
 

+     """
 

+     Given a dictionary of PEP 517 backend config_settings,
 

+     yield --config-settings args that can be passed to pip's CLI
 

+     """
 

+     if not config_settings:
 

+         return
 

+     for key, values in config_settings.items():
 

+         if not isinstance(values, list):
 

+             values = [values]
 

+         for value in values:
 

+             if value == '':
 

+                 yield f'--config-settings={key}'
 

+             else:
 

+                 yield f'--config-settings={key}={value}'
 

+ 

+ 

+ def build_wheel(*, wheeldir, stdout=None, config_settings=None):
 

      command = (
 

          sys.executable,
 

          '-m', 'pip',
 
@@ -15,11 +53,26 @@ 
 

          '--no-clean',
 

          '--progress-bar', 'off',
 

          '--verbose',
 

+         *get_config_settings_args(config_settings),
 

          '.',
 

      )
 

      cp = subprocess.run(command, stdout=stdout)
 

      return cp.returncode
 

  

  

+ def parse_args(argv=None):
 

+     parser = argparse.ArgumentParser(prog='%pyproject_wheel')
 

+     parser.add_argument('wheeldir', help=argparse.SUPPRESS)
 

+     parser.add_argument(
 

+         '-C',
 

+         dest='config_settings',
 

+         action='append',
 

+         help='Configuration settings to pass to the PEP 517 backend',
 

+     )
 

+     args = parser.parse_args(argv)
 

+     args.config_settings = parse_config_settings_args(args.config_settings)
 

+     return args
 

+ 

+ 

  if __name__ == '__main__':
 

-     sys.exit(build_wheel(wheeldir=sys.argv[1]))
 

+     sys.exit(build_wheel(**vars(parse_args())))

file modified
+1 
@@ -63,6 +63,7 @@ 
 

              requirement_files=requirement_files,
 

              use_build_system=use_build_system,
 

              output=output,
 

+             config_settings=case.get('config_settings'),
 

          )
 

      except SystemExit as e:
 

          assert e.code == case['result']

file added
+50 
@@ -0,0 +1,50 @@ 
 

+ Name:           config-settings-test
 

+ Version:        1.0.0
 

+ Release:        1%{?dist}
 

+ Summary:        Test config_settings support
 

+ 

+ License:        MIT
 

+ URL:            ...
 

+ Source0:        config_settings_test_backend.py
 

+ 

+ 

+ %description
 

+ %{summary}.
 

+ 

+ 

+ %prep
 

+ %autosetup -cT
 

+ 

+ cp -p %{sources} .
 

+ 

+ cat <<'EOF' >config_settings.py
 

+ """
 

+ This is a test package
 

+ """
 

+ EOF
 

+ 

+ cat <<'EOF' >pyproject.toml
 

+ [build-system]
 

+ build-backend = "config_settings_test_backend"
 

+ backend-path = ["."]
 

+ requires = ["flit-core", "packaging", "pip"]
 

+ 

+ [project]
 

+ name = "config_settings"
 

+ version = "%{version}"
 

+ dynamic = ["description"]
 

+ EOF
 

+ 

+ 

+ %generate_buildrequires
 

+ %pyproject_buildrequires -C abc=123 -C xyz=456 -C--option-with-dashes=1 -C--option-with-dashes=2
 

+ %{!?el9:%pyproject_buildrequires -C abc=123 -C xyz=456 -C--option-with-dashes=1 -C--option-with-dashes=2 -w}
 

+ 

+ 

+ %build
 

+ %{!?el9:%pyproject_wheel -C abc=123 -C xyz=456 -C--option-with-dashes=1 -C--option-with-dashes=2}
 

+ 

+ 

+ %changelog
 

+ * Fri May 19 2023 Maxwell G <maxwell@gtmx.me>
 

+ - Initial package

file added
+40 
@@ -0,0 +1,40 @@ 
 

+ """
 

+ This is a test backend for pyproject-rpm-macros' integration tests
 

+ It is not compliant with PEP 517 and omits some required hooks.
 

+ """
 

+ 

+ from flit_core import buildapi
 

+ from packaging.version import parse
 

+ from pip import __version__ as pip_version
 

+ 

+ EXPECTED_CONFIG_SETTINGS = [{"abc": "123", "xyz": "456", "--option-with-dashes": ["1", "2"]}]
 

+ # Older pip did not accept multiple values,
 

+ # but we might backport that later,
 

+ # hence we accept it both ways with older pips
 

+ if parse(pip_version) < parse("23.1"):
 

+     EXPECTED_CONFIG_SETTINGS.append(
 

+         EXPECTED_CONFIG_SETTINGS[0] | {"--option-with-dashes": "2"}
 

+     )
 

+ 

+ 

+ def _verify_config_settings(config_settings):
 

+     print(f"config_settings={config_settings}")
 

+     if config_settings not in EXPECTED_CONFIG_SETTINGS:
 

+         raise ValueError(
 

+             f"{config_settings!r} does not match expected {EXPECTED_CONFIG_SETTINGS!r}"
 

+         )
 

+ 

+ 

+ def build_wheel(wheel_directory, config_settings=None, metadata_directory=None):
 

+     _verify_config_settings(config_settings)
 

+     return buildapi.build_wheel(wheel_directory, None, metadata_directory)
 

+ 

+ 

+ def get_requires_for_build_wheel(config_settings=None):
 

+     _verify_config_settings(config_settings)
 

+     return buildapi.get_requires_for_build_wheel(None)
 

+ 

+ 

+ def prepare_metadata_for_build_wheel(metadata_directory, config_settings=None):
 

+     _verify_config_settings(config_settings)
 

+     return buildapi.prepare_metadata_for_build_wheel(metadata_directory, None)

file modified
+3 
@@ -94,6 +94,9 @@ 
 

      - escape_percentages:
 

          dir: .
 

          run: ./mocktest.sh escape_percentages
 

+     - config-settings-test:
 

+         dir: .
 

+         run: ./mocktest.sh config-settings-test
 

      required_packages:
 

      - mock
 

      - rpmdevtools

rebased onto a924da07e8206e776aa3179ad1e2290719552de6
a year ago

rebased onto 14fc9738cc6e41f8f9e1e2786fad76a4f99c9074
a year ago

rebased onto af517856719b08f5589f3ab1d6d1e8072027effe
a year ago

Hmm, I'm not sure why zuul didn't trigger

recheck

rebased onto 81d02f56abdd71e8b0ad80ccbb666f0602bd08fc
a year ago

recheck

What does - actually mean? I've never seen it. is it supported in RHEL 9?

What does - actually mean? I've never seen it. is it supported in RHEL 9?

Using - for the opts field disables macro option processing which we don't need here as pyproject_wheel.py processes the arguments itself. This makes it possible to e.g. write macros that accept --long-option flags. Originally, I was going to have %pyproject_wheel accept both -C and --config-settings, but since %pyproject_buildrequires uses macro option processing that is incompatible with long options, I figured I'd stay consistent and only accept -C for both of them.

Looking at https://github.com/rpm-software-management/rpm/pull/1392, it seems this feature is only available in RPM 4.17 and onward so not RHEL 9 :(. I guess I'll change it to %pyproject_wheel(C:) in that case.

Hmm, I'm not sure why zuul didn't trigger

I submitted https://pagure.io/fedora-ci/general/issue/410

recheck

recheck

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/6ae6cd70928141d498532acba7ed10b4

Thanks for updating pyproject-srpm-macros. Note to self: Add a test that ensures the two declarations are in sync.
Edited a year ago by churchyard

Consistency nitpick: Other macros use %{?**} instead of %**. I am not sure the question mark is necessary, but at least add the {} please.

Let's add a useful help message here.

The expected newline surprised me. Could you return ["test-no-config-setting"] and expect that instead?

Thanks again for working on this.

I have some opinionated feedback but I am happy to provide the changes myself if you don't want to. Let me know.

Thanks for updating pyproject-srpm-macros. Note to self: Add a test that ensures the two declarations are in sync.

That would be helpful. I missed the comment reminding people to update it. I only figured it out, as I left out the Requires: python3-devel in the integration test specfile to use the new pyproject-srpm-macros feature and spent a while trying to figure out why the SRPM build failed 😅.

Consistency nitpick: Other macros use %{?**} instead of %**. I am not sure the question mark is necessary, but at least add the {} please.

Done. I amended the first commit.

Let's add a useful help message here.

Done in a separate commit. Let me know if you're happy with the help text. I'll squash before merging.

The expected newline surprised me. Could you return ["test-no-config-setting"] and expect that instead?

Done in a separate commit. I'll squash before merging.

Thanks again for working on this.

Sure!

I have some opinionated feedback but I am happy to provide the changes myself if you don't want to. Let me know.

Feel free to give me your opinionated feedback :).

rebased onto c53247a161569cd0dc99496212bc7b33e9cc2a8e
a year ago

Thanks for updating pyproject-srpm-macros. Note to self: Add a test that ensures the two declarations are in sync.

That would be helpful. I missed the comment reminding people to update it. I only figured it out, as I left out the Requires: python3-devel in the integration test specfile to use the new pyproject-srpm-macros feature and spent a while trying to figure out why the SRPM build failed 😅.

https://src.fedoraproject.org/rpms/pyproject-rpm-macros/pull-request/387

Let's add a useful help message here.

Done in a separate commit. Let me know if you're happy with the help text. I'll squash before merging.

I am happy with the help text. I had a thought about DRY but importing the argparser argument from pyproject_wheel is probably overkill.

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/1db344b6573b4c8da710fec9dfe7acc3

I am happy with the help text. I had a thought about DRY but importing the argparser argument from pyproject_wheel is probably overkill.

I had the same exact thought and then also decided it was overkill :).

I have some opinionated feedback...

I meant: I have provided some opinionated feedback... :)

Open question: Should %pyproject_buildrequires -C ... fail without -w?

I have some opinionated feedback...

I meant: I have provided some opinionated feedback... :)

Oh, I thought you meant you had more feedback :).

Open question: Should %pyproject_buildrequires -C ... fail without -w?

The config settings are also passed to get_requires_for_build_wheel and prepare_metadata_for_build_wheel.

Open question: Should %pyproject_buildrequires -C ... fail without -w?

The config settings are also passed to get_requires_for_build_wheel and prepare_metadata_for_build_wheel.

Oh, OK. It should definitively not fail then.

Next step: Adjust the README: Describe the new option and explain when it is required to pass it to %pyproject_buildrequires, when just to %pyproject_wheel, and when to both.
Edited a year ago by churchyard

rebased onto 924cb3b618bf1afd52b7ce2d950e5c71369eb86d
a year ago

I rebased and squashed the PR.

Next step: Adjust the README: Describe the new option

I'll try to work on it later today. If you have something in mind, feel free to push it to my fork.

and explain when it is required to pass it to %pyproject_buildrequires, when just to %pyproject_wheel, and when to both.

That really depends on the build backend.

See the note about passing repeated arguments in https://meson-python.readthedocs.io/en/latest/how-to-guides/config-settings.html#how-to-guides-config-settings and https://github.com/pypa/pip/issues/11681 / https://github.com/pypa/pip/pull/11853.

We have to figure out how to handle this. We can concatenate repeated arguments for the pyproject_buildrequires code but the behavior will be inconsistent with %pyproject_wheel, as we're just passing those arguments to pip which won't do this unless it's >= 23.1. At some point, we should consider switching to build or using something like pyproject-hooks to call the backend hooks directly.

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/af44168cc88545fc844fcc9c84c78809

recheck

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/30390d9de5784c87926289807378861f

I rebased and squashed the PR.

Next step: Adjust the README: Describe the new option

I'll try to work on it later today. If you have something in mind, feel free to push it to my fork.

I have a draft locally.

recheck

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/34b200e95fb14d23af9d73ce5c04263f

2 new commits added

  • document config_settings support
  • buildrequires: make -C and -N mutually exclusive
a year ago

rpm-sti-test : FAILURE in 25m 40s

It seems the escape_percentages test is failing. I guess something changed in rpm 4.19.

Also, I pushed a commit to document this change. In the process, I decided to make -C and -N mutually exclusive.

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/eb436418b07342818d917d6e9c2b3842

rpm-sti-test : FAILURE in 25m 40s

It seems the escape_percentages test is failing. I guess something changed in rpm 4.19.

Awesome. Good that we added the test.

There is a superfluous space after the colon.

I like the documentation, thanks!

3 new commits added

  • document config_settings support
  • buildrequires: make -C and -N mutually exclusive
  • Allow passing config_settings to the build backend
a year ago

There is a superfluous space after the colon.

Fixed!

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/b19c1bec0ed043d38cc33bf658f5b4a1

Thanks. I plan to merge this together with the other PRs after the weekend.

Cool! Would you like me to squash anything or are the three commits okay?

Separate commits are fine, their scope is well-defined.

The documentation in terms of PEP 517 is clear enough, but it’s not obvious to me—without experimentation or a lot of reading—whether this should be useful for packages that are based on setuptools (or perhaps distutils) and are designed to rely on ad-hoc command-line arguments passed to python3 setup.py build.

I’m talking of the case that would look like this under the “old guidelines”: 

%py3_build -- --do-not-log-the-rainforest

This depends on which setuptools version you're using. Unfortunately, stopping rain forest logging isn't so easy...

Before v63 (f37 and below),

%pyproject_wheel -C--global-option=--do-not-log-the-rainforest

will do what you want here.

In v63, setuptools added --build-option and deprecated [1,2] --global-option: 

%pyproject_wheel -C--build-option=--do-not-log-the-rainforest

[1] https://github.com/pypa/setuptools/commit/66538d8cee51e1adee5b489e6e16429ac0ce239a

[2] The deprecation warning says:

        ********************************************************************************
        The arguments ['--do-not-log-the-rainforest'] were given via `--global-option`.
        Please use `--build-option` instead,
        `--global-option` is reserved for flags like `--verbose` or `--quiet`.

        By 2023-Sep-26, you need to update your project and remove deprecated calls
        or your builds will no longer be supported.
        ********************************************************************************

Also, it seems RHEL 9's pip wheel doesn't support --config-settings at all.

RHEL 9 is not getting any younger and I suspect keeping the macros in sync will eventually be harder and harder.

See also https://github.com/pypa/pip/issues/11681 -- we probably want to do the same. WDYT?

1 new commit added

  • Accept multiple values for the same config settings
a year ago

See also https://github.com/pypa/pip/issues/11681 -- we probably want to do the same. WDYT?

I've pushed an initial attempt, totally untested.

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/923b2ffc66db46a682caef8342d57fce

I've opened https://gitlab.com/redhat/centos-stream/rpms/pyproject-rpm-macros/-/merge_requests/18 for c9s. Once it passes the CI I plan to test the open PRs from here there including this one. Depending on the behavior of pip in RHEL 9 we might document it in the README.

See also https://github.com/pypa/pip/issues/11681 -- we probably want to do the same. WDYT?

See https://src.fedoraproject.org/rpms/pyproject-rpm-macros/pull-request/385#comment-143554. The tests you added will fail on every release except rawhide when pyproject_wheel is called, as older pip versions convert --config-settings=--option-with-dashes=1 --config-settings=--option-with-dashes=2 to {'--option-with-dashes: '2'} instead of {'option-with-dashes': ['1', '2']}.
Edited a year ago by gotmax23

See the note about passing repeated arguments in https://meson-python.readthedocs.io/en/latest/how-to-guides/config-settings.html#how-to-guides-config-settings and https://github.com/pypa/pip/issues/11681 / https://github.com/pypa/pip/pull/11853.

We have to figure out how to handle this. We can concatenate repeated arguments for the pyproject_buildrequires code but the behavior will be inconsistent with %pyproject_wheel, as we're just passing those arguments to pip which won't do this unless it's >= 23.1. At some point, we should consider switching to build or using something like pyproject-hooks to call the backend hooks directly.

Oh, I have not seen this comment. Sorry about that. I think we can adjust the test and consider this a problem of pip:

  • on RHEL 9 w/Python 3.9, pip won't handle this at all
  • on Fedora 38, pip will not handle multiple values for one key

This is something that I can live with -- assuming we document it.

And, if folks need it, backporting the multiple values thing seems trivial (unlike backporting the entire support to RHEL 9, unfortunately).
Edited a year ago by churchyard

1 new commit added

  • fixup! Accept multiple values for the same config settings
a year ago

5 new commits added

  • fixup! Accept multiple values for the same config settings
  • Accept multiple values for the same config settings
  • document config_settings support
  • buildrequires: make -C and -N mutually exclusive
  • Allow passing config_settings to the build backend
a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/6bc4b421359d46a49a405bbfe4492080

And, if folks need it, backporting the multiple values thing seems trivial (unlike backporting the entire support to RHEL 9, unfortunately).

I opened https://src.fedoraproject.org/rpms/python-pip/pull-request/124 as an RfC.

I also opened https://src.fedoraproject.org/rpms/pyproject-rpm-macros/pull-request/394 against f38 so we could see CI results.

We can discuss tomorrow.

5 new commits added

  • fixup! Accept multiple values for the same config settings
  • Accept multiple values for the same config settings
  • document config_settings support
  • buildrequires: make -C and -N mutually exclusive
  • Allow passing config_settings to the build backend
a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/fb6e1986aa4542c8b1efe3b74e14ca1d

5 new commits added

  • fixup! Accept multiple values for the same config settings
  • Accept multiple values for the same config settings
  • document config_settings support
  • buildrequires: make -C and -N mutually exclusive
  • Allow passing config_settings to the build backend
a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/eaa7dad978d54f038a82f09ae533400e

5 new commits added

  • fixup! Accept multiple values for the same config settings
  • Accept multiple values for the same config settings
  • document config_settings support
  • buildrequires: make -C and -N mutually exclusive
  • Allow passing config_settings to the build backend
a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/416762f4c5f84a4a80eb25344424892f

rebased onto 156e2fc
a year ago

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/a05b1c79b3a347db9de6931dee541d55

1 new commit added

  • fixup! Accept multiple values for the same config settings
a year ago

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/dccc9a9c7565471997a7f4db06be558e

4 new commits added

  • Accept multiple values for the same config settings
  • document config_settings support
  • buildrequires: make -C and -N mutually exclusive
  • Allow passing config_settings to the build backend
a year ago

4 new commits added

  • Accept multiple values for the same config settings
  • document config_settings support
  • buildrequires: make -C and -N mutually exclusive
  • Allow passing config_settings to the build backend
a year ago

@gotmax23 I think this is ready. Final ack?

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/a49311950e3c4ac4b3634ad962754ed3
  • On some systems (noticeably on RHEL 9 with Python 3.9),

s/noticeably/notably/

:white_check_mark:

Other than that, this looks good to me. Thanks!

4 new commits added

  • Accept multiple values for the same config settings
  • document config_settings support
  • buildrequires: make -C and -N mutually exclusive
  • Allow passing config_settings to the build backend
a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/ef35abe803cf4a2c897901162cc2a313

[citest]

recheck

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/fb17061e37694ab4a8a392e7809101b1

Pull-Request has been merged by churchyard
a year ago
Metadata
None
No Tags
Changes Summary 11
+37 -0
file changed
README.md
+1 -1
file changed
macros.aaa-pyproject-srpm
+4 -3
file changed
macros.pyproject
+5 -1
file changed
pyproject-rpm-macros.spec
+20 -6
file changed
pyproject_buildrequires.py
+37 -0
file changed
pyproject_buildrequires_testcases.yaml
+55 -2
file changed
pyproject_wheel.py
+1 -0
file changed
test_pyproject_buildrequires.py
+50
file added
tests/config-settings-test.spec
+40
file added
tests/config_settings_test_backend.py
+3 -0
file changed
tests/tests.yml
Powered by Pagure 5.13.3
Documentation • File an Issue • About this Instance • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.