https://bugzilla.redhat.com/show_bug.cgi?id=830869 --- Comment #29 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> --- (In reply to Jaroslav Škarvada from comment #28) > (In reply to Zbigniew Jędrzejewski-Szmek from comment #27) > > Thanks for the review. > > > Please add %global _docdir_fmt %{name}. Currently hpl-common has files in > > /usr/share/doc/hpl-common which is confusing and unnecessary. > > > Fixed > > > Documentation should be in an unversioned directory. > > [https://fedoraproject.org/wiki/Changes/UnversionedDocdirs] > > > Fixed, it was because this review request is very very old, and in that good > old times, documentation was versioned :) > > > Use %license for COPYRIGHT. > > [https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text] > > > Fixed, the same as previous, there was no %license when this review request > was submitted. > > > The binary currently tries to open /etc/hpl/HPL.dat and segfaults. I think > > it should be patched to use the new location. > > > Please see previous discussion on HPL.dat location. I finally moved HPL.dat > to /usr/share/hpl, but I am not sure whether it is good thing to add a > fallback for HPL to read the file from there, probably it isn't. > Nevertheless it shouldn't coredump. I patched it to cleanly exit. I don't see why it shouldn't. What about the following simpler patch: --- a/testing/ptest/HPL_pdinfo.c +++ b/testing/ptest/HPL_pdinfo.c @@ -294,11 +294,12 @@ /* * Open file and skip data file header */ - if( ( infp = fopen( "HPL.dat", "r" ) ) == NULL ) + if( ( infp = fopen( "HPL.dat", "r" ) ) == NULL && + ( infp = fopen( "/usr/share/hpl/HPL.dat", "r" ) ) == NULL) { HPL_pwarn( stderr, __LINE__, "HPL_pdinfo", "cannot open file HPL.dat" ); - error = 1; goto label_error; + exit( 1 ); } (void) fgets( line, HPL_LINE_MAX - 2, infp ); This way the package does something useful out-of-the-box, and gives a rough test/benchmark of the mpi installation. If the user wants to customize, they can provide their own HPL.dat file in $CWD. > > README.Fedora still refers to mpich2. It's mpich now. > > > Fixed and also updated regarding HPL.dat location. hpl-README.Fedora still has old contents ("mpich2"). > > Shouldn't this be run in %check? It would test both mpi implementations on > > all archs. > > > I am currently not sure. Did you mean running the HPL benchmark by hand over > all available implementations? And with which configuration? With the > supplied example HPL.dat? What number of processes to use? One? Is it good > thing to do? Yes, that's what I'd do. Something like this: %check %{_openmpi_load} mpirun -n 4 xhpl_openmpi %{_openmpi_unload} %{_mpich_load} mpirun -n 4 xhpl_mpich %{_mpich_unload} This takes about two seconds on my machine. I guess it'd vary a lot between architectures, but should not be more then maybe ten seconds anywhere. The point is less to benchmark anything, but rather to test that it still runs at all. I think 4 is a good number, even if compiling on one cpu: high enough to test that things work, but does not consume undue resources. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review