[Bug 446651] Review Request: rvm - RVM library

[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 report.

Summary: Review Request: rvm - RVM library


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





------- Additional Comments From j.w.r.degoede at hhs.nl  2008-05-15 16:26 EST -------
(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 at 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


-- 
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, or are watching someone who is.