[Bug 2107403] Review Request: python3-azure-nspkg - Microsoft Azure Namespace Package [Internal]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=2107403



--- Comment #5 from Ben Beasley <code@xxxxxxxxxxxxxxxxxx> ---
(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.


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2107403
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux