[Bug 634911] Review Request: nodejs - Evented I/O for v8 JavaScript

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=634911

--- Comment #8 from Damian Wrobel <dwrobel at ertelnet.rybnik.pl> 2010-11-04 15:50:12 EDT ---
Please find some additional comments:

(In reply to comment #5)
> (In reply to comment #4)
> > - The %{_prefix}/lib should be replaced with %{_libdir}
> > 
> > NOT OK
> 
> Not really. This is /usr/share/lib even on 64-bit architectures, since it
> contains the architecture independent-code.

According to the [1] the %{_libdir} by default resolves to: /usr/lib or
/usr/lib64 (on 64-bit architectures). You can check it by invoking:
rpm --eval "%{_libdir}" on your platform.

IMHO the usage of: %{_prefix}/lib is not correct mainly because:

a) it generates rpmlint error (already pointed in the comment #4):
 nodejs.spec:78: E: hardcoded-library-path in %{_prefix}/lib/node

b) you can't be sure that the %{_prefix}/lib directory really exists on the
platform.


Is there any reason why do you manually install: node.h and node_object_wrap.h
headers and do not place them in the "node" subdirectory in the %{_includedir}?
It looks that you can safely use: "waf install" for that purpose.

Originally they are placed in the "node" subdirectory which is better than try
to put everything directly into the %{_includedir}. It would also be
appreciated to provide an appropriate pkgconfig file.


According to the [2], please add a comment or a bug link to the provided
patches.


The way the %files section is organized is quite confusing, and it's very
difficult to guess what directories the package owns and whether it's done
intentionally or not.


> > Please consider to update to the latest available version[3].
> > 
> > NOT OK
> 
> Done.
They've managed to release yet another version [3].


Please also use macros consistently[4].


As the package do not compile on all architectures it should have an
appropriate description as per[5].


References:
[1]. http://fedoraproject.org/wiki/Packaging:RPMMacros
[2].
http://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
[3]. http://nodejs.org/dist/node-v0.2.4.tar.gz
[4]. http://fedoraproject.org/wiki/Packaging/Guidelines#macros
[5].
http://fedoraproject.org/wiki/Packaging/Guidelines#Architecture_Build_Failures

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.