#422 Add ability to assert License-File in %pyproject_save_files
Merged 5 months ago by churchyard. Opened 8 months ago by churchyard.
rpms/ churchyard/pyproject-rpm-macros assert_license  into  rawhide

file modified
+6
@@ -288,6 +288,12 @@ 

  and  language (`*.mo`) files with `%lang` macro and appropriate language code.

  Only license files declared via [PEP 639] `License-File` field are detected.

  [PEP 639] is still a draft and can be changed in the future.

+ It is possible to use the `-l` flag to declare that a missing license should

+ terminate the build or `-L` (the default) to explicitly disable this check.

+ Packagers are encouraged to use the `-l` flag when the `%license` file is not manually listed in `%files`

+ to avoid accidentally losing the file in a future version.

+ When the `%license` file is manually listed in `%files`,

+ packagers can use the `-L` flag to ensure future compatibility in case the `-l` behavior eventually becomes a default.

  

  Note that `%pyproject_save_files` uses data from the [RECORD file](https://www.python.org/dev/peps/pep-0627/).

  If you wish to rename, remove or otherwise change the installed files of a package

file modified
+2 -2
@@ -108,7 +108,7 @@ 

  # See this thread http://lists.rpm.org/pipermail/rpm-list/2021-June/002048.html

  # Since RPM 4.19, 2 signs are needed instead. 4.18.90+ is a pre-release of RPM 4.19.

  # On the CI, we build tests/escape_percentages.spec to verify the assumptions.

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

+ %pyproject_save_files(lL) %{expand:\\\

  %{expr:v"0%{?rpmversion}" >= v"4.18.90" ? "RPM_PERCENTAGES_COUNT=2" : "RPM_PERCENTAGES_COUNT=8" } \\

  %{__python3} %{_rpmconfigdir}/redhat/pyproject_save_files.py \\

    --output-files "%{pyproject_files}" \\
@@ -119,7 +119,7 @@ 

    --python-version "%{python3_version}" \\

    --pyproject-record "%{_pyproject_record}" \\

    --prefix "%{_prefix}" \\

-   %{*}

+   %{**}

  }

  

  # -t - Process only top-level modules

file modified
+6 -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.10.0

+ Version:        1.11.0

  Release:        1%{?dist}

  

  # Macro files
@@ -171,6 +171,11 @@ 

  

  

  %changelog

+ * Wed Sep 27 2023 Miro Hrončok <mhroncok@redhat.com> - 1.11.0-1

+ - Add the -l/-L flag to %%pyproject_save_files

+ - The -l flag can be used to assert at least 1 License-File was detected

+ - The -L flag explicitly disables this check (which remains the default)

+ 

  * Wed Sep 13 2023 Python Maint <python-maint@redhat.com> - 1.10.0-1

  - Add %%_pyproject_check_import_allow_no_modules for automated environments

  - Fix handling of tox 4 provision without an explicit tox minversion

file modified
+27 -2
@@ -693,12 +693,15 @@ 

      return dist.metadata

  

  

- def pyproject_save_files_and_modules(buildroot, sitelib, sitearch, python_version, pyproject_record, prefix, varargs):

+ def pyproject_save_files_and_modules(buildroot, sitelib, sitearch, python_version, pyproject_record, prefix, assert_license, varargs):

      """

      Takes arguments from the %{pyproject_save_files} macro

  

      Returns tuple: list of paths for the %files section and list of module names

      for the %check section

+ 

+     Raises ValueError when assert_license is true and no License-File (PEP 639)

+     is found.

      """

      # On 32 bit architectures, sitelib equals to sitearch

      # This saves us browsing one directory twice
@@ -710,11 +713,15 @@ 

      final_file_list = []

      final_module_list = []

  

+     # we assume OK when not asserting

+     license_ok = not assert_license

+ 

      for record_path, files in parsed_records.items():

          metadata = dist_metadata(buildroot, record_path)

          paths_dict = classify_paths(

              record_path, files, metadata, sitedirs, python_version, prefix

          )

+         license_ok = license_ok or bool(paths_dict["metadata"]["licenses"])

  

          final_file_list.extend(

              generate_file_list(paths_dict, globs, include_auto)
@@ -723,6 +730,15 @@ 

              generate_module_list(paths_dict, globs)

          )

  

+     if not license_ok:

+         raise ValueError(

+             "No License-File (PEP 639) in upstream metadata found. "

+             "Adjust the upstream metadata "

+             "if the project's build backend supports PEP 639 "

+             "or use `%pyproject_save_files -L` "

+             "and include the %license file in %files manually."

+         )

+ 

      return final_file_list, final_module_list

  

  
@@ -734,6 +750,7 @@ 

          cli_args.python_version,

          cli_args.pyproject_record,

          cli_args.prefix,

+         cli_args.assert_license,

          cli_args.varargs,

      )

  
