Bug 165992

1.
You should use %{?dist} in Release tag.

2.
I can't compile this package:
gcc -c -I../../include -I../../src/mesa -I../../src/mesa/main
-I../../src/mesa/glapi -I../../src/mesa/math -I../../src/mesa/tnl
-I../../src/mesa/shader -I../../src/mesa/swrast -I../../src/mesa/swrast_setup
-Wall  -ansi -pedantic -fPIC -D_POSIX_SOURCE -D_POSIX_C_SOURCE=199309L
-D_SVID_SOURCE -D_BSD_SOURCE -DUSE_XSHM -DUSE_X86_ASM -DUSE_MMX_ASM
-DUSE_3DNOW_ASM -DUSE_SSE_ASM -DPTHREADS -DFX -I/usr/include/glide3
drivers/glide/fxapi.c -o drivers/glide/fxapi.o
drivers/glide/fxapi.c: In function `fxMesaCreateContext':
drivers/glide/fxapi.c:468: error: `GR_PIXFMT_AA_8_ARGB_1555' undeclared (first
use in this function)
drivers/glide/fxapi.c:468: error: (Each undeclared identifier is reported only once
drivers/glide/fxapi.c:468: error: for each function it appears in.)
drivers/glide/fxapi.c:487: error: `GR_PIXFMT_AA_8_RGB_565' undeclared (first use
in this function)
drivers/glide/fxapi.c:508: error: `GR_PIXFMT_AA_8_ARGB_8888' undeclared (first
use in this function)
drivers/glide/fxapi.c:632: warning: int format, FxI32 arg (arg 3)
drivers/glide/fxapi.c:632: warning: int format, FxI32 arg (arg 5)
make[4]: *** [drivers/glide/fxapi.o] Error 1
make[4]: Leaving directory `/home/marcin/rpm/BUILD/Mesa-6.2.1/src/mesa'
make[3]: *** [default] Error 2
make[3]: Leaving directory `/home/marcin/rpm/BUILD/Mesa-6.2.1/src/mesa'
make[2]: *** [subdirs] Error 1
make[2]: Leaving directory `/home/marcin/rpm/BUILD/Mesa-6.2.1/src'
make[1]: *** [default] Error 1
make[1]: Leaving directory `/home/marcin/rpm/BUILD/Mesa-6.2.1'
make: *** [linux-x86-glide] Error 2

My box is FC2.

I have Glide3-devel-20010520-30 (FC2), and there isn't any
GR_PIXFMT_AA_8_ARGB_1555 in header files. But Glide3-devel-20010520-36 (FE4)
include GR_PIXFMT_AA_8_ARGB_1555 in g3ext.h. Also Glide3-devel-20010520-33 (FC3)
don't have  GR_PIXFMT_AA_8... So you should change BuildRequires. I don't know
in wich release GR_PIXFMT_AA_8_ARGB_1555 appeard.

In response to 1:
You should use %{?dist} in Release tag.

No I shouldnot it is not mandatory, and actually is pretty useless. What one
should do is make sure that ALL EVR's released for a certain FC version are all
older then all EVR's released for the next, so initial release would be:
FC3:   xxx-version-1
FC4:   xxx-version-2
devel: xxx-version-3

Then when an update is issued you get:
FC3:   xxx-version-1.1
FC4:   xxx-version-2.1
devel: xxx-version-4

Thats the only way to make sure upgrades will work reliable in all
circumstances, disttag is actually pretty useless.

With disttag one would get:

Initial release:
FC3:   xxx-version-1.fc3
FC4:   xxx-version-1.fc4
devel: xxx-version-1.fc5

FC5 gets released:
FC5:   xxx-version-2.fc5

Update (skip release 2 because that was used for Fc5 rebuild):
FC3:   xxx-version-3.fc3
FC4:   xxx-version-3.fc4
devel: xxx-version-3.fc5

No assume someone is running FC4, and runs yum update regulary, he will have
installed:
FC4:   xxx-version-3.fc4

