Bug 678955

Lots of issues with this package:

rpmlint opencsg-*rpm
..
opencsg.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/libopencsg.so.1.3.1 ['../lib']
opencsg.x86_64: E: library-without-ldconfig-postin /usr/lib64/libopencsg.so.1.3.1
opencsg.x86_64: E: library-without-ldconfig-postun /usr/lib64/libopencsg.so.1.3.1
opencsg.x86_64: E: non-standard-executable-perm /usr/lib64/libopencsg.so.1.3.1 0775L
opencsg-debuginfo.x86_64: E: debuginfo-without-sources

All of them are serious and need to be fixed.

* Mon Feb 21 2011 Jeff Moe <moe> - 1.3.1-3
- Add ldconfig to %%post and %%postun
- Fix rpath
- Fix library permissions

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

I'm not sure what is causing the "debuginfo-without-sources". That may take a bit longer to fix (suggestions welcome).

Thanks!

(In reply to comment #2)
> I'm not sure what is causing the "debuginfo-without-sources". 
The origin of this is this package's makefiles not honouring RPM_OPT_FLAGS and them applying strip on binaries.

Both issues can be worked around by re-generating this packages's makefiles.

I just blow out the Makefiles and qmake creates new ones which fixes both rpath and -debuginfo issues.

Also, it includes a glew 1.5.2 sub directory. I just remove this to make certain it uses Fedora's glew 1.5.7 instead.


* Sat Feb 26 2011 Jeff Moe <moe> - 1.3.1-4
- Regenerate Makefiles to fix rpath and debuginfo-without-sources
- Use Fedora's glew instead of included copy
- Remove tab from .spec

====================
Latest files:

http://repos.fedorapeople.org/repos/jebba/reprap/opencsg.spec
http://repos.fedorapeople.org/repos/jebba/reprap/14/SRPMS/opencsg-1.3.1-4.fc14.src.rpm

Info on adding it as a repo here:
http://repos.fedorapeople.org/repos/jebba/reprap/readme.html

I'll review this package.

MUST items:
- rpmlint output:
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewGenOcclusionQueriesNV
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewXGetFBConfigAttribSGIX
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewXQueryGLXPbufferSGIX
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewFramebufferTexture2DEXT
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewDepthBoundsEXT
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewDeleteOcclusionQueriesNV
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewGenFramebuffersEXT
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLEW_NV_occlusion_query
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLEW_ARB_depth_texture
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewDeleteFramebuffers
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewRenderbufferStorageEXT
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewGetQueryObjectuivARB
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewXChooseFBConfigSGIX
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLEW_ARB_framebuffer_object
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewEndOcclusionQueryNV
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewGenRenderbuffers
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLEW_ARB_texture_env_dot3
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLEW_ATI_texture_float
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLEW_ARB_occlusion_query
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLEW_NV_float_buffer
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLEW_NV_texture_rectangle
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLXEW_SGIX_fbconfig
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewXDestroyGLXPbufferSGIX
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewBindFramebufferEXT
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewFramebufferRenderbufferEXT
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewXGetCurrentDisplay
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLXEW_SGIX_pbuffer
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewDeleteQueriesARB
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLXEW_NV_float_buffer
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewFramebufferRenderbuffer
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewBindRenderbuffer
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewEndQueryARB
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewGenRenderbuffersEXT
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewBeginOcclusionQueryNV
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLEW_EXT_packed_depth_stencil
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewGetOcclusionQueryuivNV
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewDeleteRenderbuffersEXT
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewXCreateGLXPbufferSGIX
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewRenderbufferStorage
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLEW_EXT_depth_bounds_test
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewDeleteFramebuffersEXT
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewCheckFramebufferStatusEXT
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewBeginQueryARB
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewGenQueriesARB
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewXCreateContextWithConfigSGIX
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewDeleteRenderbuffers
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewBindFramebuffer
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewGenFramebuffers
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewBindRenderbufferEXT
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __GLEW_EXT_framebuffer_object
opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1 __glewCheckFramebufferStatus
opencsg.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libopencsg.so.1.3.1 /usr/lib64/libGLU.so.1
opencsg.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libopencsg.so.1.3.1 /usr/lib64/libQtGui.so.4
opencsg.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libopencsg.so.1.3.1 /usr/lib64/libQtCore.so.4
opencsg.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libopencsg.so.1.3.1 /lib64/libm.so.6
opencsg-devel.x86_64: W: no-documentation
3 packages and 1 specfiles checked; 0 errors, 56 warnings.

This means that you need to change the link command for libopencsg.so so that -lglew IS included, and -lGLU, -lQtGui, -lQtCore, and -lm are NOT included.
- package name: OK
- spec file name: OK
- packaging guidelines:
  Why are you passing -j1 to make?  If parallel make does not work (see https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make), then please include a comment stating so.
- license: OK
- license field: the license file lists an explicit exception to GPLv2, so the license field should read "GPLv2 with exceptions".
- license file in %doc: no.  You must add license.txt to %doc.
- spec file in American English: OK
- spec file legible: OK
- source file matches upstream: OK, both have md5sum 6d2f91e72ebc0af55ea2a606059b1a79
- package builds: OK, checked on Fedora 14 x86_64
- appropriate use of ExcludeArch: N/A
- all build requirements in BuildRequires: OK
- locale handling: N/A
- ldconfig in %post and %postun: OK
- no copies of system libraries: OK
- relocatable package: N/A
- package owns all dirs it creates: N/A
- no duplicates in %files: OK
- proper permissions on files: OK
- consistent use of macros: OK, although I personally detest the %{__mkdir}, %{__mv}, and %{__rm} macros :-)
- code or permissible content: OK
- large documentation in -doc: N/A
- no runtime dependencies in %doc: OK
- header files in -devel: OK
- static libraries in -static: N/A
- .so files in -devel: OK
- -devel requires main package: OK, although you should consider using %{?_isa} for multilib safety, like so:
Requires: %{name}%{?_isa} = %{version}-%{release}
- No libtool archives: OK
- desktop file for GUI applications: N/A
- do not own already-owned files/dirs: OK
- filenames are valid UTF-8: OK

