Bug 481355 - Review Request: nurbs++ - A C++ library to manipulate and create NURBS curves and surfaces.
Review Request: nurbs++ - A C++ library to manipulate and create NURBS curves...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
10
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-23 12:49 EST by D Haley
Modified: 2009-08-08 15:25 EDT (History)
4 users (show)

See Also:
Fixed In Version: 3.0.11-6.fc11
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-07-15 13:53:23 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description D Haley 2009-01-23 12:49:18 EST
SPEC URL: http://dhd.selfip.com/427e/nurbs++.spec
SRPM URL: http://dhd.selfip.com/427e/nurbs++-3.0.11-1.fc10.src.rpm

Koji Scratch:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1076942

rpmlint:
$ rpmlint -i nurbs++.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
$ rpmlint ../SRPMS/nurbs++-3.0.11-1.fc10.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
$ rpmlint ../RPMS/i386/nurbs++-3.0.11-1.fc10.i386.rpm 
nurbs++.i386: W: shared-lib-calls-exit /usr/lib/libnurbsf.so.0.1.0 exit@GLIBC_2.0
nurbs++.i386: W: shared-lib-calls-exit /usr/lib/libnurbsd.so.0.1.0 exit@GLIBC_2.0
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

The shared lib calls have been posted upstream.

Description:
A C++ library to manipulate and create NURBS curves and surfaces. Also
featuring vector, matrix, and OpenGL support classes.
Comment 2 D Haley 2009-01-23 20:14:13 EST
SPEC URL: http://dhd.selfip.com/427e/nurbs++.spec
SRPM URL: http://dhd.selfip.com/427e/nurbs++-3.0.11-2.fc10.src.rpm

* Sat Jan 24 2009 <mycae(a!t)yahoo.com> 3.0.11-2 
- Use version macro in URL name
Comment 3 Jason Tibbitts 2009-06-04 19:29:02 EDT
Does not build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1394170
Comment 4 D Haley 2009-06-06 01:39:50 EDT
SPEC URL: http://dhd.selfip.com/427e/nurbs++-4.spec
SRPM URL: http://dhd.selfip.com/427e/nurbs++-3.0.11-4.fc10.src.rpm

Thanks for letting me know about the build fail. I updated it to build under dist-f11 (gcc 4.4):

Koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1396600

Changelog:
* Sat Jun 06 2009 <mycae(a!t)yahoo.com> 3.0.11-4
- Patch to fix build for gcc-4.4
- Add buildrequires to fix autoreconf/libtool command missing in f11 koji
* Sat Jan 24 2009 <mycae(a!t)yahoo.com> 3.0.11-3
- Modified patch to enable opengl build
Comment 5 Mamoru TASAKA 2009-06-20 13:34:21 EDT
I will review this shortly. Instead I will appreciate it if
you would review either of my review requests
(bug 504707, bug 504710, bug 505406 or bug 506168)
Comment 6 Mamoru TASAKA 2009-06-20 14:17:48 EDT
Well,
- COPYING file is empty
- No other files indicates under which license the overall of this
  software is released
- There are some codes which contains no license information

So I must say the license of this software is unclear. Would you
ask the upstream about this?
Comment 7 D Haley 2009-06-27 01:46:09 EDT
I emailed upstream several days ago, two of the listed addresses are no longer valid, and no response from the other maintainer so far.
Comment 8 Mamoru TASAKA 2009-07-01 10:13:08 EDT
Once I want to hear spot's opinion.
Comment 9 Tom "spot" Callaway 2009-07-06 09:55:10 EDT
Did some digging here, and found a copy of the original website, which was current at the time of the last release of libnurbs (aka, nurbs++), 3.0.11.

http://web.archive.org/web/20021128215257/libnurbs.sourceforge.net/

From that, it is clear that the author intended nurbs++ to be under the LGPLv2+. Looking at the CVS history, the author appears to have been the only person who has checked in files, so I have no reason to assume that the unlicensed files come from questionable licensing.

In addition, most of the unlicensed files are little more than stubs for c++ (e.g. nurbs/f_nurbsS_sp.cpp), the only unlicensed files of any real significance are:

numerical/statistic.cpp
numerical/chebexp.cpp
numerical/integrate.h
numerical/fft.cpp

Based on that, I think it is safe to assume that all of the unlicensed files are LGPLv2+.

So, please fix the license tag to be:

License: LGPLv2+

