Bug 481355

URL:		http://downloads.sourceforge.net/libnurbs/nurbs%2B%2B-3.0.11.tar.bz2?modtime=1022212800&big_mirror=0
Source:		nurbs++-3.0.11.tar.bz2

should be

URL:		http://sourceforge.net/projects/libnurbs/
Source:		http://downloads.sourceforge.net/libnurbs/nurbs++-%{version}.tar.bz2

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

Does not build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1394170

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

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)

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?

I emailed upstream several days ago, two of the listed addresses are no longer valid, and no response from the other maintainer so far.

Once I want to hear spot's opinion.

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.

Okay, thank you for clarifying.

Thank you for your continued thoroughness in identifying and reviewing these issues!

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
....
----------------------------------------------------

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.

(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.

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
nurbs++.i386: W: shared-lib-calls-exit /usr/lib/libnurbsf.so.0.1.0 exit
nurbs++.i386: W: shared-lib-calls-exit /usr/lib/libnurbsd.so.0.1.0 exit
nurbs++.i386: W: shared-lib-calls-exit /usr/lib/libnurbsf.so.0.1.0 exit
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.

Now okay.

-------------------------------------------------------
  This package (nurbs++) is APPROVED by mtasaka
-------------------------------------------------------

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:

CVS done.

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

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

Please rebuild this package also on devel (F-12).

Now closing.

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.

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.