[Bug 807383] Review Request: PythonMagick - Interface to ImageMagick for Python written in C++

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

 



Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=807383

--- Comment #9 from Michael Schwendt <mschwendt@xxxxxxxxx> ---
A few mistakes here, including one or two eyebrow-raisers. Let's start with the
reviews in comment 3 and comment 7:


> [+] MUST: The package must meet the Packaging Guidelines .

Please be careful here. Basically, this MUST item is the hardest one to
acknowledge with a brief '[+]', since that means you've checked _everything_
written on the following hierarchy of Wiki pages:
https://fedoraproject.org/wiki/Packaging:Guidelines

Not only would you need to try to find a section in the guidelines for every
line of the spec file, you would also need to do that for the built rpms and
the build job output (as created by Mock or plain rpmbuild).


> [.] MUST: Static libraries must be in a -static package. [19]
> 
> [.] OK, not applicable

Cannot be true, because the reviewed package places a static lib in the -devel
packages:

| %files devel
| %{python_sitearch}/%{name}/_PythonMagick.a

Please revisit
https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries
and comment on it, if you disagree or if there are questions.


> [+] MUST: Development files must be in a -devel package. [20]

PythonMagick is a Python module to be used within Python software. Leaving
aside the Static Library guidelines for a moment, how does the _PythonMagick.a
library fit into all this?

$ rpmls -p PythonMagick-devel-0.9.7-2.fc18.x86_64.rpm
-rw-r--r--  /usr/lib64/python2.7/site-packages/PythonMagick/_PythonMagick.a



> [+] MUST: In the vast majority of cases, devel packages must
> require the base package using a fully versioned dependency:
> Requires: %{name}%{?_isa} = %{version}-%{release} [21]

The reviewed package does
  Requires:    %{name} = %{version}-%{release}
so %_isa is not used. Minor issue only, but can lead to trouble in some
situations.


> %description devel
> 
> %{name}-devel contains the library links you'll need to develop 
> Python ImageMagick applications. 

This description would deserve an explanation. Specifically: Which "links"? And
when are they needed?


> Group:		Development/Libraries

"Development/Languages" is very common for Python modules.


> Requires:	boost-python
> Requires:	ImageMagick-c++ >= 6.4
> Requires:	python >= 2.4

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

In short: Add comments to the spec file giving the rationale for each of those
explicit dependencies or drop them as appropriate. The section in the
guidelines may read as if it's specific to shared libs (here the
"ImageMagick-c++" explicit Requires), but basically it applies to all other
explicit Requires, too.

> Requires:	python >= 2.4

Currently the package automatically depends on

  python(abi) = 2.7
  libpython2.7.so.1.0()(64bit)

and explicitly on

  python >= 2.4

so which is right? Preferably, you drop the explicit dep on python >= 2.4,
since the automatic dependency is on Python 2.7.


> /bin/sh ./libtool --tag=CXX   --mode=link g++  -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4  -m64 -mtune=generic -DBOOST_PYTHON_DYNAMIC_LIB -avoid-version -module -L/usr/lib -Wl,-z,relro  -o _PythonMagick.la -rpath /usr/lib64/python2.7/site-packages/PythonMagick  pythonmagick_src/libpymagick.la helpers_src/libhelper.la -L/usr/lib -lboost_python -lMagick++ -lMagickCore    -lpython2.7 
> 

This is a line from the x86_64 build job output. The '-L/usr/lib' indicates
that somewhere an incorrect libdir value, perhaps a hardcoded one, is used.
Tracking down where and telling upstream about it might be worthwhile.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=qDbOWsGwWx&a=cc_unsubscribe
_______________________________________________
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]