If upstream ever comes back to life, it would still be nice if they fixed the licensing on the unlicensed files (along with the other issues highlighted in the sourceforge ticket), but I won't hold this review up for it.

Lifting FE-Legal.
Comment 10 Mamoru TASAKA 2009-07-06 10:29:47 EDT
Okay, thank you for clarifying.
Comment 11 Tom "spot" Callaway 2009-07-06 10:38:02 EDT
Thank you for your continued thoroughness in identifying and reviewing these issues!
Comment 12 Mamoru TASAKA 2009-07-06 13:20:47 EDT
For 3.0.11-4:

* License
  - As spot commented, please change the license
    tag to "LGPLv2+".

* Conditional BuildRequires
-----------------------------------------------------
   274  checking for cppunit-config... no
-----------------------------------------------------
  - cppunit-devel is available on Fedora. Would you try
    to add "BR: cppunit-devel" ?

  - Also configure.in suggests that ImageMagick support
    should be enabled by default. Would you try to
    add "BR: ImageMagick-devel"?

* Man files
----------------------------------------------------
%files
%{_datadir}/man/man1/%{name}-config.1.gz

%files devel
%{_bindir}/nurbs++-config
---------------------------------------------------
 - For macros:
    * Please use %{_mandir} for %{_datadir}/man
    * If you use %{name} macro for %{name}-config.1.gz, please
      also use %{name} in %{_bindir}/nurbs++-config
  Then:
  - %{name}-config.1.gz man file should belong to -devel subpackage,
    not to main package.

* Undefined non-weak symbols
  - $ rpmlint nurbs++ shows lots of rpmlint warnings related to
    undefined non-weak symbols:
---------------------------------------------------
nurbs++.i586: W: undefined-non-weak-symbol /usr/lib/libnurbsd.so.0.1.0 _ZTIN4PLib11ClassPOvoidIdEE
nurbs++.i586: W: undefined-non-weak-symbol /usr/lib/libnurbsd.so.0.1.0 _ZTIN4PLib6MatrixIfEE
nurbs++.i586: W: undefined-non-weak-symbol /usr/lib/libnurbsd.so.0.1.0 _ZTIN4PLib6MatrixIdEE
....
....
nurbs++.i586: W: undefined-non-weak-symbol /usr/lib/libnurbsf.so.0.1.0 _ZTIN4PLib11ClassPOvoidIfEE
nurbs++.i586: W: undefined-non-weak-symbol /usr/lib/libnurbsf.so.0.1.0 _ZTIN4PLib6MatrixIfEE
nurbs++.i586: W: undefined-non-weak-symbol /usr/lib/libnurbsf.so.0.1.0 _ZTIN4PLib6MatrixIdEE
---------------------------------------------------

    You can see these undefined non-weak symbols also by:
---------------------------------------------------
$ ldd -r /usr/lib/libnurbsd.so.0.1.0 >/dev/null
undefined symbol: _ZTIN4PLib11ClassPOvoidIdEE	(/usr/lib/libnurbsd.so.0.1.0)
undefined symbol: _ZTIN4PLib6MatrixIfEE	(/usr/lib/libnurbsd.so.0.1.0)
undefined symbol: _ZTIN4PLib6MatrixIdEE	(/usr/lib/libnurbsd.so.0.1.0)
.......
---------------------------------------------------
    It seems that
    - libnurbsd.so.0 should be linked also against
      * libmatrix.so
      * libmatrixN.so
      * libmatrixI.so
    - libnurbsf.so should be linked also against
      * The above 3 libraries
      * Also libGL.so, libGLU.so
    For example:
