[Bug 823679] Review Request: python-pdfminer - PDF data parsing library and tool

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

 



Comment # 1 from
Just a brief look. There are a few eyebrow raisers in the spec file:

> %prep
> %setup -q -n %{srcname}-%{version}
> export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:/usr/lib/pkgconfig

What's the purpose of this export? Nonarch pkgconfig files are stored in
/usr/share/pkgconfig/, so there should not be any reason to alter the search
path like that.


> %build
> CFLAGS="$RPM_OPT_FLAGS"

This isn't needed at all, is it? If any C code/data files were compiled in this
Python based package, that would deserve a comment in the spec file and would
ask for a closer look during review.


> %{__python} setup.py build
> make cmap
> chmod +x build/lib/pdfminer/*
> mv build/scripts-2.7/dumppdf.py build/scripts-2.7/dumppdf
> mv build/scripts-2.7/pdf2txt.py build/scripts-2.7/pdf2txt
> mv build/scripts-2.7/latin2ascii.py build/scripts-2.7/latin2ascii

I would move the "chmod" line and the three "mv" lines into the %install
section. In order to distinguish between the steps, which are needed to build
the upstream software, and the additional steps, which may only be necessary to
bring into shape the files for installation/packaging.


> %install
> [...]
> mkdir -p %{buildroot}%{_mandir}/man1/

No manual pages are available, however.


> %files
> [...]
> %{python_sitelib}/%{srcname}-20110515-py2.7.egg-info
> %{python_sitelib}/%{srcname}/*

https://fedoraproject.org/wiki/Packaging:UnownedDirectories


> %defattr(-,root,root,-)
> %doc docs/*

https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions

If there's a special/secret reason for putting the %defattr there, please
document it.


You are receiving this mail because:
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]