Bug 2107403 - Review Request: python3-azure-nspkg - Microsoft Azure Namespace Package [Internal]
Summary: Review Request: python3-azure-nspkg - Microsoft Azure Namespace Package [Inte...
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2022-07-14 22:34 UTC by rj.layco
Modified: 2023-08-22 00:45 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2023-08-22 00:45:32 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description rj.layco 2022-07-14 22:34:34 UTC
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-1.fc35.src.rpm
Description: This is the Microsoft Azure namespace package.
Fedora Account System Username: rommell

Comment 1 rj.layco 2022-07-15 00:14:44 UTC
Koji Build: https://koji.fedoraproject.org/koji/taskinfo?taskID=89521446

Comment 2 Ben Beasley 2022-07-15 12:59:38 UTC
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.

Comment 3 Major Hayden 🤠 2022-07-15 13:11:06 UTC
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#_93

Comment 4 rj.layco 2022-07-17 21:37:34 UTC
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.

Comment 5 Ben Beasley 2022-07-22 19:54:17 UTC
(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/b24f494eca22e0389ac2e81b1b734453f187b69c95f039aa202f6f798b84/%{pypi_name}-%{version}.zip

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-unversioned-provides

-----

> %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_sections

-----

The egg-info directory appears to be listed twice in the %files section.

-----

I hope that was helpful.

Comment 6 Ben Beasley 2022-07-22 19:57:55 UTC
My suggestion

> Source0:        %{pypi_source %{pypi_name}}

should have been

> Source0:        %{pypi_source %{pypi_name} %{version} zip}

Comment 7 Package Review 2023-07-23 00:45:24 UTC
This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

Comment 8 Package Review 2023-08-22 00:45:32 UTC
This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.


Note You need to log in before you can comment on or make changes to this bug.