----------------------------------------------------
$ LD_PRELOAD=/usr/lib/libmatrix.so.1:/usr/lib/libmatrixN.so.1:/usr/lib/libmatrixI.so.1:/usr/lib/libGL.so:/usr/lib/libGLU.so  ldd -r /usr/lib/libnurbsf.so 
	linux-gate.so.1 =>  (0x00c8b000)
	/usr/lib/libmatrix.so.1 (0x007f0000)
	/usr/lib/libmatrixN.so.1 (0x00aaa000)
	/usr/lib/libmatrixI.so.1 (0x00eea000)
	/usr/lib/libGL.so (0x00110000)
	/usr/lib/libGLU.so (0x00ce0000)
	libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0x003e5000)
	libm.so.6 => /lib/libm.so.6 (0x008c7000)
	libc.so.6 => /lib/libc.so.6 (0x00184000)
	libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x00b61000)
	libX11.so.6 => /usr/lib/libX11.so.6 (0x004dc000)
	libXext.so.6 => /usr/lib/libXext.so.6 (0x006c2000)
	libXxf86vm.so.1 => /usr/lib/libXxf86vm.so.1 (0x00300000)
	libXdamage.so.1 => /usr/lib/libXdamage.so.1 (0x00327000)
	libXfixes.so.3 => /usr/lib/libXfixes.so.3 (0x00305000)
	libdrm.so.2 => /usr/lib/libdrm.so.2 (0x0030a000)
	libpthread.so.0 => /lib/libpthread.so.0 (0x00c22000)
	libdl.so.2 => /lib/libdl.so.2 (0x00caa000)
	/lib/ld-linux.so.2 (0x00761000)
	libxcb.so.1 => /usr/lib/libxcb.so.1 (0x00730000)
	libXau.so.6 => /usr/lib/libXau.so.6 (0x00bbd000)
	librt.so.1 => /lib/librt.so.1 (0x00315000)
----------------------------------------------------

* %changelog format
  - It is useful in Fedora CVS that one line is put between each %changelog
    entry like:
----------------------------------------------------
* Sat Jun 06 2009 <mycae(a!t)yahoo.com> 3.0.11-4
- Patch to fix build for gcc-4.4
- Add buildrequires to fix autoreconf/libtool command missing in f11 koji

* Sat Jan 24 2009 <mycae(a!t)yahoo.com> 3.0.11-3
- Modified patch to enable opengl build

* Sat Jan 24 2009 <mycae(a!t)yahoo.com> 3.0.11-2
....
----------------------------------------------------
Comment 13 D Haley 2009-07-07 10:21:54 EDT
SPEC URL: http://dhd.selfip.com/427e/nurbs++-5.spec
SRPM URL: http://dhd.selfip.com/427e/nurbs++-3.0.11-5.fc10.src.rpm 


>  - As spot commented, please change the license
>    tag to "LGPLv2+".
Done

>  - Also configure.in suggests that ImageMagick support
>    should be enabled by default. Would you try to
>    add "BR: ImageMagick-devel"?
Done

>- cppunit-devel is available on Fedora. Would you try
>    to add "BR: cppunit-devel" ?

I avoided this as unfortunately their cppunit tests do not compile, and I am not familiar with that particular framework. I suspect this would requires some modest level of patching to fix.

====
test_matrix.cpp:150: error: explicit instantiation of 'void CppUnit::assertEquals(const T&, const T&, CppUnit::SourceLine, const std::string&) [with T = double]' in namespace 'CppUnit::TestAssert' (which does not enclose namespace 'CppUnit')
test_matrix.cpp:151: error: explicit instantiation of 'void CppUnit::assertEquals(const T&, const T&, CppUnit::SourceLine, const std::string&) [with T = float]' in namespace 'CppUnit::TestAssert' (which does not enclose namespace 'CppUnit')
test_matrix.cpp:152: error: explicit instantiation of 'void CppUnit::assertEquals(const T&, const T&, CppUnit::SourceLine, const std::string&) [with T = int]' in namespace 'CppUnit::TestAssert' (which does not enclose namespace 'CppUnit')
test_matrix.cpp:155: error: 'NoExceptionExpected' is not a member of 'CppUnit'
test_matrix.cpp:155: error: 'NoExceptionExpected' is not a member of 'CppUnit'
test_matrix.cpp:155: error: wrong number of template arguments (2, should be 1)
re ';' token


et cetera...
====


>    * Please use %{_mandir} for %{_datadir}/man
>    * If you use %{name} macro for %{name}-config.1.gz, please
>      also use %{name} in %{_bindir}/nurbs++-config

Fixed.

>  - %{name}-config.1.gz man file should belong to -devel subpackage,
>    not to main package.

Fixed.

>  - $ rpmlint nurbs++ shows lots of rpmlint warnings related to undefined non-weak symbols:

Odd, my rpmlint (which is up-to-date) does not catch this error. Anyway, its clearly was problem, and is now Fixed.

[makerpm@box SPECS]$ ldd -r /usr/lib/libnurbsd.so.0.1.0 > /dev/null 
[makerpm@box SPECS]$ 