Some weeks after the FC5 release he has heard no problems so he decides to
upgrade to FC5. He has a slow download, so he gets CD's from a friend including
FE cd's (oneday there will hopefully be FE isos), so he upgrades his system, but
the FE cd's contains the initial FE release which is xxx-version-2.fc5
He has installed xxx-version-3.fc4, which is newer, even though it has an older
disttag. So the upgrader won't upgrade xxx. Leaving him with an xxx build
against older libs which therefore might not work properly. And no dependencies
won't always catch this, dependencies although there are great are limited ibn
what they can do.

Notice how with my versioning scheme (which is also used by lots of other FE
maintainers) this problem does not exist.
I was actually going to ask why you used a disttag for xscorch, since IMHO
disttags are not all that good.

In response to 2:
Yes Glide3-devel package's not build by me contain a bug they install the
voodoo3 headers instead of the voodoo5 headers thus missing certain defines.
I have no plans to build this for FC3 since FC3's Glide release doesnot contain
glide binaries compiled for the Voodoo1 and 2 which are the very cards this is
package is intended for. (Glide3 from FC3 lacks glide-v2.so and glide-v1.so)

Please build/test this under atleast FC4 or even better Rawhide. The inital cvs
import of a package is always rawhide, so that really is where on should test.

The disttag scenario in comment 1 is otherwise valid, but it makes the 
assumption that packages for older distro versions won't be updated to newer 
upstream _versions_.  That doesn't hold true in Fedora Core nor Extras, so 
there's no universal way of "protecting" upgrades to the next baseline (eg. 
CD/DVD-based) distro version using the release tag alone, disttags or not. 

(Oops, s/comment 1/comment 3/.) 

Comment 4/5 you're right I didn't think about that one. Hmm.

Comment 1/2 I've explained why I have done things as I have done them. About the
BuildRequires, the guidelines specificly state that BuildRequires should not be
versioned unless one plans to build for a target where an older then required
version has been available. Could you continue your review please and tell me
what needs to be changed in order to get this package approved, thanks!

My own biggest concern is the name, can anyone think of a better name?

I have voodoo3 :-( , Fedora 4.

I tried during three weeks to find out the reason for which I didn´t have direct
rendering enabled on mesa3D (by glxinfo) despite I had 
"Direct Rendering Enabled" in the /var/log/Xorg.0.log. 
Thanks to glxinfo debug info, I found out that some libraries of Glide3 are
missing. Although the Fedora installation recognized my voodoo card, It didn´t
installed the Glide3 RPM package. If It would, I wouldn´t have to spend three
weeks for this little and annoying problem.

I hope in Fedora 5 this little problem will be solved :-)

Unfortunatly since Glide3 has been moved from Core to Extras you will always
need to install Glide3 yourself. Fortunatly, its being activly maintained in
Extras, which is better then leaving it (bit)rotting in core. Also this bug is
not the appropiate place for comments like this.

Below is a quick patch to the spec file :
- Keep spaces/tabs consistent
- Shorten Source0 URL and use version macro (less errors when updating)
- Add exclusivearch to ix86 and x86_64 (seems to make sense, tell me otherwise)
- Simplify %build (non duplicate lines are less prone to errors when changed)
- Remove libver define as it can easily be avoided
- Simplify %install section as the proper relative symlink already exists

I tested a build on FC4, and a package install/uninstall. All went fine, but I
am unfortunately unable to test the package's actual functionnality because I
lack Voodoo hardware.

I have a question, though : Instead of using the wrapper script approach,
wouldn't it be possible to create a package which could be a system wide
replacement for xorg-x11-Mesa-libGL? Having the libGL libary split off was
(AFAIK) done exactly for that purpose.

$ rpm -q --provides xorg-x11-Mesa-libGL
Mesa
libGL = 1
libGL.so.1
xorg-x11-Mesa-libGL = 6.8.2-37.FC4.49.2

So if we want to do it, renaming the package Glide3-Mesa-libGL for instance, and
providing explicitely "Mesa" and "libGL = 1" should be enough. The actual
install step will need to be manual though, as an obsoletes is not what is
wanted (everyone would end up with the Glide3 version!), and the conflicting
libGL.so.1 symlink will prevent any parallel installation. I can't think of
anything other than "rpm -e --nodeps xorg-x11-Mesa-libGL" prior to installing
this package.

