Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1409648 - Review Request: python-olefile - Python package to parse, read and write Microsoft OLE2 files
Summary: Review Request: python-olefile - Python package to parse, read and write Micr...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Igor Gnatenko
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1471557 (view as bug list)
Depends On:
Blocks: 1471561
TreeView+ depends on / blocked
 
Reported: 2017-01-02 18:07 UTC by Sandro Mani
Modified: 2017-07-17 13:18 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-01-02 21:34:52 UTC
Type: ---
Embargoed:
ignatenko: fedora-review+

Attachments (Terms of Use)

Description Sandro Mani 2017-01-02 18:07:49 UTC 
Spec URL: https://smani.fedorapeople.org/review/python-olefile.spec
SRPM URL: https://smani.fedorapeople.org/review/python-olefile-0.44-0.1.gitbc9d196.fc26.src.rpm
Description: Python package to parse, read and write Microsoft OLE2 files
Fedora Account System Username: smani

Using a snapshot since it contains fixes which were originally in the fork bundled with python-pillow (and the reason for this package is that the fork was removed in favor of the upstream version)
Comment 1 Igor Gnatenko 2017-01-02 18:28:01 UTC 
Some preliminary comments:

* %if 0%{?commit:1} -> %if %{defined commit}
* %{__python2} setup.py build -> %py2_build
* %{__python3} setup.py build -> %py3_build
* %{__python2} setup.py install --skip-build --root %{buildroot} -> %py2_install
* %{__python3} setup.py install --skip-build --root %{buildroot} -> %py3_install

* You can use same summary by using %{summary} in subpackages
* You can reuse %{url} in Source tag
* Would be nice to not use macro in URL, so it will be clickable
* Please move BuildRequires under subpackages
* Tests are there, but not ran

%{python3_sitelib}/* is too wide, please specify more carefully, like:
%{python3_sitelib}/%{modname}-*.egg-info
%{python3_sitelib}/%{modname}/
Comment 2 Sandro Mani 2017-01-02 18:38:39 UTC 
Spec URL: https://smani.fedorapeople.org/review/python-olefile.spec
SRPM URL: https://smani.fedorapeople.org/review/python-olefile-0.44-0.2.gitbc9d196.fc26.src.rpm

%changelog
* Mon Jan 02 2017 Sandro Mani <manisandro> - 0.44-0.2.gitbc9d196
- Use %%py_build and %%py_install macros
- Use %%summary, %%url to reduce duplicate text
- Add %%check
- Move BR to subpackages
Comment 3 Igor Gnatenko 2017-01-02 18:42:04 UTC 
Add %{?python_provide:%python_provide pythonX-%{srcname}} under each subpackage (replace 2 and 3 accordingly).

Forgot about this one...

Btw, you can use common description:
%global _description \
foo\
bar\
baz.

%description %{_description}
...
%description -n python2-%{srcname} %{_description}

Python 2 version.
...
%description -n python3-%{srcname} %{_description}

Python 3 version.
Comment 4 Sandro Mani 2017-01-02 18:46:41 UTC 
Spec URL: https://smani.fedorapeople.org/review/python-olefile.spec
SRPM URL: https://smani.fedorapeople.org/review/python-olefile-0.44-0.3.gitbc9d196.fc26.src.rpm

%changelog
* Mon Jan 02 2017 Sandro Mani <manisandro> - 0.44-0.3.gitbc9d196
- Further reduce duplicate text
- Add python_provides
Comment 5 Igor Gnatenko 2017-01-02 18:53:17 UTC 
So, after full review -- still some issues, but easy to fix during import:

* E: wrong-script-interpreter /usr/lib/python3.6/site-packages/OleFileIO_PL.py /usr/local/bin/python
* E: wrong-script-interpreter /usr/lib/python3.6/site-packages/olefile/__init__.py /usr/local/bin/python 
* E: wrong-script-interpreter /usr/lib/python3.6/site-packages/olefile/olefile.py /usr/bin/env python
-> find -type f -name '*.py' -exec sed -i -e '1{\@^#!/usr/bin/env python@d}' {} ';'

* E: wrong-script-end-of-line-encoding /usr/lib/python3.6/site-packages/OleFileIO_PL.py
* E: wrong-script-end-of-line-encoding /usr/lib/python3.6/site-packages/olefile/__init__.py
* W: wrong-file-end-of-line-encoding /usr/share/doc/python3-olefile/Contribute.md
* W: wrong-file-end-of-line-encoding /usr/share/doc/python3-olefile/Install.md
* W: wrong-file-end-of-line-encoding /usr/share/doc/python3-olefile/License.md
* W: wrong-file-end-of-line-encoding /usr/share/doc/python3-olefile/OLE_Overview.md
-> sed -i -e "s/\r//" *.md in %prep

Also please ask upstream to make such changes, there is no reason to not do them.
Comment 6 Sandro Mani 2017-01-02 19:53:48 UTC 
Upstream ticket at https://github.com/decalage2/olefile/issues/50

Thanks for the quick review!
Comment 7 Kevin Fenzi 2017-01-02 21:20:12 UTC 
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/python-olefile
Comment 8 Michal Ambroz 2017-07-17 13:18:16 UTC 
*** Bug 1471557 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.