Spec URL: https://jjames.fedorapeople.org/gap-pkg-guava/gap-pkg-guava.spec SRPM URL: https://jjames.fedorapeople.org/gap-pkg-guava/gap-pkg-guava-3.13.1-1.fc26.src.rpm Fedora Account System Username: jjames Description: GUAVA is a package that implements coding theory algorithms in GAP. Codes can be created and manipulated and information about codes can be calculated. GUAVA consists of various files written in the GAP language, and an external program from J. S. Leon for dealing with automorphism groups of codes and isomorphism testing functions. Several algorithms that need the speed are integrated in the GAP kernel. The functions within GUAVA can be divided into four categories: - Construction of codes. GUAVA can construct non-linear, linear and cyclic codes over an arbitrary finite field. Examples are HadamardCode, ReedMullerCode, BestKnownLinearCode, QRCode and GoppaCode. - Manipulation of codes. These functions allow the user to transform one code into another or to construct a new code from two codes. Examples are PuncturedCode, DualCode, DirectProductCode and UUVCode. - Computation of information about codes. This information is stored in the code record. Examples are MinimumDistance, OuterDistribution, IsSelfDualCode and AutomorphismGroup. - Generation of bounds on linear codes. The table by Brouwer and Verhoeff (as it existed in the mid-1990s) is incorporated into GUAVA. For example, BoundsMinimumDistance.
It is not required to have: BuildRequires: gcc --- This looks suspecting, as it means it will loop only once: ./src/randschr.c:238:10: warning: this 'if' clause does not guard... [-Wmisleading-indentation] if ( hOrder % primeList[i] == 0 ) ^~ ./src/randschr.c:240:13: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if' break; ^~~~~ code is: for ( i = 0 ; primeList[i] != 0 ; ++i ) { if ( hOrder % primeList[i] == 0 ) raisePermToPower( h, hOrder / primeList[i]); break; } and the data is: guava-3.13.1/src/leon/src/primes.c:Unsigned primeList[] = {2,3,5,7,9,11,13,17,19,23,29,31,37,41,43,47,53,59, ... 0}; --- || and && have different precedence, so this code is also suspecting: ./src/randschr.c:391:43: warning: suggest parentheses around '&&' within '||' [-Wparentheses] lowestOrder == curOrder && levelLowestOrder > finalLevel) ) { ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ code is: if ( nonInvolRejectCount > 0 && (...|| (lowestOrder < (curOrder = permOrder(h)) || ...&&...) lowestOrder == curOrder && levelLowestOrder > finalLevel) ) { --- Another case of suspicious code: ./src/ccent.c: In function 'nextBasePointEltCent': ./src/ccent.c:90:24: warning: suggest parentheses around '+' inside '<<' [-Wparentheses] priority = 2000000000ul - (unsigned long) MIN(cycleLen[pt],1000) << 20 ~~ + cSize; ^~~~~~~ --- The package appears in good shape, but while I checked all other compiler warnings, and they appear to just warn about improper indentation or somewhat uncommon coding practive, the above warnings may be valid, so, I would like some comment about it.
Thanks for the review, Paulo. (In reply to Paulo Andrade from comment #1) > It is not required to have: > BuildRequires: gcc Actually, it is now. See: https://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B#BuildRequires_and_Requires https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/XGXCCAJNEOIEN3KK6TN2657LSIGZGB3N/ > This looks suspecting, as it means it will loop only once: > ./src/randschr.c:238:10: warning: this 'if' clause does not guard... > [-Wmisleading-indentation] Good catch! I will add braces to the warning patch to fix that. > || and && have different precedence, so this code is also suspecting: > ./src/randschr.c:391:43: warning: suggest parentheses around '&&' within > '||' [-Wparentheses] > lowestOrder == curOrder && levelLowestOrder > finalLevel) > ) { > ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ That one I think is okay. It appears to me that the && really should be within ||, so it is parsed in the correct order. But ... it's easy to patch, so I will add parentheses there. > Another case of suspicious code: > ./src/ccent.c: In function 'nextBasePointEltCent': > ./src/ccent.c:90:24: warning: suggest parentheses around '+' inside '<<' > [-Wparentheses] > priority = 2000000000ul - (unsigned long) > MIN(cycleLen[pt],1000) << 20 > > ~~ > + cSize; > ^~~~~~~ This one worries me. I'm not sure if the + is supposed to be inside or outside of the <<, so I am reluctant to touch it. I think I'd better check with upstream before I do anything here, unless you think you can see where the parentheses should go.
I think the last one is likely a bug, because the "+ cSize" is added in the next line, but (adding parenthesis,) what is happening is: priority = (2000000000ul - (unsigned long)MIN(cycleLen[pt],1000)) << (20 + cSize); So, it might have two bugs. The smaller the value of cycleLen[pt] and cSize, the more likely the code does what is intended, as it appears to be some kind of heuristic priority choosing.
I added those parentheses. I also noticed that guava has a software popcount implementation, so I added another patch to use a CPU popcount instruction instead if one is available. New URLs: Spec URL: https://jjames.fedorapeople.org/gap-pkg-guava/gap-pkg-guava.spec SRPM URL: https://jjames.fedorapeople.org/gap-pkg-guava/gap-pkg-guava-3.13.1-2.fc26.src.rpm
Looks like the parallel command is broken in rawhide: $ head -1 /usr/bin/parallel ##!/usr/bin/perl note the two '#'. Fixing it manually and building the package I do not see any other issue. You might want to either check what is broken with the parallel package, or not use it... See the sed command in http://pkgs.fedoraproject.org/cgit/rpms/parallel.git/commit/?id=b941a86552fa42ca018a58bffc3ba35c428b0d07 (I just fedpkg co'd parallel and it is indeed broken). Otherwise, the package is now approved. All the issues have been fixed!
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/gap-pkg-guava
gap-pkg-guava-3.13.1-2.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-bcd5fe40a8
gap-pkg-guava-3.13.1-2.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-2547786f05
gap-pkg-guava-3.13.1-2.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-bcd5fe40a8
gap-pkg-guava-3.13.1-2.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-2547786f05
gap-pkg-guava-3.13.1-2.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.
gap-pkg-guava-3.13.1-2.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.