Bug 545045

Some comments:

* Wrt. this construct:
%ifarch x86_64 ppc64
sed -e '/^LIB/s/lib/lib64/g' -i Makefile
%endif

- Such sed calls can be generalized by applying %{_lib} (expands to "lib" on 32bit and to "lib64" on 64bit targets)

- Such sed calls are equivalent to "configuring" a package. They belong into %build

* static libs
%files devel
...
%{_libdir}/*.a

Feel strongly encouraged not to ship static libs. If you really can't avoid shipping them, move the *.a's to a separate *-static package.


* Upstream's tarball name is CQRlib. Fedora convention is to keep upstream's
package name => Your package should be named "CQRlib", not cqrlib.

* Your patch adds changes the shared libs from --shared to --export-dynamic and removes --release - Why? This is a significant behavioral change to upstream, I don't see any sense in.

(In reply to comment #1)

Spec URL: http://t-matsuu.sakura.ne.jp/mock/CQRlib.spec
SRPM URL: http://t-matsuu.sakura.ne.jp/mock/CQRlib-1.0.3-2.20090805.fc12.src.rpm

Thank you for your reviewing.

I removed the patch file because all of the modification for Makefile is now in spec file.


> * Wrt. this construct:
> %ifarch x86_64 ppc64
> sed -e '/^LIB/s/lib/lib64/g' -i Makefile
> %endif
> 
> - Such sed calls can be generalized by applying %{_lib} (expands to "lib" on
> 32bit and to "lib64" on 64bit targets)
> 
> - Such sed calls are equivalent to "configuring" a package. They belong into
> %build

Use %{_lib} and the sed call is moved to %build section.


> * static libs
> %files devel
> ...
> %{_libdir}/*.a
> 
> Feel strongly encouraged not to ship static libs. If you really can't avoid
> shipping them, move the *.a's to a separate *-static package.

Static library is separated to static subpackage.


> * Upstream's tarball name is CQRlib. Fedora convention is to keep upstream's
> package name => Your package should be named "CQRlib", not cqrlib.

Renamed to CQRlib.


> * Your patch adds changes the shared libs from --shared to --export-dynamic and
> removes --release - Why? This is a significant behavioral change to upstream, I
> don't see any sense in.  

Probably "--shared" you said means "-dynamic".
I found that "-dynamic" should be not "--export-dynamic" but "-rdynamic".  So I fix it to use "-rdynamic".

For "--release" issue, I keep the option in Makefile

(In reply to comment #2)
> Spec URL: http://t-matsuu.sakura.ne.jp/mock/CQRlib.spec
> SRPM URL:
> http://t-matsuu.sakura.ne.jp/mock/CQRlib-1.0.3-2.20090805.fc12.src.rpm

Spec URL: http://t-matsuu.sakura.ne.jp/mock/CQRlib/CQRlib.spec
SRPM URL: http://t-matsuu.sakura.ne.jp/mock/CQRlib/CQRlib-1.0.3-2.20090805.fc12.src.rpm

Spec URL: http://t-matsuu.sakura.ne.jp/mock/CQRlib/CQRlib.spec
SRPM URL: http://t-matsuu.sakura.ne.jp/mock/CQRlib/CQRlib-1.0.3-3.20090805.fc12.src.rpm

* Update based on the comment for another in-parallel reviewing package.
* remove static lirary

* The issue for the option in generating dynamic library has been reported to the upstream developer.

Uah, the file name of the shared library include full version number. :-(

It's not good.
If this package is updated, we have to recompile all the software which link to this library.

Now I'm contacting the upstream developer.

(In reply to comment #6)
> Uah, the file name of the shared library include full version number. :-(
> 
> It's not good.
Yep, but - this can be intentional. It's a matter of design.

If, for some reasons, upstream wants it this way, you have to stick with what upstream wants - In most cases, however --release is a matter of upstreams not being familiar with Linux shared libs.

Please also read 
info libtool 'Versioning'

After the discussion with upstream developer, he has no objection to remove -release.

I'll prepare a patch to remove -release and propose to upstream.

(In reply to comment #8)
> After the discussion with upstream developer, he has no objection to remove
> -release.
> I'll prepare a patch to remove -release and propose to upstream.    

No need to do that if upstream wants it this way.

I cannot download your spec and SRPM, the server is down?

(In reply to comment #9)
I believe you can download from the URL in the comment #5.
Please try again.

Updated
Spec URL: http://t-matsuu.sakura.ne.jp/mock/CQRlib/CQRlib.spec
SRPM URL:
http://t-matsuu.sakura.ne.jp/mock/CQRlib/CQRlib-1.0.5-1.fc12.src.rpm

IMHO, sed -e 's,-dynamic,-rdynamic,g' -i Makefile should be replaced by sed -e 's,-dynamic,-shared,g' , also you may also need to add -fPIC to CFLAGS since CQRlib is a shared lib.


Do you mind to report those issues to upstream?

After taking a look at Makefile, the -dynamic argument is irrelevant to the shared lib and static lib, so the above hack is not needed at all. BUILD_COMMAND_DYNAMIC is for example files only.

(In reply to comment #12)
> Do you mind to report those issues to upstream?

I've already reported last year.
I'll try again later.


(In reply to comment #13)
> After taking a look at Makefile, the -dynamic argument is irrelevant to the
> shared lib and static lib, so the above hack is not needed at all.
> BUILD_COMMAND_DYNAMIC is for example files only.

Do you mean your hack in comment #12 is not needed?

sourceforge now has CQRlib 1.0.6

I put together a quick autoconf'd version at 

http://www.five-ten-sg.com/cqrlib.spec
http://www.five-ten-sg.com/autoconf-files.tar.gz
http://www.five-ten-sg.com/CQRlib-1.0.6.tar.gz
http://www.five-ten-sg.com/CQRlib-1.0.6-1.fc12.src.rpm

That might be easier than extensive hacking on the upstream makefile, and it might also be applicable to the other parts needed for rasmol (cvector, neartree, cbflib).

Now I'm contacting upstream developer for /-dynamic/-rdynamic/ issue.
I'll back here next week.

Updated:
Spec URL: http://t-matsuu.sakura.ne.jp/mock/CQRlib/CQRlib.spec
SRPM URL:
http://t-matsuu.sakura.ne.jp/mock/CQRlib/CQRlib-1.1-0.fc13.src.rpm

Compile failed on i[36]86 architecture.
I've reported upstream.

Waiting for fix.

Updated:
Spec URL: http://t-matsuu.sakura.ne.jp/mock/CQRlib/CQRlib.spec
SRPM URL:
http://t-matsuu.sakura.ne.jp/mock/CQRlib/CQRlib-1.1.1-0.fc13.src.rpm

rpmlint CQRlib*rpm

CQRlib.src: W: %ifarch-applied-patch Patch1: CQRlib-1.0.6-lib64.patch

It'll better to avoid of using %inarch in spec, actually there are some other 64bit archs(e.g. sparc64 mipsel64).

%ifarch x86_64 ppc64 s390x
%patch1 -p1 -b .lib64
%endif
->
% %{_lib}==lib64
%patch1 -p1 -b .lib64
%endif

CQRlib.x86_64: W: non-coherent-filename getfile?taskID=2503499&name=CQRlib-1.1.1-0.fc15.x86_64.rpm CQRlib-1.1.1-0.fc15.x86_64.rpm

ping?

Sorry for my late response.

I'll back next week because my machine for working this bug is out of network now.

Updated:
Spec URL: http://t-matsuu.sakura.ne.jp/mock/CQRlib/CQRlib.spec
SRPM URL:
http://t-matsuu.sakura.ne.jp/mock/CQRlib/CQRlib-1.1.1-0.1.fc13.src.rpm

Issue mentioned in comment #20 is now fixed.

Approved!

New Package SCM Request
=======================
Package Name: CQRlib
Short Description: ANSI C API for quaternion arithmetic and rotation
Owners: tmatsuu
Branches: f12 f13 f14 el5 el6
InitialCC:

Git done (by process-git-requests).

CQRlib-1.1.1-1.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/CQRlib-1.1.1-1.el5

CQRlib-1.1.1-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/CQRlib-1.1.1-1.fc14

CQRlib-1.1.1-1.el5 has been pushed to the Fedora EPEL 5 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update CQRlib'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/CQRlib-1.1.1-1.el5

CQRlib-1.1.1-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

CQRlib-1.1.1-1.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.