Matthias

Created attachment 119978 [details]
Spec file patch

Thinking a bit more about the above, wouldn't it be best to rename this package
to Glide3-Mesa-libGL anyway since it isn't actually a full Mesa distribution,
but only the libGL part?

Hi,

Thanks for reviewing, I've reviewed your patch and I'll apply it. Especially
thanks for catching the lack of exclusive arch since Glide itself is
exclusivearch that will safe me one failed build attempt :)

About the wrapper script versus libgl replacement. The old voodoo cards for
which this version of Mesa is are 3d addon cards which work next to a 2d card
and as such are fullscreen only, that is somewhere between kinda bad and real
bad for some applications, hence the wrapper approach, so that the user can
choose between slow but window capable 3d and "fast" and fullscreen 3d.

About the name I'm all for a better name. Glide3-Mesa-libGL sounds better then
what I came up with.

I made my name start with Glide3 because its a special version of Mesa for Glide
and thus kindoff a subpackage, but the base/source of the package is Mesa, so
you could argue that making the name of the package start with Glide3 is wrong.

So is my package approved with you patch + name change? And/or do you know a
better name?

I've accidently removed the old version some time ago, but nobody seems to have
noticed. So now I'm back with a new version and finally a descent name! :
Spec Name or Url: http://home.zonnet.nl/jwrdegoede/Glide3-libGL.spec
SRPM Name or Url: http://home.zonnet.nl/jwrdegoede/Glide3-libGL-6.2.1-2.src.rpm

Review please!

I'll pick it up, changing to FE-REVIEW.

Pre-review comments:

- Package name: following mesa-libGL as example, it might be better to make the
package name "glide3-libGL", ie Mesa is also with capital M, but choose to start
with lower case 'm' in package name, for consitency it would be good to follow
this example (don't forget to change spec file name too)

- Out of consitency i would then choose to change the name of the shell script
too, ie glide3-libGL

- Licence entry makes rpmlint complain, but thats an ignorable warning. 

- However "and others" could be any kind of licence that would conflict with
MIT/X11 licences, can't you stick to MIT/X11? If not, is there any way to
clarify this a bit more? Or are you depending on the licence.html file being
sufficient?

As soon as you've addressed the above issues i'll go thru the formal review list

1) I picked up the Glide3 name with a G from the Glide3 package, from which this 
   is sortoff / kindoff a subpackage (not really, but making it seem like a 
   subpackage, so that people might actually find it is the idea).

   I took over the Glide3 package from Core including the capital G so thats
   a heritage I'm stuck with.

2) I took the license part from the Core Mesa package, I see they have dropped 
   the "and others" now a days, so I'll drop that too, if you don't mind I'll
   wait with making a new spec and SRPM until the formal review (slow upload
   connection).

Hmm confusing naming, Mesa is 'Mesa' and not 'mesa' in the libGL package (only
changed very recently for modular x), but your right about the Glide3 /
Glide3-devel package names, however its all PackageNamingGuidelines compliant
and both cases have a disadvantage, so i'll fully trust your judgement on this.

MUST review items:
- Builds cleanly on FC5 devel.
- rpmlint output:
W: Glide3-libGL invalid-license MIT/X11, and others
- Source included matches upsteam source (md5sum)
- Package name meets guidelines
- spec file name is in %{name}.spec format
- Licence (when changed to MIT/X11) is fedora extra's compatible & is included
in spec
- Spec file is in (american) english
- Does not list buildrequires that are excepted in the package guidelines
- All build dependencies are listed
- No ldconfig needed
- All files have proper permissions
- Package is not relocatable
- No duplicate files in %files section
- No missing files in %files section (only ships basic so & shell script)
- Has a proper %clean section with rm -rf $RPM_BUILD_ROOT
- Uses macro's described in PackagingGuidelines
- No entries in %doc that are required for standard program operation
- No -devel package needed (3d programs use standard libGL for -devel files)
- Proper directory-ownerships

