Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.

Bug 188974

Summary: Review Request: mesa-libGLw - Xt / Motif OpenGL widgets
Product: [Fedora] Fedora Reporter: Adam Jackson <ajax>
Component: Package ReviewAssignee: Rex Dieter <rdieter>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: hdegoede, mharris, pertusus, tcallawa
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-09-29 17:22:21 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 163779    
Attachments:
Description Flags
patch for spec file to simplify %build and %install
none
remove -L from link, and leave non existent dirs in PROGRAM_DIRS none

Description Adam Jackson 2006-04-14 03:37:22 UTC
Spec URL: http://people.freedesktop.org/~ajax/fedora/libGLw/libGLw.spec
SRPM URL: http://people.freedesktop.org/~ajax/fedora/libGLw/libGLw-1.0-1.src.rpm

Description:
libGLw is a library for Xaw and Motif OpenGL widgets.  Currently it's built as one of the products of the 'mesa' package in core.  This is foolish for a number of reasons:

- The lib is _extremely_ stable, bumping it for every Mesa release is needless churn
- No packages in core (or extras!) depend on libGLw
- libGLw introduces a dependency on openmotif, which I would like to move out of core, because it's no longer 1996.

Upon acceptance into extras I'll be happy to remove it from the core Mesa build and update the Requires/Obsoletes appropriately.

Comment 1 Patrice Dumas 2006-04-14 07:30:45 UTC
Some quick comments after looking at the spec:

* It would be better to have your real name on the bug report.
Not a big deal, it is in the spec anyway.
* the Buildroot is no the right one
http://fedoraproject.org/wiki/Packaging/Guidelines#head-f196e7b2477c2f5dd97ef64e8eacddfb517f1aa1
* no need to conditionalize openmotif support, it should always 
be true in fedora extras
* the non free file shouldn't be distributed. what I do in such 
cases is provide with a script in SourceXX that unpack the upstream 
tarball, remove the offending files and repack, such that a reviewer 
can easily reproduce what you did in the SOURCES directory. An example 
where I do that is grads (with the script grads-remove-files and the 
list of files grads-removed-files-list) although you could do more simply
since you only have one file to remove.

Comment 2 Patrice Dumas 2006-04-14 07:48:55 UTC
Another remark, maybe
BuildRequires: mesa-libGL-devel
should be
BuildRequires: libGL-devel
but I haven't really investigated if there is a need for the mesa 
implementation or not.

Comment 3 Patrice Dumas 2006-04-14 08:21:38 UTC
2 more comments:

* You should add to %docs Mesa-6.5/src/glw/README
as it contains the licence.
* I am not convinced that it is right to make a source package out of a 
whole package by taking only bits of what is in the tarball while the 
remaining is in another source package. May be worth asking on the extras
list. Couldn't the upstream project be convinced to split the tarball?

