Bug 446651

I would appreciate it if you would review my review request
(bug 444933)

For 1.15-1:

* License:
----------------------------------------------------
include/coda_mmap_anon.h	GPLv2
rvm/rvm_map.c			#include "coda_mmap_anon.h"
   792  DEBUG:  gcc -DHAVE_CONFIG_H -I. -I. -I.. -I../include -DRVM_USELWP -O2
-g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables -MT librvmlwp_la-rvm_map.lo -MD -MP -MF
.deps/librvmlwp_la-rvm_map.Tpo -c rvm_map.c  -fPIC -DPIC -o
.libs/librvmlwp_la-rvm_map.o
   828  DEBUG: gcc -shared  .libs/librvmlwp_la-rvm_init.o
.libs/librvmlwp_la-rvm_map.o .libs/librvmlwp_la-rvm_unmap.o
.libs/librvmlwp_la-rvm_tran
s.o .libs/librvmlwp_la-rvm_logstatus.o .libs/librvmlwp_la-rvm_logflush.o
.libs/librvmlwp_la-rvm_logrecovr.o .libs/librvmlwp_la-rvm_utils.o .libs/
librvmlwp_la-rvm_io.o .libs/librvmlwp_la-rvm_status.o
.libs/librvmlwp_la-rvm_debug.o .libs/librvmlwp_la-rvm_printers.o  -llwp  -m32
-march=i386 -
mtune=generic -Wl,-soname -Wl,librvmlwp.so.1 -o .libs/librvmlwp.so.1.2.2

seg/rvm_segutil.c		#include "coda_mmap_anon.h"
   846  DEBUG:  gcc -DHAVE_CONFIG_H -I. -I. -I.. -I../include -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4
-m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -MT
libseglwp_la-rvm_segutil.lo -MD -MP -MF .deps/libseglwp_la-rvm_segutil.Tpo -c
rvm_segutil.c  -fPIC -DPIC -o .libs/libseglwp_la-rvm_segutil.o
   857  DEBUG: gcc -shared  .libs/libseglwp_la-rvm_segutil.o
.libs/libseglwp_la-rvm_loadseg.o .libs/libseglwp_la-rvm_createseg.o .libs/libseglwp_
la-rvm_releaseseg.o  ../rvm/.libs/librvmlwp.so  -m32 -march=i386 -mtune=generic
-Wl,-soname -Wl,libseglwp.so.1 -o .libs/libseglwp.so.1.2.2

   900  DEBUG: gcc -shared  .libs/librdslwp_la-rds_coalesce.o
