fedora-python / pyp2rpm

Tool to convert a package from PyPI to RPM SPECFILE or to generate SRPM.
MIT License
128 stars 39 forks source link

Wrong sphinx-build-3 versus sphinx-build-%{python3_version} used in epel7 template #272

Closed nkadel closed 1 year ago

nkadel commented 3 years ago

pyp2rpm has a built-in issue with what Red Hat elected to do when packaging python 3.6 as "python3". The previously used syntax of referring to the packages as "python%{python3_pkgversion}" and binary referred python programs as "pkgname-%{python_version}" breaks down for the current pyp2rpm setup for "python3-sphinx". The "python36-sphinx" package doesn't install "/usr/bin/sphinx-build-3". It installs "/usr/bin/sphinx-build-3.6".

The ongoing inconosistency breaks documentation generation for all pyp2rpm generated .spec fils unless the inconsistently named add-on wrapper "python3-sphinx" wrapper is also installed. The fix is straightforward: adjust the template for epel-7 to use "sphinx-build-%{python3_version}". I'm a bit nervous about the complexities of the existing template, or I'd offer the patch myself.

nkadel commented 3 years ago

I'm suggesting something like the patch below. I'm having some difficulty tweaking my local config, and would welcome insights as to why an edited epel7.spec file is not being used for new pyp2rpm commands

`$ diff -u epel7.spec.orig epel7.spec --- epel7.spec.orig 2021-09-19 19:20:18.424797773 -0400 +++ epel7.spec 2021-09-26 11:57:38.890409099 -0400 @@ -51,7 +51,7 @@ {%- endfor %} {%- if data.sphinx_dir %}

generate html docs

-PYTHONPATH=${PWD} {{ "sphinx-build"|script_name_for_python_version(data.base_python_version, False, False) }} {{ data.sphinx_dir }} html +PYTHONPATH=${PWD} {{ "sphinx-build-%{python{{ pv }}_version}" {{ data.sphinx_dir }} html

remove the sphinx-build leftovers

rm -rf html/.{doctrees,buildinfo} {%- endif %} `

gordonmessmer commented 3 years ago

Can you provide the full command you're using, to demonstrate the issue?

nkadel commented 3 years ago

I use mock to build RPMs from SRPMs. For the "six" module:

My working environment usually has both the "python3-sphinx" and the "python36-sphinx" packages, which is why it works for rpmbuild -ba. But mock has a much less generous default build environment.

This command fails. Snipped from the build.log at /var/lib/mock/epel-7-x86_64/result/build.log

`RPM build errors:

As you can see, it's currently calling "sphinx-build-3". This is exactly the sort of naming inconsistency I've been encountering since Red Hat elected to build "python3" packages instead of EPEL's selected "python36" package names. The "python36-sphinx" package selected by pyp2rpm installs "sphinx-build-3.6". The CentOS published "python3" module contains "spninx-build-3. If pyp2rpm is going to set the dependencies to require "python%{python3_pkgversion}-sphinx", it's going to be the EPEL RPM, and the binary will be called sphinx-build-%{python3_version}

gordonmessmer commented 3 years ago

It looks like changing the filter from script_name_for_python_version(data.base_python_version, False, False) to script_name_for_python_version(data.base_python_version, True, False) in epel7.spec should solve the problem.

nkadel commented 3 years ago

On Mon, Sep 27, 2021 at 10:48 AM Gordon Messmer @.***> wrote:

It looks like changing the filter from script_name_for_python_version(data.base_python_version, False, False) to script_name_for_python_version(data.base_python_version, True, False) in epel7.spec should solve the problem.

Can we activate that? Would you need a pull request?

gordonmessmer commented 3 years ago

I can submit the change... Have you tried that change to confirm that it fixes the issue you're reporting?

nkadel commented 3 years ago

It doesn't seem to. Neither does my suggested change. I'm at a loss as to why that edited epel7.spec template is not used?

gordonmessmer commented 2 years ago

Are you using "-t epel7"?

I've confirmed the issue that you reported, and that pyp2rpm -b 3 -t epel7 -o epel7 outputs a working spec after applying #273.

nkadel commented 2 years ago

I'd been editing the locally installed pyp2rpm directory's "epel7.spec" file in place. Needing to use both "-t" and "-o" is a bit confusing, but this worked:

pyp2rpm -t /usr/local/pyp2rpm/lib/python3.6/site-packages/pyp2rpm/templates/epel7.spec -o epel7

But that command produced a workable:

PYTHONPATH=${PWD} sphinx-build-%{python3_version} documentation html`

