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 188207
Summary: | Review Request: erlang-esdl - SDL bindings for Erlang | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Gérard Milmeister <gemi> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | j |
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-04-25 21:38:01 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, 188208 |
Description
Gérard Milmeister
2006-04-06 23:16:26 UTC
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. |