Bug 198829 - (wfmath) Review Request: wfmath - WorldForge math libraries
Review Request: wfmath - WorldForge math libraries
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christopher Stone
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT mercator eris
  Show dependency treegraph
 
Reported: 2006-07-13 18:41 EDT by Wart
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-07-27 14:31:29 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Wart 2006-07-13 18:41:44 EDT
Spec URL: http://www.kobold.org/~wart/fedora/wfmath.spec
SRPM URL: http://www.kobold.org/~wart/fedora/wfmath-0.3.4-1.src.rpm
Description: 
WFMath provides mathematical functions for WorldForge clients.  The primary
focus of WFMath is geometric objects. Thus, it includes several shapes (boxes,
balls, lines), in addition to the basic math objects that are used to build
these shapes (points, vectors, matricies). WFMath provides a means for other
system compenents to pass geometric information around in a common format.
Comment 1 Christopher Stone 2006-07-13 21:36:57 EDT
- rpmlint output clean
- package named according to package naming guidelines
- spec filename matches base package %{name}
- package meets packaging guidelines
- package licensed with open source compatible license
- license in spec file matches actual license
- license contained in %doc
- spec file written in American english
- spec file legible
- sources match upstream
6a14f7de9d467d7b72b37da5ca5f92c5  wfmath-0.3.4.tar.gz
- package successfully compiles and builds on x86_64 FC-5
- All BuildRequires listed
- no locales
- ldconfig called in %post/%postun
- package is not relocatable
- package owns all directories it creates
- package contains no duplicate files
- file permissions set properly
- contains proper %clean section
- macro usage is consistant
- package contains permissible content
- no large documentation
- %doc does not affect runtime of application
- header files are in devel package
- pkgconfig .pc file in devel
- .so files w/o suffix in devel
- devel package requires base package
- package does not contain .la files
- not a GUI app needing a .desktop file
- package does not own files or directories owned by other packages


==== MUST ====
- Add Requires: pkgconfg to devel package
- package does not contain a %check section
 - make check fails on 1 test, investigate why (FAIL: intstring_test)
Comment 2 Wart 2006-07-14 13:33:25 EDT
(In reply to comment #1)
> ==== MUST ====
> - Add Requires: pkgconfg to devel package
> - package does not contain a %check section
>  - make check fails on 1 test, investigate why (FAIL: intstring_test)

I'll contact upstream and see if they have a fix or explanation for this test
failure.  It seems to only fail on x86_64, not i386.  I'll make a new package
once the test failure is resolved.
Comment 3 Hans de Goede 2006-07-19 03:41:17 EDT
I saw the long list of review request for worldforge, so I thought I would give
a hand. But as usual you guys (Chris this time) have beaten my to it.

So I saw this 64bit problem and I thought I would/could give a hand. The test
fails because of 2 problems

1) wfmath/MersenneTwister.h, line 77 says:
 typedef unsigned long uint32;  // unsigned integer type, at least 32 bits
 Which is strange, because you would expect a uint32 to be 32 bits period,
 I've reviewed the rest of the code and it seems to assume 32bits everywhere/
 never return random values larger then 32 bit, so I believe this should be 
 changed to:
 typedef unsigned int uint32;
 This makes things work out as the name suggests and will also be faster since
 32 bit values are faster then 64 bit (even on x86_64).
2) wfmath/intstring_test.cpp, second test uses a long to restore the unsigned
 long value of the random function. The idea here is to cause negative numbers
 for 2^31 unsigned long numbers to test negative conversion however > 2 ^ 31 
 will fit fine in the long on 64 bits, without becoming a negative number.
 Next this long is passed to IntToString, who correctly converts this to a
 string holding a large (> 2 ^ 31) positive number, after which the string
 gets passed to atoi. However atoi returns an int which cannot represent this
 bug a positive number > caboom!

 Fix use (unsigned) int 's in the test instead of long's.
Comment 4 Wart 2006-07-19 13:05:01 EDT
Thanks for the test investigation, Hans!

I built a patch to fix the test results.  Here's the new package that includes
the patch and the review MUSTFIX items:

http://www.kobold.org/~wart/fedora/wfmath.spec
http://www.kobold.org/~wart/fedora/wfmath-0.3.4-2.src.rpm
Comment 5 Christopher Stone 2006-07-20 00:48:25 EDT
Only other comment is make check should use smpflags.  APPROVED.
Comment 6 Wart 2006-07-20 13:26:46 EDT
Imported, but the build failed on ppc due to a single test failure.  I'll follow
up with upstream on this.

I'll defer closing this ticket until the test issue is resolved.
Comment 7 Wart 2006-07-27 14:31:29 EDT
I disabled 'make check' on ppc until upstream can provide a fix.  Now that this
has been imported and built on devel I'm closing the ticket.

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