Should items:
- Includes upstream licence file (licence.html)
- No insane scriplets
- No unnescesarry requires

Package does have an exclusive arch, and the guidlines do have something to say
about this (from http://fedoraproject.org/wiki/PackageReviewGuidelines):
Each architecture listed in ExcludeArch needs to have a bug filed in bugzilla,
describing the reason that the package does not compile/build/work on that
architecture. The bug number should then be placed in a comment, next to the
corresponding ExcludeArch line. New packages will not have bugzilla entries
during the review process, so they should put this description in the comment
until the package is approved, then file the bugzilla entry, and replace the
long explanation with the bug number.

However i'm not so sure Voodoo3D hardware even works in a PPC machine, or ever
was put in a PPC machine (the only architecture your missing), so if this is the
case you can skip that step of filing a bug for it.

Please update / clarify the licence and i can sign off on the updated src.rpm
and change to FE-ACCEPT 

Glide3 which is a hard requirement for this package has the same ExclusiveArchs
(also inherited from Core) I do believe that there we're macs with voodoo's.
Atleast the Glide code is full of mac defines, unfortunatly only for the
metroworks compiler, so the chances of getting it work on mac-linux are small
and this is an impossible job with a mac. Anyone care to donate me a PPC machine?

So for the near future no PPC version of Glide3 and thus no PPC version of this
package.

I've put an SRPM and spec with fixed license at:
Spec Name or Url: http://home.zonnet.nl/jwrdegoede/Glide3-libGL.spec
SRPM Name or Url: http://home.zonnet.nl/jwrdegoede/Glide3-libGL-6.2.1-3.src.rpm

(In reply to comment #17)
> Glide3 which is a hard requirement for this package has the same ExclusiveArchs
> (also inherited from Core) I do believe that there we're macs with voodoo's.
> Atleast the Glide code is full of mac defines, unfortunatly only for the
> metroworks compiler, so the chances of getting it work on mac-linux are small
> and this is an impossible job with a mac. Anyone care to donate me a PPC machine?

I don't believe the VooDoo's in macs worked the same way they did on PC's.
On my iMac, anyway - the VooDoo2 only was used for 3D acceleration - using the
built in ATI chipset for all things 2D - and was never supported in Linux.

I suppose it is possible that some of the PCI voodoo2 cards worked differently,
but Fedora only supports new world macs, which I believe had believe (with
exception of iMac) had better built in 3D than the VooDoo2.

I know you can install Fedora on old world macs that use BootX - so if the PCI
VooDoo2 cards did ever work in Linux, it might be worth it - but I honestly
don't know that they ever did. I think they were pass through acceleration for
3D only, and that 2D was still handled by the built in video. I might be wrong
though.

Final official review:

MUST review items:
- Builds cleanly on FC5 devel.
- rpmlint output:
W: Glide3-libGL invalid-license MIT/X11
But this is ignorable, licence is compatible, it just doesn't recognise it
- Source included matches upsteam source (md5sum)
- Package name meets guidelines
- spec file name is in %{name}.spec format
- Licence (MIT/X11) is fedora extra's compatible & is included
in spec
- Spec file is in (american) english
- Does not list buildrequires that are excepted in the package guidelines
- All build dependencies are listed
- No ldconfig needed
- All files have proper permissions
- Package is not relocatable
- No duplicate files in %files section
- No missing files in %files section (only ships basic so & shell script)
- Has a proper %clean section with rm -rf $RPM_BUILD_ROOT
- Uses macro's described in PackagingGuidelines
- No entries in %doc that are required for standard program operation
- No -devel package needed (3d programs use standard libGL for -devel files)
- Proper directory-ownerships

Should items:
- Includes upstream licence file (licence.html)
- No insane scriplets
- No unnescesarry requires

FE-ACCEPTED

Hans please assign the bug to me, i don't have fedorabugs group access to do so
yet, import, make tag && make build and close this bug with NEXTRELEASE please.
Congrats on _finally_ getting it in after 3 months :-)

Imported & Build, Thanks!