Spec URL: http://jhladky.fedorapeople.org/dieharder.spec SRPM URL: http://jhladky.fedorapeople.org/dieharder-3.31.0-0.fc14.src.rpm Description: dieharder is a fairly involved random number/uniform deviate generator tester. It can either test any of its many prebuilt and linked generators (basically all of those in the Gnu Scientific Library plus others I've added) or a potentially random data-set in a file. With file input, it can manage either a variety of ASCII-formatted input or a raw binary bit string. It is thus suitable for use in testing both software RNG's and hardware RNG's. dieharder encapsulates (or will eventually encapsulate) basically all the random number tests I have been able to find -- George Marsaglia's "Diehard" battery of tests, STS (v1.5) from NIST FIPS, Knuth's tests, and more. Check in the man page or /usr/share documentation for a complete list of the tests and references where possible. It is intended to be the "Swiss army knife of random number testers", or "the last suite of random number testers you'll ever wear". Known issues: 1) rpmlint -i RPMS/i686/dieharder-3.31.0-0.fc14.i686.rpm dieharder.i686: W: shared-lib-calls-exit /usr/lib/libdieharder.so.3.31.0 exit This library package calls exit() or _exit(), probably in a non-fork() context. Doing so from a library is strongly discouraged - when a library function calls exit(), it prevents the calling program from handling the error, reporting it to the user, closing files properly, and cleaning up any state that the program has. It is preferred for the library to return an actual error code and let the calling program decide how to handle the situation. => informed the upstream. Developers has committed to fix it. 2)SMP build is not working. I have currently no clue what's wrong Please compare SMP build: http://koji.fedoraproject.org/koji/getfile?taskID=3413089&name=build.log make[1]: *** No rule to make target `../libdieharder/libdieharder.la', needed by `dieharder'. Stop. make[1]: Leaving directory `/builddir/build/BUILD/dieharder-3.31.0/dieharder' make: *** [dieharder.time] Error 2 Build on one CPU is working just fine: http://koji.fedoraproject.org/koji/taskinfo?taskID=3413118 3413118 build (dist-f15, dieharder-3.31.0-0.fc14.src.rpm) completed successfully IMHO, these two issues are not blocking and we can release the package. Thanks a lot Jirka
Just a few comments for now: - Why is the prober source0 commented out? Did you change the source somehow, so it's not this anymore?: http://www.phy.duke.edu/~rgb/General/dieharder/dieharder-%{version}.tgz - Why do you: autoreconf ./autogen.sh ? ./configure works directly without that and you apply no patches. - The description looks odd, could you delete the passages with "I have" and such thinks? - Did you notify upstream of "incorrect-fsf-address" in some files?
Some further remarks: - The package builds using -I/usr/include. This is a beginner's mistake, which causes the package to use a broken include path (The origin of this is a bug inside of the configure script) - The warnings GCC issues, indicates this package isn't 64 bit clean (Most of the spots GCC warns about are pretty simple to fix).
Hi Thomas, please see my comments bellow: (In reply to comment #1) > Just a few comments for now: > - Why is the prober source0 commented out? > Did you change the source somehow, so it's not this anymore?: > http://www.phy.duke.edu/~rgb/General/dieharder/dieharder-%{version}.tgz No, I have used the local copy of the source code when I was doing trials to speed up the turned around time. I have switched it back to the http://www.phy.duke.edu/~rgb/General/dieharder/dieharder-%{version}.tgz > > - Why do you: > autoreconf > ./autogen.sh > ? > > ./configure works directly without that and you apply no patches. Good hint. Thanks, I have fixed it now. > - The description looks odd, could you delete the passages with "I have" and > such thinks? I have just copy and paste the original description. I have now changed it so that it's neutral and objective. > - Did you notify upstream of "incorrect-fsf-address" in some files? I cannot find such warning in build.log. My latest trials are at: http://koji.fedoraproject.org/koji/getfile?taskID=3418216&name=build.log http://koji.fedoraproject.org/koji/getfile?taskID=3418217&name=build.log Can you please provide more details what you mean? Thanks! Jirka
Hi Ralf, (In reply to comment #2) > Some further remarks: > > - The package builds using -I/usr/include. > This is a beginner's mistake, which causes the package to use a broken include > path (The origin of this is a bug inside of the configure script) I can see what you mean. How can I fix it? $grep usr/incl configure oldincludedir='/usr/include' --oldincludedir=DIR C header files for non-gcc [/usr/include] Could please give me an advice how to deal with it? > > - The warnings GCC issues, indicates this package isn't 64 bit clean > (Most of the spots GCC warns about are pretty simple to fix). I have compared 32bit and 64 bit builds. It seems that in 64 bit builds following type of the warning will appear (not in i686 builds): bits.c: In function 'get_int_bit': bits.c:327:4: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Wformat] Is it what you mean? Thanks Jirka
Updated SPEC file and SRPM are at Spec URL: http://jhladky.fedorapeople.org/dieharder.spec SRPM URL: http://jhladky.fedorapeople.org/dieharder-3.31.0-0.fc14.src.rpm Latest koji trials: http://koji.fedoraproject.org/koji/getfile?taskID=3418216&name=build.log http://koji.fedoraproject.org/koji/getfile?taskID=3418217&name=build.log
Hello, I have informed the author of the software to check all the gcc warnings. He is looking into it. @Ralf: could you please comment on -I/usr/include issue? How can I fix it? Thanks Jirka
Hello, I got the updated version of the source code which will remove almost all warnings during the compilation. I have informed the upstream about the remaining ones. I have uploaded new version of SPEC file and source rpm to this location: Spec URL: http://jhladky.fedorapeople.org/dieharder.spec SRPM URL: http://jhladky.fedorapeople.org/dieharder-3.31.1-0.fc14.src.rpm Last koji trials are here: http://koji.fedoraproject.org/koji/getfile?taskID=3432846&name=build.log http://koji.fedoraproject.org/koji/getfile?taskID=3432847&name=build.log I have addressed all issues raised in comment 1 and comment 2 except for /usr/include. For this I would really appreciate a hint how to proceed. Could somebody please check the newest build.log? Thanks Jirka
Hello, is anybody interested to review this package? Thanks a lot Jirka
I never thought I would say this, but I think you have too many comments in your spec file. It's actually hurting legibility. You don't have to comment every line... Comments are for unusual situations that require explanation. Some tips: 1. The release tag should start with 1, not 0. 2. If you're not going to build for EL 5 you can remove the following from your spec file: - Buildroot: - %clean entirely - rm -rf %{buildroot} from %install - %defattr from %files 3. The devel subpackage should be arch specific. Change: Requires: %{name} = %{version}-%{release} to Requires: %{name}%{?_isa} = %{version}-%{release} 4. Leave the "*" off of: %{_includedir}/%{name}/* If you don't then the "dieharder" directory will not be owned by the package as it should be. You can leave the trailing "/". That's all I can think of right now. Richard
Hi Richard, thanks for your comments. I have cleaned-up a SPEC file a little bit, removing some unnecessary comments:-) > 1. The release tag should start with 1, not 0. I didn't know that, fixed. > 2. If you're not going to build for EL 5 you can remove the following from your spec file: My intention was to provide rpm for EPEL-5, EPEL-6. The developer has stated that he has used RHEL and Fedora to develop the software. I think it woould be nice to provide packages for RHEL. > 3. The devel subpackage should be arch specific. Fixed. > 4. Leave the "*" off of. Fixed. I have uploaded new version of SPEC file and source rpm to this location: Spec URL: http://jhladky.fedorapeople.org/dieharder.spec SRPM URL: http://jhladky.fedorapeople.org/dieharder-3.31.1-1.fc16.src.rpm Thanks Jirka
Ok, I went ahead and built the package, which was successfuly on my Fedora 15 x86_64 system, so that's good. A couple of things: 1. Here's the rpmlint output of the installed package. There's things rpmlint can catch here that it can't from just checking the package: $ rpmlint dieharder dieharder.x86_64: I: enchant-dictionary-not-found en_US dieharder.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libdieharder.so.3.31.1 /usr/lib64/libgslcblas.so.0 dieharder.x86_64: W: shared-lib-calls-exit /usr/lib64/libdieharder.so.3.31.1 exit.5 1 packages and 0 specfiles checked; 0 errors, 2 warnings. In this case it caught an unused shared library dependency (see below). This isn't a showstopper but should be reported upstream. The configure script probably just needs to be tweaked. $ rpmlint -I unused-direct-shlib-dependency unused-direct-shlib-dependency: The binary contains unused direct shared library dependencies. This may indicate gratuitously bloated linkage; check that the binary has been linked with the intended shared libraries only. 2. You don't need the "*" glob in %files for %{_bindir}/%{name} since there's nothing else to glob. I'll start the full review when I get a chance. Perhaps tonight or tomorrow.
Thanks for running rpmlint on the installed package. I was not aware that it's supported. I have just informed the developer of the package about the problem with shared libraries. ad 2) I have removed glob as suggested I have uploaded new version of SPEC file and source rpm to this location: Spec URL: http://jhladky.fedorapeople.org/dieharder.spec SRPM URL: http://jhladky.fedorapeople.org/dieharder-3.31.1-2.fc16.src.rpm Thanks Jirka
Hi I have informed upstream about the rpmlint warnings dieharder.i686: W: unused-direct-shlib-dependency /usr/lib/libdieharder.so.3.31.1 linux-gate.so.1 dieharder.i686: W: unused-direct-shlib-dependency /usr/lib/libdieharder.so.3.31.1 /usr/lib/libgslcblas.so.0 and I got following reply ====================================================================== linux-gate.so.1 First, read: http://www.trilithium.com/johan/2005/08/linux-gate/ This isn't a real library, and I have no control over it. It is a kernel artifact and completely irrelevant to bloat. IIRC it is also known as vdso and listed (by e.g. ldd) on some systems as linux-vsdo.so instead of linux-gate.so. rpmlint should "probably" just ignore this when it pops up in a binary as I didn't turn it on and don't have any idea how to turn it off, under system/kernel control (artifact to help manage shared objects) not mine. The gslblas library is "required" for full transparent GSL functionality. To quote the GSL manual's recommended build line: $ gcc -L/usr/local/lib example.o -lgsl -lgslcblas -lm I'd argue that this should remain, partly because while I don't use BLAS at this particular instant to do linear algebra associated with some of the tests, at some point I will (I have in some of the unreleased stuff I'm playing with). In any event, since the primary distribution form of the tarball (which is in the src rpm) is designed to encourage tinkering, and I really want the tinkering to be done using GSL functions and linear algebra where possible, I think there is virtue in keeping -lgslcblas as is. That way people (including me!) won't be surprised if they link a gsl function that requires it while playing around and the make breaks....;-) Hope that helps. If I absolutely must, I can try turning BLAS off in the configuration in a way that people can turn back on again easily, but it will probably remain on in my default build. Linear algebra is a big part of statistical analysis and it's just a matter of time before some matrix or another will need to be inverted in dieharder. ====================================================================== I think that both points are valid. rpmlint should not warn about linux-gate.so.1 As for -lgslcblas I agree with the upstream that it should stay as it is. See http://www.gnu.org/s/gsl/manual/html_node/Linking-programs-with-the-library.html Please let me know your opinion on it. Thanks Jirka
Ok! We're almost there, I promise! In doing my formal review (and therefore more in depth) I did notice a couple of things which should all be pretty simple to fix. 1. In your Source0: tag go ahead and replace all instances of "dieharder" with %{name}. (nit pick) 2. The source has a very clear separation of libdieharder from the binary, including separate COPYING, NOTES, and README. I think the *BEST* way of handling this is to go ahead and create a separate subpackage for the library. That way it can have it's own %doc. Just let me know if you need any help. You'll need to add an arch specific "Require:" for the library package in the main package. If you're opposed to doing that, then at a minimum you should rename the doc files in the libdieharder source to append ".lib" to each of them and add them to your existing %doc. 3. There's a manual directory which includes latex based documentation that when built produces a pdf. I would normally describe the process here but I already added it to my copy of your spec file so I'll just attach it here instead for your review. I also ended up creating a separate libs package :) Feel free to use it or not, it just seems like the author designed the library to be both with or separate from the binary. Richard
Created attachment 533852 [details] Updates spec with pdf docs and separate libs package.
(In reply to comment #13) > Please let me know your opinion on it. Good enough. There's no requirement that they do anything about it and in this case it doesn't seem that they should. It's just good to ask and document the response which you've now done. Richard
Hi Richard, thanks a lot for all your effort! I do really appreciate that. Thanks also for uploading updated SPEC file. It feels like I'm now reviewing your changes:-) ad 1) Done. ad 2) I have checked difference between COPYING, NOTES and README files under libdieharder/ directory and the top level directory and it seems to me like that COPYING, NOTES and README files under libdieharder/ directory are just older versions of the files in the top level. I have contacted developer to check what is the current status of these files and what is his opinion on splitting the main package to library and program. I personally think that only COPYING, NOTES and README files are maintained. In such case I would vote to move the files COPYING ChangeLog Copyright NOTES README from the main package to the libs package as the libs package has to be always installed. But let's wait for the developer response first. ad 3) Thanks for that! It was on my TODO list. I have just moved manual/%{name}.pdf from the devel package to the main package. Or perhaps it belongs to the libs package? It's pretty general documentation of random number testing methodology and then it goes to the detailed description of the tests provided by the library. I have uploaded new version of SPEC file and source rpm to this location: Spec URL: http://jhladky.fedorapeople.org/dieharder.spec SRPM URL: http://jhladky.fedorapeople.org/dieharder-3.31.1-3.fc16.src.rpm I will post update here as soon as I get response from the author. Thanks Jirka
(In reply to comment #17) > ad 2) I have checked difference between COPYING, NOTES and README files under > libdieharder/ directory > and the top level directory and it seems to me like that COPYING, NOTES and > README files under libdieharder/ directory are just older versions of the files > in the top level. That may be, but one thing I noticed when doing a diff of the two COPYING files: $ diff -u COPYING libdieharder/COPYING --- COPYING 2011-10-14 08:41:37.000000000 -0500 +++ libdieharder/COPYING 2011-10-14 08:41:37.000000000 -0500 @@ -1,8 +1,8 @@ -$Id: COPYING 215 2006-07-25 18:57:50Z rgb $ +$Id: COPYING 221 2006-08-16 22:43:03Z rgb $ License is granted to build or use the accompanying software: - dieharder + libdieharder according to the following standard Gnu General Public License or any later versions, with the one minor "Beverage" modification listed below. --- The files specifically callout dieharder and libdieharder. I'm not familiar enough with the legalease of software licenses but the safest thing to do is how I set things up in my spec. Hopefully the developer will clarify. As far as the manual goes, since it has instructions for the binary, I would keep it in the main package. I think you're pretty much done, just change the documentation as necessary depending on what the developer says. I already did the review but it's on my work computer so instead of doing it all over again I'll approve your package first thing in the morning! Richard
+: OK -: must be fixed =: should be fixed (at your discretion) ?: Question or clairification needed N: not applicable MUST: [+] rpmlint output: shown in comment: none [+] follows package naming guidelines [+] spec file base name matches package name [+] package meets the packaging guidelines [+] package uses a Fedora approved license: GPLv2+ [+] license field matches the actual license. [+] license file is included in %doc: COPYING [+] spec file is in American English [+] spec file is legible [+] sources match upstream: md5sum matches (b57404dfb812d4548caaf71a05be2d17) [+] package builds on at least one primary arch: Tested F15 x86_64 [N] appropriate use of ExcludeArch [+] all build requirements in BuildRequires [N] spec file handles locales properly [+] ldconfig in %post and %postun [+] no bundled copies of system libraries [N] no relocatable packages [+] package owns all directories that it creates [+] no files listed twice in %files [+] proper permissions on files [+] consistent use of macros [+] code or permissible content [N] large documentation in -doc [+] no runtime dependencies in %doc [+] header files in -devel [N] static libraries in -static [+] .so in -devel [+] -devel requires main package [+] package contains no libtool archives [N] package contains a desktop file, uses desktop-file-install/validate [+] package does not own files/dirs owned by other packages [+] all filenames in UTF-8 SHOULD: [+] query upstream for license text [N] description and summary contains available translations [+] package builds in mock [+] package builds on all supported arches: Tested x86_64 [?] package functions as described: Didn't test [+] sane scriptlets [+] subpackages require the main package [N] placement of pkgconfig files [N] file dependencies versus package dependencies [+] package contains man pages for binaries/scripts *** APPROVED ***
Hi Richard, thanks a lot for the reviewing the package. It's my fourth package for Fedora and I always learn something new. I'm still waiting for the reply from the author. I will let you know as soon as I get feedback from him. Thanks Jiri
Hi Richard, I'm still waiting for the response from the author. It's strange as he usually responds within one day... I will go ahead with packaging process using the approach to split the main program and library with the option to revert it back in case that author will dislike this approach. Thanks Jirka
New Package SCM Request ======================= Package Name: dieharder Short Description: Random number generator tester and timer Owners: jhladky Branches: f15 f16 el5 el6 InitialCC: jhladky
(In reply to comment #21) > Hi Richard, > > I'm still waiting for the response from the author. It's strange as he usually > responds within one day... > > I will go ahead with packaging process using the approach to split the main > program and library with the option to revert it back in case that author will > dislike this approach. That's fine. I'm actually surprised you waited this long :) Not specific to this package but one reason I suggested breaking the package up is this: What if a program had a simple GUI that required QT4 or GTK? When you're packaging it, you think, "Hey, it's only X Kb, why create a separate package?", not realizing that it's not the size of the binary that's the problem, but all the dependencies of QT or GTK that it pulls in. It's entirely plausible that someone might want access to a library without wanting the GUI interface on a system with little disk space, or they may just want a minimal install. Richard
Git done (by process-git-requests).
(In reply to comment #23) > (In reply to comment #21) > > Hi Richard, > > > > That's fine. I'm actually surprised you waited this long :) :-) Well, so far I have very good relationship with the author and I consider asking a question and then just go ahead without waiting for response to be rude. But it's now taking too long... > Not specific to this package but one reason I suggested breaking the package up > is this: > > What if a program had a simple GUI that required QT4 or GTK? When you're > packaging it, you think, "Hey, it's only X Kb, why create a separate package?", > not realizing that it's not the size of the binary that's the problem, but all > the dependencies of QT or GTK that it pulls in. It's entirely plausible that > someone might want access to a library without wanting the GUI interface on a > system with little disk space, or they may just want a minimal install. Yes, good point. I was thinking about it last week and I agree that it's a good approach. Initially I was uncertain why to split such a small package but you hit the nail on the head that it's not size but structure of the package what matters. Thanks again Jirka
(In reply to comment #24) > Git done (by process-git-requests). Thanks a lot! Jirka
dieharder-3.31.1-3.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/dieharder-3.31.1-3.fc16
dieharder-3.31.1-3.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/dieharder-3.31.1-3.fc15
dieharder-3.31.1-3.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/dieharder-3.31.1-3.el6
dieharder-3.31.1-4.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/dieharder-3.31.1-4.el5
dieharder-3.31.1-4.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/dieharder-3.31.1-4.fc15
dieharder-3.31.1-4.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/dieharder-3.31.1-4.fc16
dieharder-3.31.1-4.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/dieharder-3.31.1-4.el6
dieharder-3.31.1-4.el5 has been pushed to the Fedora EPEL 5 testing repository.
dieharder-3.31.1-4.el6 has been pushed to the Fedora EPEL 6 stable repository.
dieharder-3.31.1-4.el5 has been pushed to the Fedora EPEL 5 stable repository.
dieharder-3.31.1-4.fc15 has been pushed to the Fedora 15 stable repository.
dieharder-3.31.1-4.fc16 has been pushed to the Fedora 16 stable repository.