I'm going to assume that this is a desirable change for all the other .spec files as well.

gordonmessmer commented 2 years ago

I'm going to assume that this is a desirable change for all the other .spec files as well.

Probably not. It'd only affect the output if the default Python version for the platform were Python 2. epel6 is retired, so I don't expect to change that template. pld is still listed as a Python 2 default, but I suspect that's simply out of date.

nkadel commented 2 years ago

The particular line does not affect python 2.x, which has been consistently laid out. RHEL 6 based operating systems are not quite dead yet, and won't be dead for a few years longer due to Amazon Linux 1.

gordonmessmer commented 2 years ago

The particular line does not affect python 2.x, which has been consistently laid out

I'll rephrase my statement: sphinx-build should only require a version suffix containing a major and minor version on platforms where the default python is Python 2. If you can locate any supported platform where the mock build root installs sphinx-build with a version suffix containing a major and minor version number, and not "sphinx-build", then we can update that template as well.

RHEL 6 based operating systems are not quite dead yet, and won't be dead for a few years longer due to Amazon Linux 1.

OK, but EPEL 6 is. And, in any case, it doesn't make sense to adjust the templates haphazardly, without a reference system as a target.

nkadel commented 2 years ago

Permit me to disagree. This has nothing to do with python 2. This is a python3 packaging problem, because someone doing EPEL packaging decided to make distinct "python3-sphinx" and "python36-sphinx" packages and not include "/usr/bin/sphinx-build-3" and "/usr/bin/sphinx-build-3.6" in the same package, unlike most other RHEL "python3" and most more recent EPEL "python36" packages. I suspect it's rooted in Red Hat's decision to discard EPEL's "python36" naming scheme, and EPEL's attempts to emulate it, and that has been causing real compatibility issues with Amazon Linux 2. AMZ 2 followed Red Hat's lead, and they use "python3" packages built with python 3.7. and this is breaking EPEL provided python modules.

This approach of using the dotted version suffix works, and I appreciate the pointer to activate it correctly.

gordonmessmer commented 2 years ago

This has nothing to do with python 2.

You're missing the point. I'm explaining why the other templates don't need to be adjusted. On platforms where python 3 is the default (indicated in settings.py), the expected sphinx build command is "sphinx-build". Therefore, the only templates that could need adjustment are epel6, epel7, and pld. epel6 is deprecated, which leaves only pld as a candidate. But I suspect that if I look into pld, I'll find that they've made python3 the default, and the command will be "sphinx-build" there as well.

nkadel commented 2 years ago

I don't believe I've missed the point. Some quick tests with CentOS 8, and selecting for python 3.8, generate the same incorrect "sphinx-build-3" entry as was previously published by default for CentOS 7 by default. The idea that "python3" or "sphinx-build-3" is always the correct binary is not correct in various situations and should not be the mandated command line for .spec files. This has little to nothing to do with python 2, which doesn't have the variety of python 3.x binaries and binary names built-in.

gordonmessmer commented 2 years ago

It would be helpful if you could provide a reproduction case.

nkadel commented 2 years ago

I did, above, for the "six" module. There are two python3 based sphinx modules for RHEL 7. One provides python3-sphinx sphinx-build-3, python36-sphinx provides sphinx-build-3. Both are python 3.6 based, one is from RHEL and one is from EPEL, and Red Hat has been careless about the packaging as they rebuindled and rewroapped it, so using "python%{python3 _pkgvesion} dependencies provides a binary python-sphinx-3.6.

Inconsistent naming convections for python 3 versus python36, python38, python310, and other packages is confusing. The default suffix should always be linked to a specific python subversion, not merely "2" or "3", to avoid just this sort of conflict as python versions are installed in parallel.

gordonmessmer commented 2 years ago

As I mentioned previously, I confirmed the bug you reported (with the six module), and #273 fixes that bug, as far as I know. I believe I deferred merging it because you seemed to think the solution was incomplete.

Is there a problem with pyp2rpm after applying #273?

nkadel commented 2 years ago

Only that it's still not in a tagged version, and has to be built locally rather than using "pip3 install --user --upgrade pyp2rpm". Any chance we'll see a fresh tag submitted to pypi.org soon?

nkadel commented 2 years ago

It's also a desirable change for Fedora and other releases where the developer may use an alternative python, such as the python310 packages for RHEL 8.

nkadel commented 2 years ago

This is still desirable for all RHEL and Fedora templates.