I'm not familiar with Erlang so I thought I'd take a look, but I'm having some
problems building in mock.

The build likes to pause for quite some time running inet_gethost; I'm not sure
what this is.  I enabled parallel make and things went much faster (the failure
below did not change) but if it's talking to the 'net then the build system
might have a problem.

The build fails:
+ make DESTDIR=/var/tmp/esdl-0.95.0630-2-root-mockbuild install
Found erlang at /usr/lib/erlang
Installing esdl-0.95.0630 in
/var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630
mkdir /var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630/src
mkdir
/var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630/c_src
mkdir
/var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630/include
mkdir /var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630/doc
mkdir
/var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630/ebin
mkdir
/var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630/priv
cp license.terms Readme*
/var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630
cp src/*.?rl
/var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630/src
cp c_src/*.[ch]
/var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630/c_src
cp include/*.hrl
/var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630/include
cp doc/*.html
/var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630/doc
cp ebin/*beam
/var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630/ebin
cp priv/*.*
/var/tmp/esdl-0.95.0630-2-root-mockbuild/usr/lib/erlang/lib/esdl-0.95.0630/priv
cp: cannot stat `priv/*.*': No such file or directory
make: *** [install] Error 1
error: Bad exit status from /var/tmp/rpm-tmp.57229 (%install)

There have been missing buildrequires.
The fixed package is at the same place.

OK, it builds now.  Still no parallel make, although it doesn't seem to hurt
anything when I enable it.

rpmlint complains about all of the .h and .c files from the c_src directory.  I
don't know much about Erlang but I doubt these are needed at runtime.  Can they
be shifted out to a -devel package without breaking things?

rpmlint also complains about the lack of a changelog entry for your last changes.

Finally, since this is an Erlang package (perhaps the first in Extras), might we
consider avoiding some of the mess that Python is in and adopt a reasonable
Perl-like naming convention like erlang-SDL or erlang-esdl?

(In reply to comment #3)
> OK, it builds now.  Still no parallel make, although it doesn't seem to hurt
> anything when I enable it.
added %{?_smp_mflags}

> rpmlint complains about all of the .h and .c files from the c_src directory.  I
> don't know much about Erlang but I doubt these are needed at runtime.  Can they
> be shifted out to a -devel package without breaking things?
I split off a devel package.
There is now an problem report by rpmlint indicating that
there are only non-binaries in /usr/lib in esdl-devel.
This should be ignored.
 
> Finally, since this is an Erlang package (perhaps the first in Extras), might we
> consider avoiding some of the mess that Python is in and adopt a reasonable
> Perl-like naming convention like erlang-SDL or erlang-esdl?
This might be worth considering. However, AFAIK, other
distributions don't do it, so it would possibly create some confusion.

Package at the usual place.

I believe the aim is to maintain consistency within Fedora even if it means we
use different package names than other distributions.  The package naming
guidelines are pretty clear about what addon packages should be named, and I
think this qualifies as an addon package.  "esdl" just gives no reasonable
indication that this is an Erlang package.

I'm not trying to be a pain; perhaps someone else could weigh in with their opinion.

Ok, I renamed the package, but I also included a "Provides: esdl".

http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/erlang-esdl-0.95.0630-3.fc5.src.rpm

This seems acceptable. I'll add "erlang-*" to the NamingGuidelines.

Tom, are you saying that the "Provides: esdl" is acceptable as well?  I'm
vaguely disquieted by it because it shouldn't be necessary; nothing in Extras
will require it.

I'll go ahead and work up a full review in any case.

(In reply to comment #8)
> Tom, are you saying that the "Provides: esdl" is acceptable as well?  I'm
> vaguely disquieted by it because it shouldn't be necessary; nothing in Extras
> will require it.
You are right, I'll remove it.

I'll work on the assumption that you'll remove the Provides: from the next version.

The package builds fine; I even learned a bit about erlang so that I could
install and test it.  Unfortunately there are a few problems.  The only thing I
think is blocking is the errant provide of sdl_driver.so, although I'd like to
know what's up with the debuginfo RPM.

rpmlint says:
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl_util.c
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl_audio.c
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl_wrapper.c
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl_glext.h
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl_glu.c
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl_gen.c
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl_glext.c
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl.h
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl_driver.c
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl_spec.c
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl_video.c
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl_events.c
E: erlang-esdl-debuginfo script-without-shellbang
/usr/src/debug/esdl-0.95.0630/c_src/esdl_gl.c

I have no idea what it's on about here, or why RPM would choose to put copies of
development files into the debuginfo RPM.  I'm not inclined to hold up inclusion
of this package because of this, 

E: erlang-esdl-devel only-non-binary-in-usr-lib

This is ignorable.

Issues:
The license 2-clause BSD like with an added section indicating what rights the
US government gets (and also indicating that this doesn't limit anyone's
rights).  To me it seems clearly free; someone might want to look over it and
decide whether there a better license tag for this than "Distributable", but I
don't see this as a blocker.

The final package provides sdl_driver.so, but I'm not sure that it should since
that library is buried in /usr/lib/erlang/lib/esdl-0.95.0630/priv/.  What's your
opinion here?  Note that this also ends up in the -debuginfo package's provide
list, and I'm not sure it's even possible to filter it.

Review:
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
O license field is "Distributable", which is an accurate description.  License
text is included in the package.
* source files match upstream:
   e892f64e9c5f6eca037757e5c38667ce  esdl-0.95.0630.src.tar.gz
   e892f64e9c5f6eca037757e5c38667ce  esdl-0.95.0630.src.tar.gz-srpm
* BuildRequires are proper.
* package builds in mock (development, i386 and x86_64).
O rpmlint complains; see above.
X final provides list is not completely sane.
* shared libraries are present, but not in any of the linker's paths so there's
no need to call ldconfig.
* package is not relocatable.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* %clean is present.
O %check is not present; the tests are graphical in nature.  I ran them myself
to ensure that the package built correctly.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers and included C source files are in a -devel package.
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app.

(In reply to comment #10)
> think is blocking is the errant provide of sdl_driver.so, although I'd like to
> know what's up with the debuginfo RPM.
> 
> rpmlint says:
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl_util.c
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl_audio.c
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl_wrapper.c
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl_glext.h
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl_glu.c
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl_gen.c
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl_glext.c
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl.h
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl_driver.c
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl_spec.c
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl_video.c
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl_events.c
> E: erlang-esdl-debuginfo script-without-shellbang
> /usr/src/debug/esdl-0.95.0630/c_src/esdl_gl.c
> 
> I have no idea what it's on about here, or why RPM would choose to put copies of
> development files into the debuginfo RPM.  I'm not inclined to hold up inclusion
> of this package because of this, 

These can be fixed by removing the exec bit from the listed files using chmod in
%prep. There's no reason for .c or .h files to be executable...

(In reply to comment #10)
> The package builds fine; I even learned a bit about erlang so that I could
> install and test it.  Unfortunately there are a few problems.  The only thing I
> think is blocking is the errant provide of sdl_driver.so,
This seems not to be problem, it also happens with other packages, for
example mesa-libGL provides mga_dri.so, although this is in /usr/lib/dri,
or epiphany-extensions and many others.

> although I'd like to
> know what's up with the debuginfo RPM.
I can't say anything about this. On the other hand, maybe
the c_src directory can be excluded entirely.

> Issues:
> The license 2-clause BSD like with an added section indicating what rights the
> US government gets (and also indicating that this doesn't limit anyone's
> rights).  To me it seems clearly free; someone might want to look over it and
> decide whether there a better license tag for this than "Distributable", but I
> don't see this as a blocker.
I am not familiar with legal matters, so I must rely on your judgement. What
do you propose?

(In reply to comment #11)
> > I have no idea what it's on about here, or why RPM would choose to put copies of
> > development files into the debuginfo RPM.  I'm not inclined to hold up inclusion
> > of this package because of this, 
> 
> These can be fixed by removing the exec bit from the listed files using chmod in
> %prep. There's no reason for .c or .h files to be executable...
Indeed. Strangely the installed .c and .h files are non-executable, whereas
the files in build dir are.
I included a fix in the spec file.
http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/erlang-esdl-0.95.0630-3.fc5.src.rpm

To Paul:

They are chmod'ed after they're installed, and they don't have execute
permission in the final package.  So RPM is looking in the build tree for debug
stuff instead of the install tree?  That's pretty odd; I'll have to poke around
further.

To Gérard:

Good point about the extra provide; all sorts of packages (even language addon
packages like perl-PDL) do this without issue, so I'll drop my objection.

It would certainly simply a few things of c_src were simply omitted, but I don't
know enough about erlang to understand whether that's possible.  I suggest
trying out Paul's suggestion to see if it keeps RPM from stuffing unnecessary
things into the debuginfo package.

About the license, I have no problems with it as it stands; I was hoping that
someone else would have some advice.  The license text is at
http://www.math.uh.edu/~tibbs/esdl-license.terms if anyone wants to take a look.

(In reply to comment #14)
> To Paul:
> 
> They are chmod'ed after they're installed, and they don't have execute
> permission in the final package.  So RPM is looking in the build tree for debug
> stuff instead of the install tree?  That's pretty odd; I'll have to poke around
> further.

My understanding of the debuginfo package is that it contains symbol tables etc.
for the binaries that have been built (the information that gets removed by
"strip -g"). This information comes from the build area for all packages; most
don't install .c or .h files anywhere anyway.

I removed the c_src directory from erlang-esdl-devel. I compiled
Wings3D against erlang-esdl and erlang-esdl-devel and later removed
erlang-esdl-devel. Wings3D runs well, so I presume this is alright.
http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/erlang-esdl-0.95.0630-3.fc5.src.rpm

I think you linked to an older package there, but I found the -4 package in that
directory and gave it a spin.  Everything looks good; rpmlint is quiet save for
the "only-non-binary-in-usr-lib" thing which is OK.

APPROVED

Builds successfully on FC-4, FC-5 and FC-6.
Now it should be possible to review wings.