fedora-python / pyp2rpm

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

pyserial wrongly converted to Fedora package name #79

Closed hroncok closed 7 years ago

hroncok commented 7 years ago
python2 ./mybin.py -s esptool

Results in:

Requires:       python-pyserial

While the package name in Fedora for Python 2 is pyserial.

Tested on current master.

mcyprian commented 7 years ago

DantifiedNameConvertor wasn't used here, dnf can't be imported into python2. This is the result of NameConvertor's rpm_versioned_name method that simply adds ''python' prefix to the name of dependency.

Conversion works fine in DantifiedNameConvertor:

In [1]: from pyp2rpm.name_convertor import DandifiedNameConvertor
In [2]: con = DandifiedNameConvertor('fedora')
In [3]: con.rpm_versioned_name('pyserial', '2')
Out[3]: 'pyserial'
In [4]: con.rpm_versioned_name('pyserial', '3')
Out[4]: 'python3-pyserial'
hroncok commented 7 years ago

Why it cannot be imported?

mcyprian commented 7 years ago

It cannot be imported unless package python2-dnf is installed explicitly on Fedora. I added name_convertor info to log in 891ec8f8cc6553f501a9.

hroncok commented 7 years ago

I think a silent fial of import and falling back to not so good alternative is not a good idea. Do either of the following:

Unless there is a good reason to support dnf-less environments, I'd say make ti hard dependency.

mcyprian commented 7 years ago

I added a warning in commit 891ec8f8cc6553f50. Is this satisfying solution @hroncok? I think there are good reasons to support dnf-less environments:

hroncok commented 7 years ago

I created a testing sdist with syntax error in setup.py so I would test if the interpreters don't cycle there and back, which does not happen :+1:

However, I found out that the error output is not nice on Python 3 as the primary interpreter:

$ python2 ./mybin.py -s ./*.gz
INFO  Pyp2rpm initialized.
INFO  Using /home/churchyard/rpmbuild/SOURCES as directory to save source.
INFO  Local file: ./isholiday-0.1.tar.gz copyed to /home/churchyard/rpmbuild/SOURCES/isholiday-0.1.tar.gz.
INFO  Getting metadata from setup.py using SetupPyMetadataExtractor.
INFO  Running extract_dist command using runpy.
ERROR  Error occured, trying alternative python interpreter.
ERROR  Subprocess failed, stdout , stderr:   File "setup.py", line 3
    )
    ^
SyntaxError: invalid syntax

Failed to extract data from setup.py script.
$ python3 ./mybin.py -s ./*.gz
INFO  Pyp2rpm initialized.
INFO  Using /home/churchyard/rpmbuild/SOURCES as directory to save source.
INFO  Local file: ./isholiday-0.1.tar.gz copyed to /home/churchyard/rpmbuild/SOURCES/isholiday-0.1.tar.gz.
INFO  Getting metadata from setup.py using SetupPyMetadataExtractor.
INFO  Running extract_dist command using runpy.
ERROR  Error occured, trying alternative python interpreter.
ERROR  Subprocess failed, stdout b'', stderr: b'  File "setup.py", line 3\n    )\n    ^\nSyntaxError: invalid syntax\n'
Failed to extract data from setup.py script.

Seems like some bytes.decode() call is missing.

Attached the file for your convenience, it's public domain and rather small, so feel free to include it in the tests:

hroncok commented 7 years ago

I've also added a python 3 non compatible print statement now to see what happens:

$ python3 mybin.py -s ./isholiday-0.1.tar.gz
INFO  Pyp2rpm initialized.
INFO  Using /home/churchyard/rpmbuild/SOURCES as directory to save source.
INFO  Local file: ./isholiday-0.1.tar.gz copyed to /home/churchyard/rpmbuild/SOURCES/isholiday-0.1.tar.gz.
INFO  Getting metadata from setup.py using SetupPyMetadataExtractor.
INFO  Running extract_dist command using runpy.
ERROR  Error occured, trying alternative python interpreter.
ERROR  Failed to install package to virtualenv, skipping virtualenv metadata extraction.
WARNING  Some kind of error while communicating with client: None.
Traceback (most recent call last):
  File "/home/churchyard/Dokumenty/RedHat/pyp2rpm/pyp2rpm/metadata_extractors.py", line 39, in inner
    raise ValueError("Client is None.")
ValueError: Client is None.
WARNING  Template: fedora.spec was not found in /home/churchyard/Dokumenty/RedHat/pyp2rpm/fedora.spec using default template dir.
INFO  Using default template: fedora.spec.
INFO  Using name: python-isholiday.spec for specfile.
INFO  Specfile saved at: /home/churchyard/rpmbuild/SPECS/python-isholiday.spec.
INFO  That's all folks!

First of all, please notice the ValueError: Client is None part. I don't know why is that happening.

But the issue I wanted to point out here is the resulting spec file has python2 and python3 subpackages. I believe in a case when an interpreter fails to run setup.py, this version should be ruled out.

mcyprian commented 7 years ago

Thank you for your comments and testing data.

hroncok commented 7 years ago

BTW My comments were initially meant for the PR, sorry about that.

glensc commented 7 years ago

btw, i think it's best to add Requires to fedora version of pyp2rpm package that is included in distro. pyp2rpm is kind of development tool, having it python-dnf installed is good choice there.

mcyprian commented 7 years ago

Fixed in 891ec8f8cc6553f.