.libs/librdslwp_la-rds_free.o .libs/librdslwp_la-rds_init.o
.libs/librdslwp_la-rds_malloc.o .libs/librdslwp_la-rds_prealloc.o
.libs/librdslwp_la-rds_split.o .libs/librdslwp_la-rds_stats.o
.libs/librdslwp_la-rds_start.o .libs/librdslwp_la-rds_util.o
.libs/librdslwp_la-rds_zap.o .libs/librdslwp_la-rds_maxblock.o 
../seg/.libs/libseglwp.so ../rvm/.libs/librvmlwp.so -llwp  -m32 -march=i386
-mtune=generic -Wl,-soname -Wl,librdslwp.so.1 -o .libs/librdslwp.so.1.2.2
----------------------------------------------------
  - All GPLv2!! :(

* Dependency for devel package
----------------------------------------------------
[tasaka1@localhost ~]$ LANG=C pkg-config --libs rvmlwp
Package lwp was not found in the pkg-config search path.
Perhaps you should add the directory containing `lwp.pc'
to the PKG_CONFIG_PATH environment variable
Package 'lwp', required by 'Recoverable Virtual Memory (lwp build)', not found
----------------------------------------------------
  - This means that lwp-devel should be in the Requires
    of rvm-devel.

* rpath hack
----------------------------------------------------
# Don't use rpath!
sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool
sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool
# work around linking failures because of the disabling of rpath above
export LD_LIBRARY_PATH=`pwd`/rvm/.libs:`pwd`/seg/.libs
----------------------------------------------------
  - Umm.. are there any better hack than this way? Especially
    adding the need of LD_LIBRARY_PATH like above is IMO
    not good, just creating a broken libtool.

    In such case, I usually
    - add /usr/lib /lib64 to sys_lib_search_path_spec in 
      configure (as libtool is created from configure)
    
      Actually the diff of %_bindir/libtool between i386 and x86_64
      shows this.

    - Or simply "make LIBTOOL=%_bindir/libtool" or cp %_bindir/libtool
      to working directory (BR: libtool is needed).

(In reply to comment #2)
>     - add /usr/lib /lib64 to sys_lib_search_path_spec in 
      - add /usr/lib64 /lib64 to ....

>       configure (as libtool is created from configure)

(In reply to comment #2)
> For 1.15-1:
> 

Thanks for the review!

> * License:
> ----------------------------------------------------
> include/coda_mmap_anon.h	GPLv2

<snip>

>   - All GPLv2!! :(

Yes, I already saw that, but the only code that file contains is:

#define mmap_anon(raddrptr, addrptr, len, prot) do { \
    int fd = -1, flags = MAP_ANON | MAP_PRIVATE; \
    if (addrptr) flags |= MAP_FIXED; \
    if (!MAP_ANON) fd = open("/dev/zero", O_RDWR); \  
    raddrptr = mmap((char *)addrptr, len, prot, flags, fd, 0); \
    if (fd != -1) close(fd); \
} while(0);

Which IMHO is hardly enough code to be copyrightable. Anyways rvm uses lwp, and
that definitely is GPLv2 (although I believe that is not upstream's intent, I'll
contact them about this). So no matter what rvm is effectively GPLv2 licensed.

So I'm fine with either way let me know if you agree with the barely
copyrightable, or if you want the License tag changed

> * Dependency for devel package
> ----------------------------------------------------
> [tasaka1@localhost ~]$ LANG=C pkg-config --libs rvmlwp
> Package lwp was not found in the pkg-config search path.
> Perhaps you should add the directory containing `lwp.pc'
> to the PKG_CONFIG_PATH environment variable
> Package 'lwp', required by 'Recoverable Virtual Memory (lwp build)', not found
> ----------------------------------------------------
>   - This means that lwp-devel should be in the Requires
>     of rvm-devel.
> 

Will fix when I know what todo with the license tag (and rpath)

> * rpath hack
> ----------------------------------------------------
> # Don't use rpath!
> sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool
> sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool
> # work around linking failures because of the disabling of rpath above
> export LD_LIBRARY_PATH=`pwd`/rvm/.libs:`pwd`/seg/.libs
> ----------------------------------------------------
>   - Umm.. are there any better hack than this way?

Well this (except for the LD_LIBRARY_PATH is whats adviced by the guidelines

>     Especially
>     adding the need of LD_LIBRARY_PATH like above is IMO
>     not good, just creating a broken libtool.
> 
>     In such case, I usually
>     - add /usr/lib /lib64 to sys_lib_search_path_spec in 
>       configure (as libtool is created from configure)
>     
>       Actually the diff of %_bindir/libtool between i386 and x86_64
>       shows this.
> 

Yes, that will work, but I don't like patching configure, as a patch will break
with each new release, but I guess I could sed this, but then again my sed isn't
as good as yours (I've been reviewing cairo-dock, some mighty fine sed code there :)

>     - Or simply "make LIBTOOL=%_bindir/libtool" or cp %_bindir/libtool
>       to working directory (BR: libtool is needed).

Thats evil, you should never use a different libtool then the one automake used
when generating the Makefiles, otherwise their might be all kinda trouble, if
you want todo this you should use "libtoolize -f" and then "autoreconf -f -i",
then again maybe autoreconf will also call the libtoolize for you so just
autoreconf will be enough.

Anyways I prefer to keep this as is, as this is how its done in all my packages,
but if you insist I'll switch to changing sys_lib_search_path_spec

Okay, thanks you for good explanation.

-------------------------------------------------------------------------
    This package (rvm) is APPROVED by me
-------------------------------------------------------------------------

New Package CVS Request
=======================
Package Name:      rvm
Short Description: RVM library
Owners:            jwrdegoede
Branches:          F-8 F-9
InitialCC:
Cvsextras Commits: Yes

cvs done.

Imported and build for F-8 F-9 devel, will get pushed as update for F-8 and F-9
together with the rest of the coda stack once coda itself has been built too.

Package Change Request
======================
Package Name: rvm
New Branches: EL-5
Owners: agoode nhorman

cvs done.