Bug 251191

Have you seen the existing GLEW review ticket?  It's bug 229419.

(In reply to comment #1)
> Have you seen the existing GLEW review ticket?  It's bug 229419.

No I didn't, I didn't expect there to be a GLEW review as GLEW has been denied
on legal grounds by Spot several time, this version has had some surgery and
some replacements written to make it ok. I've added a comment to bug 229419 to
coordinate things.

Good work Hans. Regarding you're mail, no problem it makes sense for me to
review it. I'll hold on though until a decision is made on how to handle the
'duplicate' reviews.

mock rebuild on x86_64 show that:

cc -shared -Wl,-soname=libGLEW.so.1.4 -o lib/libGLEW.so.1.4.0 src/glew.pic_o
-L/usr/X11R6/lib64 -lXmu -lXi -lGLU -lGL -lXext -lX11 
/usr/bin/ld: cannot find -lXext
collect2: ld returned 1 exit status
make: *** [lib/libGLEW.so.1.4.0] Error 1

My point of view about multilibs is that binaries have to be split into another
(-utils) package...But that won't breaks things if i386 and x86_64 are installed
together, indeed..

I will try to test it with some package that BR glew-devel then...

(In reply to comment #4)
> mock rebuild on x86_64 show that:
> 
> cc -shared -Wl,-soname=libGLEW.so.1.4 -o lib/libGLEW.so.1.4.0 src/glew.pic_o
> -L/usr/X11R6/lib64 -lXmu -lXi -lGLU -lGL -lXext -lX11 
> /usr/bin/ld: cannot find -lXext
> collect2: ld returned 1 exit status
> make: *** [lib/libGLEW.so.1.4.0] Error 1
> 

Hmm, strange:
[hans@shalem ~]$ rpm -q --requires libXmu-devel
libXmu-devel-1.0.3-1.fc7.x86_64
libX11-devel  
libXext-devel  
<snip>

And libXmu-devel is BR'd, anyways I'll just add libXext-devel to the BR's too,
I'll do a new version with this fixed when a full review is done, as I dislike
doing a zillion iterations.

> My point of view about multilibs is that binaries have to be split into another
> (-utils) package...But that won't breaks things if i386 and x86_64 are installed
> together, indeed..
> 

I still don't know I don't necessarily object to having a -utils package, but
the other glew submission has the docs seperate too, no I think that most of the
docs should be moved to the -devel packahge, but splitting them off?

All files in the combined packages way in at 1 MB combined, I think splitting
this up 4 ways is over the top. I really only see 2 (3) reasons for splitting of
any utility binaries from libs:
1) They drag in dependencies on which the lib itself doesn't depend
2) They take up lots of diskspace
3) The cause multilib conflicts (this is fixable by making scripts identical
   between the i3986 and x86_64 version though)

