Bug 198829 (wfmath)
| Summary: | Review Request: wfmath - WorldForge math libraries | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Wart <wart> |
| Component: | Package Review | Assignee: | Christopher Stone <chris.stone> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | che666, hdegoede |
| 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-07-27 18:31:29 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, 198833, 198837 | ||
|
Description
Wart
2006-07-13 22:41:44 UTC
- 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)
(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. 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. 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 Only other comment is make check should use smpflags. APPROVED. 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. 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. |