@@ -747,7 +764,7 @@ 

          prog="%pyproject_save_files",

          add_help=False,

          # custom usage to add +auto

-         usage="%(prog)s MODULE_GLOB [MODULE_GLOB ...] [+auto]",

+         usage="%(prog)s  [-l|-L] MODULE_GLOB [MODULE_GLOB ...] [+auto]",

      )

      parser.add_argument(

          '--help', action='help',
@@ -764,6 +781,14 @@ 

      r.add_argument("--pyproject-record", type=PosixPath, required=True, help=argparse.SUPPRESS)

      r.add_argument("--prefix", type=PosixPath, required=True, help=argparse.SUPPRESS)

      parser.add_argument(

+         "-l", "--assert-license", action="store_true", default=False,

+         help="Fail when no License-File (PEP 639) is found.",

+     )

+     parser.add_argument(

+         "-L", "--no-assert-license", action="store_false", dest="assert_license",

+         help="Don't fail when no License-File (PEP 639) is found (the default).",

+     )

+     parser.add_argument(

          "varargs", nargs="+", metavar="MODULE_GLOB",

          help="Shell-like glob matching top-level module names to save into %%{pyproject_files}",

      )

@@ -48,7 +48,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files escape_percentages

+ %pyproject_save_files -L escape_percentages

  touch '%{buildroot}/two%%version'

  

  

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

    cp $original $config

  

    echo -e '\n\n' >> $config

+   echo -e 'config_opts["package_manager"] = "dnf"' >> $config

    echo -e 'config_opts["package_manager_max_attempts"] = 10' >> $config

    echo -e 'config_opts["package_manager_attempt_delay"] = 60' >> $config

    echo -e '\n\nconfig_opts[f"{config_opts.package_manager}.conf"] += """' >> $config

file modified
+1 -2
@@ -33,7 +33,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files printrun +auto

+ %pyproject_save_files -l printrun +auto

  

  

  %check
@@ -52,4 +52,3 @@ 

  

  %files -f %{pyproject_files}

  %doc README*

- %license COPYING

file modified
+1 -2
@@ -42,7 +42,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files distroinfo

+ %pyproject_save_files -l distroinfo

  

  

  %check
@@ -52,4 +52,3 @@ 

  

  %files -n python3-distroinfo -f %{pyproject_files}

  %doc README.rst AUTHORS

- %license LICENSE

file modified
+1 -2
@@ -40,7 +40,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files django

+ %pyproject_save_files -l django

  

  

  %check
@@ -56,6 +56,5 @@ 

  

  %files -n python3-django -f %{pyproject_files}

  %doc README.rst

- %license LICENSE

  %{_bindir}/django-admin

  %{_bindir}/django-admin.py

@@ -55,7 +55,8 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files lexicon

+ # the license is not marked as License-File by poetry-core, hence -L

+ %pyproject_save_files -L lexicon

  

  

  %check

@@ -38,7 +38,8 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files entrypoints

+ # the license is not marked as License-File, hence -L

+ %pyproject_save_files entrypoints -L

  

  

  %check

@@ -42,7 +42,13 @@ 

  

  %install

  %pyproject_install

+ # there is no license file marked as License-File, hence not using -l

  %pyproject_save_files flit_core

  

  

+ %check

+ # internal check for our macros, we assume there is no license

+ grep -F %%license %{pyproject_files} && exit 1 || true

+ 

+ 

  %files -n python3-flit-core -f %{pyproject_files}

file modified
+1 -1
@@ -40,7 +40,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files '*' +auto

+ %pyproject_save_files -l '*' +auto

  

  

  %check

file modified
+1 -2
@@ -56,7 +56,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files httpbin

+ %pyproject_save_files -l httpbin

  

  

  %if %{with tests}
@@ -72,4 +72,3 @@ 

  

  %files -n python3-httpbin -f %{pyproject_files}

  %doc README*

- %license LICENSE*

file modified
+1 -2
@@ -46,12 +46,11 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files 'ipykernel*' +auto

+ %pyproject_save_files -l 'ipykernel*' +auto

  

  %check

  %pyproject_check_import  -e '*.test*' -e 'ipykernel.gui*' -e 'ipykernel.pylab.*' -e 'ipykernel.trio*'

  

  %files -n python3-ipykernel -f %{pyproject_files}

- %license COPYING.md

  %doc README.md

  

file modified
+1 -2
@@ -41,7 +41,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files isort

+ %pyproject_save_files -l isort

  

  

  %check
@@ -55,5 +55,4 @@ 

  

  %files -n python%{python3_pkgversion}-%{modname} -f %{pyproject_files}

  %doc README.rst *.md

- %license LICENSE

  %{_bindir}/%{modname}

file modified
+1 -2
@@ -61,7 +61,7 @@ 

  %install

  %pyproject_install

  # We can pass multiple globs

- %pyproject_save_files 'ldap*' '*ldap'

+ %pyproject_save_files -l 'ldap*' '*ldap'

  

  

  %check
@@ -104,7 +104,6 @@ 

  

  

  %files -n python3-ldap -f %{pyproject_files}

- %license LICENCE

  %doc CHANGES README TODO Demo

  # Explicitly listed files can be combined with automation

  %pycached %{python3_sitearch}/ldif.py

file modified
+1 -2
@@ -26,7 +26,6 @@ 

  

  # In this spec, we put %%files early to test it still works

  %files -n python3-markupsafe -f %{pyproject_files}

- %license LICENSE.rst

  %doc CHANGES.rst README.rst

  

  
@@ -54,7 +53,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files markupsafe

+ %pyproject_save_files -l markupsafe

  

  

  %check

file modified
+1 -2
@@ -46,7 +46,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files mistune

+ %pyproject_save_files -l mistune

  

  

  %check
@@ -61,4 +61,3 @@ 

  

  %files -n python%{python3_pkgversion}-mistune -f %{pyproject_files}

  %doc README.rst

- %license LICENSE

@@ -45,7 +45,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files %{pypi_name}

+ %pyproject_save_files -l %{pypi_name}

  

  

  %check
@@ -54,4 +54,3 @@ 

  

  %files -n python3-%{pypi_name} -f %{pyproject_files}

  %doc README.*

- %license COPYING

file modified
+1 -2
@@ -43,7 +43,7 @@ 

  %install

  %pyproject_install

  # There are no executables, but we are allowed to pass +auto anyway

- %pyproject_save_files pluggy +auto

+ %pyproject_save_files pluggy +auto -l

  

  

  %check
@@ -52,4 +52,3 @@ 

  

  %files -n python3-%{pypi_name} -f %{pyproject_files}

  %doc README.rst

- %license LICENSE

@@ -37,8 +37,11 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files poetry

+ # the license is not marked as License-File by poetry-core, hence -L

+ %pyproject_save_files -L poetry

  

+ # internal check for our macros, -l must fail:

+ %pyproject_save_files -l poetry && exit 1 || true

  

  %files -n python3-poetry-core -f %{pyproject_files}

  %doc README.md

file modified
+1 -2
@@ -54,7 +54,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files '*pytest' +auto

+ %pyproject_save_files -l '*pytest' +auto

  

  

  %check
@@ -70,4 +70,3 @@ 

  %files -n python3-%{pypi_name} -f %{pyproject_files}

  %doc README.rst

  %doc CHANGELOG.rst

- %license LICENSE

file modified
+1 -1
@@ -76,7 +76,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files setuptools pkg_resources _distutils_hack

+ %pyproject_save_files setuptools pkg_resources _distutils_hack -l

  

  # https://github.com/pypa/setuptools/issues/2709

  rm -rf %{buildroot}%{python3_sitelib}/pkg_resources/tests/

@@ -52,7 +52,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files setuptools_scm

+ %pyproject_save_files -l setuptools_scm

  

  

  %check
@@ -73,4 +73,3 @@ 

  %files -n python3-setuptools_scm -f %{pyproject_files}

  %doc README.rst

  %doc CHANGELOG.rst

- %license LICENSE

file modified
+1 -1
@@ -43,7 +43,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files userpath

+ %pyproject_save_files -l userpath

  

  

  %check

file modified
+1 -1
@@ -52,7 +52,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files virtualenv

+ %pyproject_save_files -l virtualenv

  %{?el9:

  # old version of setuptools_scm produces files incompatible with

  # assumptions in virtualenv code, we append the expected attributes:

file modified
+1 -2
@@ -32,7 +32,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files zope +auto

+ %pyproject_save_files -l zope +auto

  

  %check

  # Internal check that the RECORD and REQUESTED files are
@@ -42,5 +42,4 @@ 

  

  %files -n python3-zope-event -f %{pyproject_files}

  %doc README.rst

- %license LICENSE.txt

  

file modified
+1 -2
@@ -36,7 +36,7 @@ 

  

  %install

  %pyproject_install

- %pyproject_save_files tldr +auto

+ %pyproject_save_files -l tldr +auto

  

  %check

  # Internal check for our macros: tests we don't ship __pycache__ in bindir
@@ -62,5 +62,4 @@ 

  %endif

  

  %files -f %pyproject_files

- %license LICENSE

  %doc README.md

no initial comment

rebased onto 801fbef93fd0b6df53d7f088d4b0f59c69c601d8

8 months ago

rebased onto 089e251

8 months ago

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

rebased onto 61a9cb88eb642fff54ffe33cb7ff3057993f775f

8 months ago

rebased onto 9532c2d9505a0b33024e5440f668f340f9097858

8 months ago

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

s/loosing/losing/

I haven’t tested this, but I like it.

rebased onto e498904a278083dd44116f2d0b86424b1a2f33b9

8 months ago

s/loosing/losing/

amended

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

The implementation looks good to me, but I have a couple notes/nitpicks:

  1. I pushed a fixup commit to clarify the error message.
  2. What's the purpose of adding the long arguments to the ArgumentParser if the RPM macro parser won't accept them?
  3. Are there any test specfiles that were not adjusted to use -l or -L? If not, It would be good to increase test complexity and make sure that -L is really the default.

1 new commit added

  • pyproject_save_files: clarify missing license file error
7 months ago

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

I pushed a fixup commit to clarify the error message.

Thanks!

What's the purpose of adding the long arguments to the ArgumentParser if the RPM macro parser won't accept them?

Honestly, they are useless. OTOH pyproject_save_files also has them, this just keeps things consistent. It's a habit. Perhaps if the RPM macro parser will ever support this, we will have them ready. I don't think it makes sense to remove them.

Are there any test specfiles that were not adjusted to use -l or -L?

Yes. python-flit-core.spec

If not, It would be good to increase test complexity and make sure that -L is really the default.

See the comment in flit-core:

# there is no license file marked as License-File, hence not using -l
%pyproject_save_files flit_core

This will blow up if the default changes. OTOH if we ever update the tested version, it might introduce a license file and later won't blow up. I can add an internal check to verify there is no %license generated in %{pyproject_files}.

1 new commit added

  • fixup: CI: assert flit_core has no %license
7 months ago

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

1 new commit added

  • README: clarify -l / -L usage instructions
6 months ago

What's the purpose of adding the long arguments to the ArgumentParser if the RPM macro parser won't accept them?

Honestly, they are useless. OTOH pyproject_save_files also has them, this just keeps things consistent. It's a habit.

Fair enough.

Perhaps if the RPM macro parser will ever support this, we will have them ready. I don't think it makes sense to remove them.

It already kind of does support it. If you change

- %pyproject_save_files(lL) %{expand:\\\
+ %pyproject_save_files(-) %{expand:\\\

RPM 4.17+ (i.e., not RHEL 9) will not preform getopt processing and will just pass all the arguments to the script via %{**}.

This will blow up if the default changes. OTOH if we ever update the tested version, it might introduce a license file and later won't blow up. I can add an internal check to verify there is no %license generated in %{pyproject_files}.

Cool!

I also pushed another change to clarify the README. The parentheses confused me a bit. I wish Pagure had a proper interface to suggest changes.


Other than that, I think this is ready to go.

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

rebased onto 1133eea0203d12a8943cda03e26acbde15cd5e85

5 months 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/b087fd10cb6a414c9ca417d9f6050c77

rebased onto f392619

5 months 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/aeec8156ffd34d7e8a39a299607109cc

1 new commit added

  • CI: Use DNF 4 in mock to workaround dnf5#1084
5 months ago

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

The CI looks alright, the commit msg is well-explained, it obviously works, and the fact that it's quite cryptic was raised in mock: https://github.com/rpm-software-management/mock/issues/1271 - all good by me.

Pull-Request has been merged by churchyard

5 months ago