SHOULD items:
- query upstream to include license text: N/A
- description and summary translations: N/A
- package builds in mock: OK
- package builds on all supported arches: did not check
- package functions as described: reviewer has no easy way to check
- sane scriptlets: OK
- subpackages depend on main package: OK
- pkgconfig files: N/A
- file dependencies: N/A
- man pages: N/A

Note that the BuildRoot tag and the %clean section in the spec file are unnecessary on currently supported versions of Fedora.  Also, the HTML files you put in %doc refer to files in the img directory; please add img to %doc.  There may be value in including (parts of) the example directory in %doc as well.

* Sat Mar  5 2011 Jeff Moe <moe> - 1.3.1-5
- Enable parallel compiling.
- Corrected license to GPLv2 with exceptions.
- Improved -devel Requires for multilib.
- Remove BuildRoot tag.
- Remove %%clean section
- Change mv and mkdir to direct commands, not macros.

=============================================================

http://repos.fedorapeople.org/repos/jebba/reprap/opencsg.spec

http://repos.fedorapeople.org/repos/jebba/reprap/14/SRPMS/opencsg-1.3.1-5.fc14.src.rpm

=============================================================

(In reply to comment #6)
> MUST items:
> - rpmlint output:
> opencsg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libopencsg.so.1.3.1
> __glewGenOcclusionQueriesNV
> ...

I don't get this when I run rpmlint:
$ rpmlint opencsg-*1.3.1-5*
opencsg-devel.i386: W: no-documentation
opencsg-devel.x86_64: W: no-documentation
7 packages and 0 specfiles checked; 0 errors, 2 warnings.

> This means that you need to change the link command for libopencsg.so so that
> -lglew IS included, and -lGLU, -lQtGui, -lQtCore, and -lm are NOT included.

OK, I will look into that.

>   Why are you passing -j1 to make?  If parallel make does not work (see
> https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make), then please
> include a comment stating so.

For awhile I thought it may have been breaking the build, but it appears to have been something else, so it is now SMP.

> - license field: the license file lists an explicit exception to GPLv2, so the
> license field should read "GPLv2 with exceptions".

Done.

> - consistent use of macros: OK, although I personally detest the %{__mkdir},
> %{__mv}, and %{__rm} macros :-)