Non of this is the case here, so I say lets just have a glew and a glew-devel
(and move most of the docs to glew-devel, as they describe mostly the API.

Ok, the issue of the duplicate review requests has been sorted out and the
decision has been made this is the one to review. Ian if you feel like it and
have the time, I would be very grateful.

No problem.

Ok i will take the review then:

* well indeed,libXext-devel was only required on FC6, since F7 it works without
BR it...If you add it the same spec file will work from FC6->devel

* splitting the doc to -devel is fine for me

* About the bin split i think it is the same as glx-utils (and glxinfo)
{case-study} - If I install binaries nvidia drivers on x86_64 but without the
compat32 libs which i don't care to have. Then, install glew.x86_64 but later an
update or a dependency bring me glew.i386. /usr/bin/{glewinfo,visualinfo} will
be i386 ... then ldd will link with default mesa's GL libs and not nvidia's
which will return wrong values if i uses thoses i386 binaries on x86_64...

* unused-direct-shlib-dependency (at least on FC6 x86_64 )
Maybe a problem with -L/usr/X11R6/lib64 that do not exist on my system

W: glew unused-direct-shlib-dependency /usr/lib64/libGLEW.so.1.4.0
/usr/lib64/libXmu.so.6
W: glew unused-direct-shlib-dependency /usr/lib64/libGLEW.so.1.4.0
/usr/lib64/libXi.so.6
W: glew unused-direct-shlib-dependency /usr/lib64/libGLEW.so.1.4.0
/usr/lib64/libGLU.so.1
W: glew unused-direct-shlib-dependency /usr/lib64/libGLEW.so.1.4.0
/usr/lib64/libXext.so.6
W: glew unused-direct-shlib-dependency /usr/lib64/libGLEW.so.1.4.0
/usr/lib64/libX11.so.6

@Ian Chapman
Sorry for the miss-collision - I haven't noticed you talk to take the review
Feel free to re-take it...

(In reply to comment #8)
> * About the bin split i think it is the same as glx-utils (and glxinfo)
> {case-study} - If I install binaries nvidia drivers on x86_64 but without the
> compat32 libs which i don't care to have. Then, install glew.x86_64 but later an
> update or a dependency bring me glew.i386. /usr/bin/{glewinfo,visualinfo} will
> be i386 ... then ldd will link with default mesa's GL libs and not nvidia's
> which will return wrong values if i uses thoses i386 binaries on x86_64...
> 

Except that if you first install the x86_64 package and then the i386 package,
the x86_64 files will be kept, if you do it the other way around the x86_64
files will override the i386 ones, so in the end if you install both you will
always end up with the 64 bit versions (on x86_64, on ppc you will always end up
with the 32 bit versions), this is how rpm multilib works, for binaries it has a
preferred arch and when both installed that arch wins.

> * unused-direct-shlib-dependency (at least on FC6 x86_64 )
> Maybe a problem with -L/usr/X11R6/lib64 that do not exist on my system
> 
> W: glew unused-direct-shlib-dependency /usr/lib64/libGLEW.so.1.4.0
> /usr/lib64/libXmu.so.6
> W: glew unused-direct-shlib-dependency /usr/lib64/libGLEW.so.1.4.0
> /usr/lib64/libXi.so.6
> W: glew unused-direct-shlib-dependency /usr/lib64/libGLEW.so.1.4.0
> /usr/lib64/libGLU.so.1
> W: glew unused-direct-shlib-dependency /usr/lib64/libGLEW.so.1.4.0
> /usr/lib64/libXext.so.6
> W: glew unused-direct-shlib-dependency /usr/lib64/libGLEW.so.1.4.0
> /usr/lib64/libX11.so.6

I will fix this as soon as the rest is sorted out (like who does the review, and
once that sorted out we need to agree on what to do with the binaties).

Well, overriding files is always bad, as it is probably unexpected...

But as others apps that uses glew-devel is linking fine with this version,
I'm ok to leave the decision to you about the binary split...

(In reply to comment #9)
> @Ian Chapman
> Sorry for the miss-collision - I haven't noticed you talk to take the review
> Feel free to re-take it...

No problem then Nicolas, seeing as you've already made a start, feel free to
continue with the review, I'm sure Hans won't object. I'm CC'ed in anyway as I
have an interest in seeing this in Fedora. Cheers.

Nicolas, I noticed that this review has been reassigned back to me. (See comment
12). Are you currently performing the review? If not I'll take it over. Please
let me know ASAP. If I don't here in the next few days, I'll assume not. Cheers :-)

Well I was expecting Hans to solve the issue state in #8
(maybe some INSTALL="install -p -c" would be better at %install step... )
I don't see anything more to be fixed...

Unless you have other comments to make, i can follow the review...

(In reply to comment #14)
> Well I was expecting Hans to solve the issue state in #8

And I was waiting for you and Ian to sort out the who is going to review this
issue, as I already said in comment #10, so I guess we were all waiting for
eachother :)

Anyways here is a new version, notice that I did _not_ add the BuildRequires:
libXext-devel, as libXext is no longer needed now that the
unused-direct-shlib-dependencies are fixed.

* Sat Aug 18 2007 Hans de Goede <j.w.r.degoede> 1.4.0-3
- Fix multiple unused direct shlib dependencies in libGLEW.so
- Remove the "SGI Free Software License B" and "GLX Public License" tekst from
  the doc dir in the tarbal
- Patch credits.html to no longer refer to the 2 non free licenses, instead it
  now points to LICENSE-README.fedora
- Put API docs in -devel instead of main package

Spec URL: http://people.atrpms.net/~hdegoede/glew.spec
SRPM URL: http://people.atrpms.net/~hdegoede/glew-1.4.0-3.fc8.src.rpm

Ok then re-assign the bug to me (last time)
(mock testing)

* mock tested in fc6 x86_64

* rpmlint on installed package is quiet

* Since header files are patched in %prep, it is not relevant to use install -p
for headers

-------------------

This package (glew) is APPROVED by me

-------------------

Thanks for the review!

New Package CVS Request
=======================
Package Name:      glew
Short Description: The OpenGL Extension Wrangler Library
Owners:            jwrdegoede / j.w.r.degoede
Branches:          FC-6 F-7 devel
InitialCC:         <empty>
Commits by cvsextras: yes

cvs done.

Imported and build, closing. I'll also be pushing this as an F-7 update, and
once it has hit the F-7 repo, I'll build it for FC-6.

Package Change Request
======================
Package Name: glew
New Branches: EL-5 EL-4
Owners: orion
InitialCC:         
Commits by cvsextras: yes


Adam has indicated his approval via email.  Don't know if he wants to co-own.

cvs done.

Can you contact me, if glew is ready for EL-5, so I can build blender agains it.

It's in updates-testing.  I think you need to request it to be added to the buildroot though to build with it now.