> - It is useful in Fedora CVS that one line is put between each %changelog
Fixed.
Comment 14 Mamoru TASAKA 2009-07-07 14:48:15 EDT
(In reply to comment #13)
> >  - Also configure.in suggests that ImageMagick support
> >    should be enabled by default. Would you try to
> >    add "BR: ImageMagick-devel"?
> Done

- Ah, no... I missed that in fact all ImageMagick related parts
  are commented out in configure.in and currently ncurbs++ does not
  use ImageMagick. You can remove BR: ImageMagick-devel, sorry.

> >- cppunit-devel is available on Fedora. Would you try
> >    to add "BR: cppunit-devel" ?
> 
> I avoided this as unfortunately their cppunit tests do not compile, and I am
> not familiar with that particular framework. I suspect this would requires some
> modest level of patching to fix.

- Okay. It seems that cppunit is used only to build test program.
 
> >  - $ rpmlint nurbs++ shows lots of rpmlint warnings related to undefined non-weak symbols:
> 
> Odd, my rpmlint (which is up-to-date) does not catch this error. Anyway, its
> clearly was problem, and is now Fixed.

- rpmlint can also be used for "installed" rpm. Then I again
  failed to notice that rpmlint complains also about this warnings
  for both libmatrixI.so and libmatrixN.so....
  Both needs linking against libmatrix.so . Please also this fix.
Comment 15 D Haley 2009-07-08 10:27:13 EDT
SPEC URL: http://dhd.selfip.com/427e/nurbs++.spec
SRPM URL: http://dhd.selfip.com/427e/nurbs++-3.0.11-6.fc10.src.rpm

RPMLint:
$ sudo rpm -i ../RPMS/i386/nurbs++-3.0.11-6.fc10.i386.rpm && rpmlint ../RPMS/i386/nurbs++-*6.fc10* ../SRPMS/nurbs++-3.0.11-6.fc10.src.rpm  nurbs++.spec nurbs++
nurbs++.i386: W: shared-lib-calls-exit /usr/lib/libnurbsd.so.0.1.0 exit@GLIBC_2.0
nurbs++.i386: W: shared-lib-calls-exit /usr/lib/libnurbsf.so.0.1.0 exit@GLIBC_2.0
nurbs++.i386: W: shared-lib-calls-exit /usr/lib/libnurbsd.so.0.1.0 exit@GLIBC_2.0
nurbs++.i386: W: shared-lib-calls-exit /usr/lib/libnurbsf.so.0.1.0 exit@GLIBC_2.0
5 packages and 1 specfiles checked; 0 errors, 4 warnings.


> rpmlint can also be used for "installed" rpm.
I should read the docs more thoroughly. Thanks for the tip. 

> You can remove BR: ImageMagick-devel, sorry.
I missed this one too. I checked the cpp file and saw the Magick calls, but didn't check the configure.in closely enough -- my bad & thanks for catching. Fixed.

> libmatrixI.so and libmatrixN.so....
> Both needs linking against libmatrix.so . Please also this fix.  
Fixed.
Comment 16 Mamoru TASAKA 2009-07-08 12:54:19 EDT
Now okay.

-------------------------------------------------------
  This package (nurbs++) is APPROVED by mtasaka
-------------------------------------------------------
Comment 17 D Haley 2009-07-08 20:19:39 EDT
New Package CVS Request
=======================
Package Name: nurbs++
Short Description: A C++ library to manipulate and create NURBS curves and surfaces.
Owners: mycae
Branches: F-10 F-11
InitialCC:
Comment 18 Jason Tibbitts 2009-07-09 23:36:49 EDT
CVS done.
Comment 19 Fedora Update System 2009-07-10 11:16:34 EDT
nurbs++-3.0.11-6.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/nurbs++-3.0.11-6.fc11
Comment 20 Fedora Update System 2009-07-11 00:51:16 EDT
nurbs++-3.0.11-6.fc10.1 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/nurbs++-3.0.11-6.fc10.1
Comment 21 Mamoru TASAKA 2009-07-11 12:28:33 EDT
Please rebuild this package also on devel (F-12).
Comment 22 Mamoru TASAKA 2009-07-15 13:53:23 EDT
Now closing.
Comment 23 Fedora Update System 2009-08-01 19:55:14 EDT
nurbs++-3.0.11-6.fc10.1 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 24 Fedora Update System 2009-08-08 15:24:53 EDT
nurbs++-3.0.11-6.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.