I changed them to just plain mkdir/mv. I just noticed now I missed rm. I fixed that for the next push.

> - -devel requires main package: OK, although you should consider using %{?_isa}
> for multilib safety, like so:
> Requires: %{name}%{?_isa} = %{version}-%{release}

Done.

> - package functions as described: reviewer has no easy way to check

This is being built as a dependency of OpenSCAD, which is also in my repo, btw.

> Note that the BuildRoot tag and the %clean section in the spec file are
> unnecessary on currently supported versions of Fedora.

Removed.

>  Also, the HTML files
> you put in %doc refer to files in the img directory; please add img to %doc. 
> There may be value in including (parts of) the example directory in %doc as
> well.

Ay, missed this one on this pass. I'll take a look at it next round.

Thanks for your review! :)

I have built and added to the repo x86_64 builds for Fedora-15 dev branch. These didn't show any of the rpmlint you show above either. Are those rawhide builds?

The package does need `make -j1`, otherwise it tries to build examples before the library is done.

(In reply to comment #8)
> I have built and added to the repo x86_64 builds for Fedora-15 dev branch.
> These didn't show any of the rpmlint you show above either. Are those rawhide
> builds?

No, that was from a build on x86_64 Fedora 14.  I just tried building on Fedora 15 and got the same result.  Are you running rpmlint on the binary rpm files, or on the installed packages?  I don't believe rpmlint can do the undefined non-weak symbol checks on the former.

> The package does need `make -j1`, otherwise it tries to build examples before
> the library is done.

That can be fixed by adding an explicit dependency to the Makefile.  Put this line just after the %{_qt4_qmake} line in your spec file:

sed -i 's/^sub-example-make_default:/sub-example-make_default: sub-src-make_default/' Makefile

I built with -j5 on a quad-core machine with that line in place and with %{?_smp_mflags} added and didn't have a problem.  There is probably some way to tweak the project file so that the Makefile is generated with that dependency in the first place, but I know pretty much nothing about qmake.

Jeff, do you intend to continue with this review?

Jeff, do you intend to continue with this review?  Please respond.

I most certainly do, I hugely apologize for the delay. I am actively using the packages themselves, I just haven't done any upgrades for them. Thank you.

That's fine, I just wanted to check whether I should keep this on my radar.  I'll wait patiently now. :-)

Months have passed.  Do you still intend to move this review forward?  If so, can you give me an expected time frame?  Thanks.

The answer is that I don't know how to workaround the "undefined-non-weak-symbol" and that is what has blocked this package the whole time.  :(

Hmmm, I still don't understand qmake.  But there's another solution.  The original makefiles just need a few small tweaks to solve this problem.  Also, %{?_smp_mflags} made rpmbuild try to build the examples before the library was built, so I removed it.  I replaced your %prep and %build sections with these:

------------------------------------------------------------------------------
%prep
%setup -q -n OpenCSG-%{version}

# Use Fedora's glew
rm -rf glew/
sed -i 's/glew //' Makefile

# Get rid of an undesirable rpath, link with libGLEW, and use Fedora's CFLAGS
sed -e "s/,libopencsg\.so\.1.*/,libopencsg.so.1 -Wl,--as-needed/" \
    -e "s/-pipe -Wall -W -O2/$RPM_OPT_FLAGS/" \
    -e "s/-lGLU/-lGLU -lGLEW/" \
    -i src/Makefile
sed -i "s/-pipe -Wall -W -O2/$RPM_OPT_FLAGS/" example/Makefile

%build
make
------------------------------------------------------------------------------

and that seemed to work for me.  What do you think of that approach?

That looks doable.

I am resigning from this though. I apologize for not getting this done. Feel free to take over. I'll close this out so a new one can get started. Sorry for your time, but it looks like you are making some progress on it. Thanks!

I have submitted a new review for this package on Bug: #825489