Comment 4 Adam Jackson 2006-04-14 15:08:27 UTC
(In reply to comment #1)
> * the non free file shouldn't be distributed. what I do in such 
> cases is provide with a script in SourceXX that unpack the upstream 
> tarball, remove the offending files and repack, such that a reviewer 
> can easily reproduce what you did in the SOURCES directory. An example 
> where I do that is grads (with the script grads-remove-files and the 
> list of files grads-removed-files-list) although you could do more simply
> since you only have one file to remove.

That feels like such overkill.  I'll give it a shot though.

> * I am not convinced that it is right to make a source package out of a 
> whole package by taking only bits of what is in the tarball while the 
> remaining is in another source package. May be worth asking on the extras
> list. Couldn't the upstream project be convinced to split the tarball?

No, they can't.  This battle has been fought and lost before.  We already slice
up the default build of Mesa, since we don't use its libglut for being non-free.

Other changes applied, along with some fixed %package statements to make the
binary rpm not be named libGLw-libGLw-*.  New versions uploaded to the same URLs
as before.

Comment 5 Patrice Dumas 2006-04-14 23:05:24 UTC
(In reply to comment #4)

> That feels like such overkill.  I'll give it a shot though.

If you speak about the idea of a script to remove the file, I agree
but if it is about removing the file I disagree. A non free file
cannot be included in a fedora extra srpm.

http://fedoraproject.org/wiki/ForbiddenItems#head-0977d53a9aaae33e484bc92e8504bf0a3ef85e52

> No, they can't.  This battle has been fought and lost before.  We already slice
> up the default build of Mesa, since we don't use its libglut for being non-free.

It is not slicing, it is removing a non free part. But anyway I don't
feel competent enough for this issue to have an interesting point of
view - and because of that issue I don't want to assign that bug to me.
But somebody with an idea on that subject may come and advise, otherwise
I believe it should be an interesting issue to post on the fedora-extras 
list.

> Other changes applied, along with some fixed %package statements to make the
> binary rpm not be named libGLw-libGLw-*.  New versions uploaded to the same URLs
> as before.

It may be better to bump the release and provide a new srpm, 
with a changelog entry stating the differences coming from the 
review process except when changes are very small. You can have a
look at other fedora extras review for examples.

I'll try to comment on all the other issues, however. Here are 
the comments:

* remove the server name (internap) from the source url
* with_dri isn't set anywhere... It certainly should be 0 or 1.
  As is, it leads to an error in the target script:
++ ./redhat-mesa-target '%{with_dri}' i386
./redhat-mesa-target: line 14: [: %{with_dri}: integer expression expected
* currently the mesa-6.5-build-config.patch doesn't lead to
  the wanted effects, because, due to the error above, the 
  configs/linux file is used instead of configs/linux-dri
* the CFLAGS used are not those from the RPM_OPT_FLAGS, but those 
  defined in configs/linux. Maybe the CFLAGS line should be removed 
  in the mesa-6.5-build-config.patch (whatever the right file config
  file is).
* rpmlint complains that in my opinion should be acted upon:
W: libGLw summary-ended-with-dot Xaw / Motif OpenGL widgets.
W: libGLw incoherent-version-in-changelog 6.5-1 1.0-1
E: libGLw-devel obsolete-not-provided Mesa-devel
E: libGLw-devel obsolete-not-provided XFree86-devel
E: libGLw-devel obsolete-not-provided xorg-x11-devel
E: libGLw-devel no-ldconfig-symlink /usr/lib/libGLw.so

* rpmlint complains that should in my opinion be ignored:
W: libGLw invalid-license MIT/X11
W: libGLw strange-permission redhat-mesa-target 0755

* Are the Obsolete really necessary?

Comment 6 Ralf Corsepius 2006-04-19 07:21:22 UTC
2 remarks:
- The summary isn't quite correct: LibGLw provides Xt and Xm OpenGL widgets.
It doesn't provide Xaw widgets.

- Instead of building a standalone libGLw from Mesa's tarball, wouldn't it be
easier to use SGI's original sources (I don't know if and where they still are
available as a separate tarball) and to patch them to Mesa's state, or even to
drop Mesa's libGLw in favor of SGI's?




Comment 7 Mike A. Harris 2006-04-25 15:32:14 UTC
(In reply to comment #6)
> 2 remarks:
> - The summary isn't quite correct: LibGLw provides Xt and Xm OpenGL widgets.
> It doesn't provide Xaw widgets.
> 
> - Instead of building a standalone libGLw from Mesa's tarball, wouldn't it be
> easier to use SGI's original sources (I don't know if and where they still are
> available as a separate tarball) and to patch them to Mesa's state, or even to
> drop Mesa's libGLw in favor of SGI's?

Possibly.  I'm not 100% certain, but I think SGI's is what was present in
XFree86 and X.Org monolithic source tree.

Might want to have the license police go over it with a fine tooth comb
first though, as ISTR there being some bad-juju SGI licenses in the tree,
although I don't remember if that was for libGLw, or for GLX.  Ajax probably
remembers though.


Comment 8 Ralf Corsepius 2006-04-25 15:59:17 UTC
(In reply to comment #7)
> Possibly.  I'm not 100% certain, but I think SGI's is what was present in
> XFree86 and X.Org monolithic source tree.
This matches with my memory.
 
> Might want to have the license police go over it with a fine tooth comb
> first though, as ISTR there being some bad-juju SGI licenses in the tree,

c.f.
http://oss.sgi.com/cgi-bin/cvsweb.cgi/projects/ogl-sample/main/gfx/lib/glw/
http://oss.sgi.com/projects/ogl-sample/faq.html
and
http://oss.sgi.com/cgi-bin/cvsweb.cgi/inventor/libSoXt/src/
http://oss.sgi.com/cgi-bin/cvsweb.cgi/inventor/libSoXt/include
[Contains slightly modified LGPL'ed versions of the GLw widgets]



Comment 9 Mike A. Harris 2006-07-27 06:44:21 UTC
(In reply to comment #1)
> Some quick comments after looking at the spec:
> 
> * It would be better to have your real name on the bug report.
> Not a big deal, it is in the spec anyway.

Irrelevant.

> * the Buildroot is no the right one
>
http://fedoraproject.org/wiki/Packaging/Guidelines#head-f196e7b2477c2f5dd97ef64e8eacddfb517f1aa1

It'd be nice if emacs et al defaulted to the official BuildRoot tag.


> * no need to conditionalize openmotif support, it should always 
> be true in fedora extras

There's no need to conditionalize the support for many of the things
which Fedora Core and Fedora Extras packages already conditionalize.
Conditionalizing features is itself a feature, which can be useful
to rebuilders, and for a variety of other reasons.

This is not a flaw in the package, and is IMHO orthagonal to inclusion
of the package into Extras.


> * the non free file shouldn't be distributed.

I generally completely agree with that.  In this case the file is
not open-source, but it both unused, and is legally redistributable,
so it is not a problem legally or practically for it to be in the
package, however it would be nice if upstream removed it in the
future.  There's no real-world gain of removing it IMHO, and it
has the cost of having to re-do that every time there is a new
upstream release of Mesa which contains it.  Also, our tarballs
no longer match upstream's then, so can't be verified by
MD5sum or whatever.


> what I do in such 
> cases is provide with a script in SourceXX that unpack the upstream 
> tarball, remove the offending files and repack, such that a reviewer 
> can easily reproduce what you did in the SOURCES directory. An example 
> where I do that is grads (with the script grads-remove-files and the 
> list of files grads-removed-files-list) although you could do more simply
> since you only have one file to remove.

Sounds reasonable for files which are not legally redistributable, such
as mp3 decoder software, and other things which are legally encumbered.
In the case of this file, it is redistributable as-is, and is unused, so
moot.





(In reply to comment #2)
> Another remark, maybe
> BuildRequires: mesa-libGL-devel
> should be
> BuildRequires: libGL-devel
> but I haven't really investigated if there is a need for the mesa 
> implementation or not.

Yup, "BuildRequires: libGL-devel" should be used by anything needing
libGL, unless a specific implementation of OpenGL is required.

(In reply to comment #3)
> 2 more comments:
> 
> * You should add to %docs Mesa-6.5/src/glw/README
> as it contains the licence.

Sounds reasonable.


> * I am not convinced that it is right to make a source package out of a 
> whole package by taking only bits of what is in the tarball while the 
> remaining is in another source package. May be worth asking on the extras
> list. Couldn't the upstream project be convinced to split the tarball?

Adam more or less _is_ the upstream person who is splitting the tarball
essentially.(In reply to comment #4)


> (In reply to comment #1)
> > * the non free file shouldn't be distributed. what I do in such 
> > cases is provide with a script in SourceXX that unpack the upstream 
> > tarball, remove the offending files and repack, such that a reviewer 
> > can easily reproduce what you did in the SOURCES directory. An example 
> > where I do that is grads (with the script grads-remove-files and the 
> > list of files grads-removed-files-list) although you could do more simply
> > since you only have one file to remove.
> 
> That feels like such overkill.  I'll give it a shot though.

Definite overkill, with no real-world gain, and no real-world risk of
not doing it.

 
> > * I am not convinced that it is right to make a source package out of a 
> > whole package by taking only bits of what is in the tarball while the 
> > remaining is in another source package. May be worth asking on the extras
> > list. Couldn't the upstream project be convinced to split the tarball?
> 
> No, they can't.  This battle has been fought and lost before.  We already slice
> up the default build of Mesa, since we don't use its libglut for being non-free.

Wrong.  Brian Paul split Mesa's GLUT library out into MesaGLUT in the 6.4
release.  Fedora doesn't remove the files, we just don't ship the MesaGLUT
tarball.  The Mesa source just seems to have some leftover glut related
files still stuck in it.  No idea if they're just files that were missed,
or if they're needed for anything, but we don't ship them in the
binary packages, so it doesn't much matter.

 
> Other changes applied, along with some fixed %package statements to make the
> binary rpm not be named libGLw-libGLw-*.  New versions uploaded to the same URLs
> as before.

libGLw-libGLw-libGLw would be a cool package name. ;)


(In reply to comment #6)
> 2 remarks:
> - The summary isn't quite correct: LibGLw provides Xt and Xm OpenGL widgets.
> It doesn't provide Xaw widgets.
> 
> - Instead of building a standalone libGLw from Mesa's tarball, wouldn't it be
> easier to use SGI's original sources (I don't know if and where they still are
> available as a separate tarball) and to patch them to Mesa's state, or even to
> drop Mesa's libGLw in favor of SGI's?

More importantly here I think, is which upstream do you think is more
likely to fix bugs in libGLw and/or respond to developer
inquiries and bug reports, etc?  I would wager the Mesa project would be
much more responsive than would SGI, and definitely Mesa project is more
likely to provide bugfixed tarballs in an open-project style manner.



Comment 10 Patrice Dumas 2006-07-27 08:09:37 UTC
(In reply to comment #9)
> (In reply to comment #1)

> > * It would be better to have your real name on the bug report.
> > Not a big deal, it is in the spec anyway.
> 
> Irrelevant.

It is important to be able to identify the people behind fedora 
extras contributors.

> > * no need to conditionalize openmotif support, it should always 
> > be true in fedora extras
> 
> There's no need to conditionalize the support for many of the things
> which Fedora Core and Fedora Extras packages already conditionalize.
> Conditionalizing features is itself a feature, which can be useful
> to rebuilders, and for a variety of other reasons.
> 
> This is not a flaw in the package, and is IMHO orthagonal to inclusion
> of the package into Extras.

Not orthogonal, such conditionnal should be avoided unless they 
correspond with real needs. It's not a must or blocker, but simpler 
is better. For example if it is an old leftover that is not usefull 
anymore, it is usefull to point it out, if such comments are not done 
during the review they'll be never done.

> > * the non free file shouldn't be distributed.
> 
> I generally completely agree with that.  In this case the file is
> not open-source, but it both unused, and is legally redistributable,

From the comment in the spec file I wrongly assumed that the file
was not redistributable. It is much better to distribute and remove it
as it is done in the spec in that case. (I didn't had a look at the file).


> Sounds reasonable for files which are not legally redistributable, such
> as mp3 decoder software, and other things which are legally encumbered.
> In the case of this file, it is redistributable as-is, and is unused, so
> moot.

Agreed.


> Adam more or less _is_ the upstream person who is splitting the tarball
> essentially.(In reply to comment #4)

I don't understand. I went to the mesa home site and indeed the 
tarball is not split in Mesa and libGLw?

> Definite overkill, with no real-world gain, and no real-world risk of
> not doing it.

Indeed, my comment was wrong. Maybe the comment in the spec file should be 
changed to

# WARNING: The following files are copyright "Mark J. Kilgard" under the GLUT
# license and are not free software (but redistributable), so we remove them.

> More importantly here I think, is which upstream do you think is more
> likely to fix bugs in libGLw and/or respond to developer
> inquiries and bug reports, etc?  I would wager the Mesa project would be
> much more responsive than would SGI, and definitely Mesa project is more
> likely to provide bugfixed tarballs in an open-project style manner.

Indeed, but not a tarball for libGLw, a tarball for Mesa, so it's dubious
to use the same source, Mesa-*.tar.bz2 for 2 distinct srpms. It is not 
explicitly forbidden but I consider it bad practice. Maybe another reviewer
could accept that practice, or as I said above we could also ask on the
fedora-extras-list for advice.

Comment 11 Adam Jackson 2006-08-07 17:07:35 UTC
(In reply to comment #10)
> Indeed, but not a tarball for libGLw, a tarball for Mesa, so it's dubious
> to use the same source, Mesa-*.tar.bz2 for 2 distinct srpms. It is not 
> explicitly forbidden but I consider it bad practice. Maybe another reviewer
> could accept that practice, or as I said above we could also ask on the
> fedora-extras-list for advice.

Mesa has no intention of splitting out libGLw.  Mesa's packaging lacks for many
things; this is one of them.

Updated spec and srpm in the same place as before:

Spec URL: http://people.freedesktop.org/~ajax/fedora/libGLw/libGLw.spec
SRPM URL: http://people.freedesktop.org/~ajax/fedora/libGLw/libGLw-1.0-1.src.rpm

I think I've addressed everything by now.  I hope.

Comment 12 Rex Dieter 2006-08-07 17:43:30 UTC
I can review this...

Comment 13 Rex Dieter 2006-08-07 18:11:19 UTC
Looks pretty good so far... offhand,

1.  IMO, package should follow upstream name and version, ie, 
Name: mesa-libGLw
Version: 6.5
that is, unless there is (strong, but so far unclear to me?) reasoning to do 
otherwise.
If renamed, then you no longer need the
Obsoletes: mesa-libGLw

2.  These Obsoletes seem overkill to me:
Obsoletes: Mesa
Obsoletes: XFree86-libs
Obsoletes: xorg-x11-libs
These are (should be!) already Obsoleted elsewhere. IMO, no need to include 
them here.

3. -devel should
Requires: libGL-devel
since its headers include references to GL/gl.h GL/glx.h

4. (minor/cosmetic) in -devel, change
Requires: libGLw = %{version}-%{release}
to
Requires: %{name} = %{version}-%{release}
to save hassle if/when pkg is ever renamed.


Comment 14 Rex Dieter 2006-08-07 18:17:38 UTC
And 
5.  (corrolary to 2,3), since this is a pkg split, you may want to consider 
including in main pkg:
Requires: mesa-libGL >= VER-REL
and in -devel pkg
Requires: mesa-libGL-devel >= VER-REL
where VER-REL represent when the split occurred.  This could also be 
accomplished with using 
Conflicts: mesa-libGL < VER-REL
but that's potentially messier.  I'll leave it up to as to how you'd rather 
deal with this (if at all).

Address these 5 items, and we're *very* close to pkg approval.

Comment 15 Ralf Corsepius 2006-08-07 18:26:11 UTC
(In reply to comment #14)
> And 
> 5.  (corrolary to 2,3), since this is a pkg split, you may want to consider 
> including in main pkg:
> Requires: mesa-libGL >= VER-REL
> and in -devel pkg
> Requires: mesa-libGL-devel >= VER-REL

This would be a fault.

libGLw must not be tied to mesa-libGL. It should require libGL.

Comment 16 Adam Jackson 2006-08-07 20:05:35 UTC
(In reply to comment #15)

> This would be a fault.
> 
> libGLw must not be tied to mesa-libGL. It should require libGL.

So libGLw should conflict with earlier mesa instead?

Comment 17 Rex Dieter 2006-08-07 20:13:26 UTC
ajax, Either way is fine with me, but Ralf's comment has merit, so it's probably
best to go the Conflicts route.

Comment 18 Mike A. Harris 2006-08-07 20:57:57 UTC
(In reply to comment #13)
> Looks pretty good so far... offhand,
> 
> 1.  IMO, package should follow upstream name and version, ie, 
> Name: mesa-libGLw
> Version: 6.5
> that is, unless there is (strong, but so far unclear to me?) reasoning to do 
> otherwise.
> If renamed, then you no longer need the
> Obsoletes: mesa-libGLw

In theory, this is a good idea, as it would allow for someone to package SGI
libGLw, or some other implementation that could theoretically exist, which
was the intent behind existing package naming and virtual provides.  I agree
that the package version should probably match the Mesa version it is from
also.  If it is named "mesa-libGLw{,-devel}", then the Obsoletes indeed is
not needed.


> 2.  These Obsoletes seem overkill to me:
> Obsoletes: Mesa
> Obsoletes: XFree86-libs
> Obsoletes: xorg-x11-libs
> These are (should be!) already Obsoleted elsewhere. IMO, no need to include 
> them here.

libGLw was present in the above packages in older OS releases previously, and
as such, those Obsoletes lines must remain in order for package upgrading to
work properly.  They should IMHO not be removed ever.


> 3. -devel should
> Requires: libGL-devel
> since its headers include references to GL/gl.h GL/glx.h

Right

> 4. (minor/cosmetic) in -devel, change
> Requires: libGLw = %{version}-%{release}
> to
> Requires: %{name} = %{version}-%{release}
> to save hassle if/when pkg is ever renamed.

Agreed, that makes sense.

(In reply to comment #14)
> And 
> 5.  (corrolary to 2,3), since this is a pkg split, you may want to consider 
> including in main pkg:
> Requires: mesa-libGL >= VER-REL
> and in -devel pkg

This is bad for 2 reasons:

1) It forces libGLw to have a dependency on the mesa implementation of libGL,
   instead of working with _any_ libGL implementation.  Whenever possible,
   dependencies on libGL should be generic, not implementation specific.

2) rpm automatically detects a dependency on libGL during packaging and will
   add a Requires: libGL.so.1 to the package, so there is no need to hard
   code one in the spec file.  The same applies to all library packages.

> Requires: mesa-libGL-devel >= VER-REL
> where VER-REL represent when the split occurred.

Also a bad idea.  It should only contain:

BuildRequires: libGL

and in the libGLw-devel it should have:
Requires: libGL-devel

No version should be specified on either, as these virtual provides are
intentionally versionless and implementation agnostic.

> This could also be 
> accomplished with using 
> Conflicts: mesa-libGL < VER-REL
> but that's potentially messier.  I'll leave it up to as to how you'd rather 
> deal with this (if at all).

In general "Conflicts" should always be avoided, and Obsoletes used instead,
for the benefit of anaconda and yum.  Obsoletes usually does the right thing
without user intervention for upgrade handling case.  If someone has an older
version of mesa-libGLw or -devel installed, so long as all of the above
Obsoletes lines I indicated should NOT be removed are present, and so long
as Obsoletes: mesa-libGLw{,devel} is present (if the package remains named
libGLw{,-devel}), then upgrades should always work properly without any
conflicts at all.

If the Obsoletes were inadvertently or intentionally missing however, rpm
itself would see the file conflicts during install time and inform the user,
so a hard coded Conflicts line is redundant.

I'd avoid using Conflicts unless there is an actual problem it is solving
which rpm does not automatically detect on it's own, or which proper
Obsoletes lines do not fix.

Hope this helps.
 


Comment 19 Adam Jackson 2006-08-07 21:06:48 UTC
So, libGLw-6.5-1 should be

Obsoletes: mesa-libGLw

?

Comment 20 Mike A. Harris 2006-08-07 21:42:26 UTC
(In reply to comment #19)
> So, libGLw-6.5-1 should be
> 
> Obsoletes: mesa-libGLw

If the new package name stays "libGLw" and "libGLw-devel", then yeah.  That
ensures that the new package gets installed on upgrades - in theory - since
it is in Extras now, that would only happen in reality if anaconda handles
Extras during install.  But it also matters for yum upgrades too, so if
someone has mesa-libGLw installed in FC5 and upgrades to FC6 using yum,
with the above Obsoletes, they should end up with the Fedora Extras libGLw
package(s) installed and everything stays working.

HTH



Comment 21 Hans de Goede 2006-08-18 22:19:51 UTC
About the whole obsoletes discussion, since mesa-libGLw is in FC-5 already
(mesa-)libGLw can be introduced into FE for FE-6 at its earliest, FE-6 should
only be used with FC-6 in which case all those obsoletes for packages which we
haven't shipped for a while can be dropped, since those will all have been
replaced with newer packages already because they are obsoleted by packages in FC-6.

So depending on the name we should include an Obsoletes: mesa-libGLw (if we
choice libGLw as name), or no obsoletes at all.

Which name is the best depends on if we are planning to ever ship an alternative
libGLw if the answer is most likely not, then libGLw is the best name IMHO.

Last about the discussion of the conditional building with / without somestuff,
I would personally prefer to see it removed but I won't insist. The same goes
for the dri stuff, AFAIK that is irrelevant for this package when build without
the rest of Mesa and thus should preferably be removed. when it comes to spec
files less often is more.

I also have a question, who is the submitter of this package review? I assume
its ajax (ajackson), but some of the comments above have confused me.

Rex (Dieter) in the light of the openmotif saga it would be nice to get this
into extras soon, I asume you're astill going todo the review? Maybe you can
post a short list of must and should fix items so that Ajax can have a go at those.


Comment 22 Rex Dieter 2006-08-18 23:12:39 UTC
IMO, items 1-4 in comment #13 should be addressed.  In light of comments, skip
suggestion 5.  After that, we're real close.

Comment 23 Rex Dieter 2006-08-18 23:23:28 UTC
Re: Obsoletes, item 2.  I'll leave that optional(SHOULD), even though imo
they're not needed anymore, mharris feels strongly otherwise (comment #18), so
I'll defer to his uber-supreme X11 judgement.

Comment 24 Hans de Goede 2006-08-19 07:35:35 UTC
If we do item 1 (name it mesa-libGLw) then is should have a virtual provides
libGLw-devel, just like mesa-libGL-devel provides libGL-devel, etc.


Comment 25 Rex Dieter 2006-08-19 12:55:37 UTC
Hans' suggestion makes sense:
6.  after 1, main pkg SHOULD 
Provides: libGLw 
and -devel SHOULD 
Provides: libGLw-devel

Comment 26 Tom "spot" Callaway 2006-08-30 19:53:22 UTC
OK, here's a new libGLw package for consideration that resolves all the items in
comment 13 except the renaming (naming is up to the maintainer):

http://www.auroralinux.org/people/spot/review/MOTIF/libGLw-1.0-3.src.rpm

I also made some very minor spec file changes to fix this package for x86_64 and
make the proper lib symlinks.

Comment 27 Rex Dieter 2006-09-01 15:12:19 UTC
spot's update looks good.  

Per comment #13 item 1, and mharris' agreement in comment #18, please rename pkg
back to mesa-libGLw, and I'll APPROVE this.

Comment 28 Rex Dieter 2006-09-14 15:03:49 UTC
ajax, it's been 1+ months since your last comment.  You still interested in
submitting/maintaining this?

Comment 29 Adam Jackson 2006-09-18 22:31:09 UTC
Renamed back to mesa-libGLw at:

http://people.freedesktop.org/~ajax/fedora/mesa-libGLw/mesa-libGLw-1.0-4.src.rpm

Comment 30 Rex Dieter 2006-09-19 12:12:33 UTC
6.  (per comment #25), main pkg missing 
Provides: libGLw 
and -devel missing
Provides: libGLw-devel

7.  OK, since we're back to the original name, 
  a. it should also have the original 
Version: 6.5
  b. a high enough Release: to ensure proper upgrade from last version in Core:

%changelog
* Thu Jul 27 2006 Mike A. Harris <mharris> 6.5-20.fc6
- Conditionalized libGLw inclusion with new with_libGLw macro defaulting
  to 1 (enabled) for now, however since nothing in Fedora Core uses libGLw
  anymore, we will be transitioning libGLw to an external package maintained
  in Fedora Extras soon.

So, I'd suggest at least
Release: 21%{?dist}

  c. drop Obsoletes: mesa-libGLw

Address these remaining items, and I'll APPROVE this.


Comment 31 Adam Jackson 2006-09-19 15:24:14 UTC
Okay.

http://people.freedesktop.org/~ajax/fedora/mesa-libGLw/mesa-libGLw-6.5.1-0.1.src.rpm

That's already rpmnewer than 6.5.1-0.rc2 that's in core right now; I'll make
sure the core libGLw build gets turned off for mesa 6.5.1-1.

Comment 32 Adam Jackson 2006-09-19 15:50:25 UTC
Oops:

http://people.freedesktop.org/~ajax/fedora/mesa-libGLw/mesa-libGLw-6.5.1-0.2.src.rpm

Now actually uses the 6.5.1 tarball.

Comment 33 Rex Dieter 2006-09-19 16:00:13 UTC
Good work, APPROVED.

Comment 34 Patrice Dumas 2006-09-28 11:14:28 UTC
* The %build and %install section are broken. Indeed make
  don't seem to care about environment variables, and besides
  there aren't set to meaningfull values, especially in the build 
  section.

  I'll attach a patch that simplifies %build and %install a lot.
  It should be tested on a x86_64, I only verified that it is 
  right on x86, although LIB_DIR=%{_lib} is there to have things
  right.


* pkgconfig don't seems to be used in the glw build, I think this
  BR should be dropped.

* lesstif-devel allready requires libXt-devel

* is it true that -devel don't requires lesstif-devel?

* When linking, the -L are not needed because we link only
  against libs in default linker directories. 

  no need to remove the dirs from PROGRAM_DIRS they don't exist
  anyway. The only one which could be removed is util, but it
  isn't there.

  I'll attach a modified mesa-6.5.1-libGLw.patch which implements 
  that.

* rpmlint has relevant warnings, but they are suppressed by the 
  change in %build

Comment 35 Patrice Dumas 2006-09-28 11:16:12 UTC
Created attachment 137304 [details]
patch for spec file to simplify %build and %install

Comment 36 Patrice Dumas 2006-09-28 11:17:48 UTC
Created attachment 137306 [details]
remove -L from link, and leave non existent dirs in PROGRAM_DIRS

This is a replacement for mesa-6.5.1-libGLw.patch

Comment 37 Ralf Corsepius 2006-09-29 03:50:10 UTC
* is it true that -devel don't requires lesstif-devel?
No. libGLw is a Motif library and uses Motif.
 
libGLw depends upon the Motif-API it has been built against and libGLw-devel
depends up the Motif-API it has been built against.

(More precisely: libGLw depends upon the details of the XmPrimitive widget class
being provided by the motif implementation underneath)

Comment 38 Patrice Dumas 2006-09-29 06:44:21 UTC
(In reply to comment #37)
> * is it true that -devel don't requires lesstif-devel?
> No. libGLw is a Motif library and uses Motif.

So there is a missing 
Requires:  lesstif-devel
in mesa-libGLw-devel

> (More precisely: libGLw depends upon the details of the XmPrimitive widget class
> being provided by the motif implementation underneath)

Thanks, I have spotted where it is now. So it also seems to me that
there is a missing #include in libGLw headers, like

#include <Xm/PrimitiveP.h>

And since GLwDrawA.h uses XmCR_EXPOSE
there could also be a missing include for Xm/Xm.h

Comment 39 Ralf Corsepius 2006-09-29 07:27:02 UTC
(In reply to comment #38)
> (In reply to comment #37)
> > (More precisely: libGLw depends upon the details of the XmPrimitive widget class
> > being provided by the motif implementation underneath)
> 
> Thanks, I have spotted where it is now. So it also seems to me that
> there is a missing #include in libGLw headers, like
> 
> #include <Xm/PrimitiveP.h>
> 
> And since GLwDrawA.h uses XmCR_EXPOSE
> there could also be a missing include for Xm/Xm.h

That's one of the oditities about the libGLwM widgets.
mesa-libGlw is a clone of SGI's libGLw.

SGI's libGLw's headers (link provided above) did not include them and requires
apps using the GLw widgets explicitly include them when needed.

So, if you want to change this API, communicate this upstream Mesa.



Comment 40 Adam Jackson 2006-09-29 17:22:21 UTC
Imported with Patrice's changes, thanks all!

Comment 41 Patrice Dumas 2006-09-30 20:48:17 UTC
It seems to me that there is a lesstif-devel requires for 
devel package missing.