https://bugzilla.redhat.com/show_bug.cgi?id=1367526 --- Comment #3 from Paulo Andrade <paulo.cesar.pereira.de.andrade@xxxxxxxxx> --- (In reply to Jerry James from comment #2) > Issues > ====== > 1. This is causing some problems: > > checking size of void *... 0 > checking size of int... 0 > checking size of long... 0 > > If you look through the build log, you'll see warnings about casting > between > pointers and integers of a different size. This is due to the configure > failure above. The problem seems to be that the configure checks are > linked > with $RPM_LD_FLAGS, but compiled without %{optflags}, so the link step > fails > due to bad relocations. The problem is this line in the spec file: > > export CXXFLAGS="$CXXFLAGS -std=c++98" > > At that point, CXXFLAGS has not been set, so the $CXXFLAGS inside quotes > is > the empty string, resulting in CXXFLAGS set to " -std=c++98". Do this > instead: > > export CXXFLAGS="%{optflags} -std=c++98" Fixed, thanks! > 2. The license field reflects the final binary contents. Since the GPL, both > versions 2 and 3, subsumes BSD, it is permissible make the license field > read simply "GPLv2+" if you wish. Otherwise, you need to document the > multiple license scenario in the spec file. See > > https://fedoraproject.org/wiki/Packaging: > LicensingGuidelines#Multiple_Licensing_Scenarios I added a comment about what code is under a BSD like license (Cudd) > 3. In the review below, I presume that polybori will be retired. For > example, > I don't mark that this package causes conflicts. It does, but that will > be > handled by removing polybori. However, that means that the "Conflicts" > tag > in the -devel package should be removed. Instead, add "Obsoletes" and > "Provides" tags to each package as described here: > > https://fedoraproject.org/wiki/Packaging:Guidelines#Renaming. > 2FReplacing_Existing_Packages Ok. Added the proper Obsoletes/Provides. Only sagemath uses it, so, once a sagemath 7.3 package is available, polybori can be retired. > 4. The rpmlint output shows some unused direct library dependencies. I like > to > do this after the %configure step to remove both rpaths and such unused > dependencies: > > # Get rid of undesirable hardcoded rpaths, and workaround libtool reordering > # -Wl,--as-needed after all the libraries. > sed -e 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' \ > -e 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' \ > -e 's|CC="\(g..\)"|CC="\1 -Wl,--as-needed"|' \ > -i libtool > > It is not enough to add -Wl,--as-needed to LDFLAGS, unfortunately, since > libtool has a long-standing bug where it reorders -Wl,--as-needed AFTER > all > of the libraries, where it does no good at all. Thanks. Changed to use the suggested pattern, that indeed corrects the problem. On not yet released versions it should actually make use of gd and png. > 5. The changelog remarks that the python sources have been prepared for > python > 3. Is it possible to make both python 2 and python 3 subpackages? Also, > see https://fedoraproject.org/wiki/Packaging:Python for information on the > %python_provide macro. I believe this is more of something to ask upstream, it explicitly requires python 2.7: AM_PATH_PYTHON([2.7]) so, I believe this is a work in progress by upstream. > 6. Note the rpmlint complaint about mixed use of space and tabs. Choose one. Corrected. Was a bad cut&paste. > 7. I don't like the summary line. Yes, this is a replacement for polybori, > but what does it DO? Maybe reuse the polybori summary line: "Framework > for > Boolean Rings". Along those lines, I think the description is confusing, > since it mentions polybori, but not brial. I used the sagemath SPKG.txt text, but indeed it was not looking right. Now it was changed the Summary to the same as polybori, and description basically s/PolyBori/BRiAl/. > 8. What version of cudd is bundled? The Provides line should indicate that; > e.g., "Provides: bundled(cudd) = 2.5.1". The code is significantly changed, but appears to be based/forked from 2.5.0, I changed to: # brial-0.8.5/Cudd/cudd/cudd.h:#define CUDD_VERSION "2.5.0" Provides: bundled(cudd) = 2.5.0 > [!]: If the package is under multiple licenses, the licensing breakdown > must be documented in the spec. should be fixed. > [!]: %build honors applicable compiler flags or justifies otherwise. should be fixed. > [!]: If the package is a rename of another package, proper Obsoletes and > Provides are present. should be fixed. > [!]: Useful -debuginfo package or justification otherwise. > This is due to the CXXFLAGS snafu described above. should be fixed. Spec URL: https://pcpa.fedorapeople.org/brial.spec SRPM URL: https://pcpa.fedorapeople.org/brial-0.8.5-2.fc26.src.rpm -- 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://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx