https://bugzilla.redhat.com/show_bug.cgi?id=2107403
Bug ID: 2107403 Summary: Review Request: python3-azure-nspkg - Microsoft Azure Namespace Package [Internal] Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: rj.layco@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://rommell.fedorapeople.org/python-azure-nspkg/python-azure-nspkg.spec SRPM URL: https://rommell.fedorapeople.org/python-azure-nspkg/python-azure-nspkg-3.0.2... Description: This is the Microsoft Azure namespace package. Fedora Account System Username: rommell
https://bugzilla.redhat.com/show_bug.cgi?id=2107403
rj.layco@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Doc Type|--- |If docs needed, set a value
--- Comment #1 from rj.layco@gmail.com --- Koji Build: https://koji.fedoraproject.org/koji/taskinfo?taskID=89521446
https://bugzilla.redhat.com/show_bug.cgi?id=2107403
Ben Beasley code@musicinmybrain.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |code@musicinmybrain.net, | |mhayden@redhat.com
--- Comment #2 from Ben Beasley code@musicinmybrain.net --- There are a few issues with the packaging—obsolete practices and unnecessary workarounds—but I want to ask about:
Since version 3.0, this is Python 2 package only, Python 3.x SDKs will use PEP420 as namespace package strategy. To avoid issues with package servers that does not support python_requires, a Python 3 package is installed but is empty.
It seems like this package doesn’t really do anything in Fedora, except providing python3dist(azure-nspkg). Is there something that depends on that explicitly, and if so, would it be better to just patch out the dependency in that package, since Fedora can rely on a modern Python and understands Python dependencies?
If you confirm it makes sense to package this, can I ask if you are targeting EPEL7 or EPEL8 with the same spec file? Or just Fedora and maybe EPEL9? The answer would affect the advice about which obsolete practices to remove.
https://bugzilla.redhat.com/show_bug.cgi?id=2107403
--- Comment #3 from Major Hayden 🤠 mhayden@redhat.com --- Ben is on the right track here. The -nspkg packages really aren't needed any more in modern python. I've spoken with upstream about it, but they're not eager to remove these (yet). Perhaps this is related to the work they did very recently to drop support for some older Python versions in their code and maybe we will see these nspkg packages dropped soon.
Either way, patching out the requirement is what I would recommend. I handle it in azure-cli[0] that way. Is there a particular package from Azure where you have a problem? I'm packaging about 90-100 packages related to azure-cli and their Python SDK at the moment.
[0] https://src.fedoraproject.org/rpms/azure-cli/blob/rawhide/f/azure-cli.spec#_...
https://bugzilla.redhat.com/show_bug.cgi?id=2107403
--- Comment #4 from rj.layco@gmail.com --- Yeah i was packaging this so I could package azure-storage-blob. I will patch it instead.
@Ben Beasley. What are the obsolete practices and unnecessary workarounds, so I can avoid them in my future spec files. I created the spec by running the command `rpmdev-newspec` and using the generated spec file as base.
https://bugzilla.redhat.com/show_bug.cgi?id=2107403
--- Comment #5 from Ben Beasley code@musicinmybrain.net --- (In reply to rj.layco from comment #4)
Yeah i was packaging this so I could package azure-storage-blob. I will patch it instead.
@Ben Beasley. What are the obsolete practices and unnecessary workarounds, so I can avoid them in my future spec files. I created the spec by running the command `rpmdev-newspec` and using the generated spec file as base.
%{?!python3_pkgversion:%global python3_pkgversion 3}
The python3_pkgversion macro is an EPEL-ism, and it is always defined in current EPEL and Fedora releases, so this line is not useful even if you are trying to support EPEL7/8 with the same spec file.
-----
Source0: https://files.pythonhosted.org/packages/39/31/b24f494eca22e0389ac2e81b1b7344...
This can and should be written:
Source0: %{pypi_source %{pypi_name}}
-----
BuildRequires: python%{python3_pkgversion}-devel BuildRequires: python%{python3_pkgversion}-setuptools
If you are packaging under the old Python guidelines (https://docs.fedoraproject.org/en-US/packaging-guidelines/Python_201x/), perhaps because you want to support EPEL7 or EPEL8 with the same spec file, this is fine. Otherwise, if you are just packaging for Fedora and maybe EPEL9, consider using the new Python guidelines (https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/). You can just write
BuildRequires: python3-devel
and, after %prep to reflect when it happens, add
%generate_buildrequires %pyproject_buildrequires
-----
%{?python_enable_dependency_generator}
The Python dependency generator has been enabled by default since RHEL8 and is unavailable in RHEL7, so this is useless.
-----
Consider using a macro for the description, like:
%global _description %{expand: This is the Microsoft Azure namespace package. … blah blah … package}
and then
%description %{_description}
and
%description -n python%{python3_pkgversion}-%{pypi_name} %{_description}
-----
%{?python_provide:%python_provide python3-%{pypi_name}}
This is unnecessary unless you are trying to target EPEL7 or EPEL8 with the same spec file. Even then, consider %py_provides instead—but otherwise, you can just delete this line.
https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#Automatic-...
-----
%if %{undefined python_enable_dependency_generator} && %{undefined python_disable_dependency_generator} # Put manual requires here: Requires: python%{python3_pkgversion}-foo %endif
All of that can be deleted unless you are trying to package for EPEL7 with the same spec file. Plus, you just have a bogus placeholder here anyway.
-----
rm -rf $RPM_BUILD_ROOT
This is unnecessary, and the guidelines advise against it: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_section...
-----
The egg-info directory appears to be listed twice in the %files section.
-----
I hope that was helpful.
https://bugzilla.redhat.com/show_bug.cgi?id=2107403
--- Comment #6 from Ben Beasley code@musicinmybrain.net --- My suggestion
Source0: %{pypi_source %{pypi_name}}
should have been
Source0: %{pypi_source %{pypi_name} %{version} zip}
package-review@lists.fedoraproject.org