fedora-copr / copr

RPM build system - upstream for https://copr.fedorainfracloud.org/
108 stars 56 forks source link

PyPI source packages using pyp2spec should allow -l override #3278

Closed brianjmurrell closed 1 month ago

brianjmurrell commented 1 month ago

When creating a PyPI source package and choosing pyp2spec as the spec generator, one should be able to supply a license override per pyp2spec's -l option.

This allows for one to continue to create and build one's package without pyp2spec failing the build due to an invalid license specification.

Yes, most ideal is to get the license specification fixed upstream but that may be delayed (it's a relatively minor update which most maintainers likely will not make a release just to satisfy) or not even happen if the upstream maintainer does not care as much about a perfect license specification as pyp2spec (and by extension COPR) does.

praiskup commented 1 month ago

Thank you for the report.

praiskup commented 1 month ago

@befeleme do you think this is a valid reason to fail builds in Copr? Could we fix pyp2spec instead?

befeleme commented 1 month ago

The license check is invoked with --fedora-compliant option which extends the Copr usage guidelines (https://docs.pagure.org/copr.copr/user_documentation.html#what-i-can-build-in-copr).

If we generally assume that users are responsible for making sure they are allowed to build the software, this line could be removed: https://github.com/fedora-copr/copr/blob/8de7a6906733ef63f5269922fcc51f388a55d444/rpmbuild/copr_rpmbuild/providers/pypi.py#L58

This will, however, allow to build in Copr packages that clearly state proprietary licenses.

brianjmurrell commented 1 month ago

I don't think that's the extent of it. I get the SPDX license error from py2spec even without --fedora-compliant and it still requires the -l argument.

praiskup commented 1 month ago

Team's triage time: It seems that we could drop the --fedora-compliant option by default; the license validation mechanism is neat, but it is only for the PyPI method anyway -> we'll need something more generic (@xsuchy is going to fill a new issue for that).

xsuchy commented 1 month ago

We will solve the compliance of license in https://github.com/fedora-copr/copr/issues/3290

brianjmurrell commented 1 month ago

As mentioned before simply dropping --fedora-compliant is not enough:

$ pyp2spec manhole --fedora-compliant --top-level
Generating configuration file
Assuming 'manhole' is a package name
Querying PyPI for package 'manhole'
Assuming --description=This is package 'manhole' generated automatically by pyp2spec.
Assuming --summary=Manhole is in-process service that will accept unix domain socket connections and present the
Assuming PyPI --version=1.8.0
Fatal exception occurred: Invalid SPDX expression: BSD 2-Clause License
$ pyp2spec manhole --top-level
Generating configuration file
Assuming 'manhole' is a package name
Querying PyPI for package 'manhole'
Assuming --description=This is package 'manhole' generated automatically by pyp2spec.
Assuming --summary=Manhole is in-process service that will accept unix domain socket connections and present the
Assuming PyPI --version=1.8.0
Fatal exception occurred: Invalid SPDX expression: BSD 2-Clause License

As you see above, it still fails even without --fedora-compliant and only works when using -l <SPDX-license>:

$ pyp2spec manhole --top-level -l 'BSD-2-Clause'
Generating configuration file
Assuming 'manhole' is a package name
Querying PyPI for package 'manhole'
Assuming --description=This is package 'manhole' generated automatically by pyp2spec.
Assuming --summary=Manhole is in-process service that will accept unix domain socket connections and present the
Assuming PyPI --version=1.8.0
Only top-level modules will be checked
Saving configuration file to './python-manhole.conf'
Configuration file was saved successfully
Generating spec file
Spec file was saved successfully to 'python-manhole.spec'
Done

The value for -l is going to have to come from the user (or some magical AI transformation of non-SPDX license expressions to SPDX license values) when they create their PyPI (source) package on COPR.

befeleme commented 1 month ago

I see, thank you for the detailed explanation.

Depending on what the actual request is, I see two ways forward:

image

brianjmurrell commented 1 month ago
  • on the Copr side: Is it possible (and sensible) for Copr to add a similar field like those for pyp2rpm, but for a license expression? It would translate to -l added to the pyp2spec invocation.

Indeed. That is the request in the original description/comment of this ticket. :smile:

  • on pyp2spec's side: the detection of an invalid license will not be terminating the application run, it'll just warn the user but still let producing a specfile. This is more in line with the focusing on human user which is the next step for pyp2spec when I have free cycles. Once we deploy it to Copr, the automated PyPI repo will start creating much more RPMs.

This would be sweet. I am for sure looking forward to the day that the PyPI repo starts creating many more RPMs!

Dealing with the list of packages that fail to build and working with their upstream to get them fixed would be much less daunting a task.

xsuchy commented 1 month ago

on the Copr side: Is it possible (and sensible) for Copr to add a similar field like those for pyp2rpm, but for a license expression? It would translate to -l added to the pyp2spec invocation.

I do not like this option. As that would complicate the UI.

on pyp2spec's side: the detection of an invalid license will not be terminating the application run, it'll just warn the user but still let producing a specfile. This is more in line with the focusing on human user which is the next step for pyp2spec when I have free cycles. Once we deploy it to Copr, the automated PyPI repo will start creating much more RPMs.

I prefer this one. I will open an issue for pyp2spec... oh Karolina already made it. https://github.com/befeleme/pyp2spec/issues/47