Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=877651
--- Comment #46 from Paulo Andrade <paulo.cesar.pereira.de.andr...@gmail.com>
---
(In reply to comment #45)
> Here's a run of fedora-review, noting a few problems that need to be fixed.
> Issues, in no particular order:
> 1. The jpackage-utils and javadoc warnings are due to jmol and sage3d; the
> Java packaging guidelines need to be followed for those portions.
I believe it is being caused because the package has symlinks to .jar
files from jmol and vecmath, but fedora-review does not consider this
condition.
NOTE: not addressed yet.
> 2. Should the -doc subpackages be noarch? How about the -data subpackages?
I changed both to noarch. I wish rpm would be smarter about changing
a directory to a symlink, since to be noarch it needs to be
moved to /usr/share, the previous directories under %_libdir
need to be changed to symlinks, and that makes updates of the
current package not easy. In the current state should be ok, but
changing later is really not desirable because it would create
too many upgrade failure conditions.
> 3. The build isn't using the right build flags. The build log shows:
> + pushd spkg/build/sage-5.6
> + pushd c_lib
> + CXX=g++
> + UNAME=Linux
> + SAGE64=auto
> + scons
> scons: Reading SConscript files ...
> scons: done reading SConscript files.
> scons: Building targets ...
> gcc -o src/convert.os -c -fPIC
> -I/builddir/build/BUILDROOT/sagemath-5.6-4.fc19.x86_64/usr/lib64/sagemath/
> local/include
> -I/builddir/build/BUILDROOT/sagemath-5.6-4.fc19.x86_64/usr/lib64/sagemath/
> local/include/python2.7
> -I/builddir/build/BUILDROOT/sagemath-5.6-4.fc19.x86_64/usr/lib64/sagemath/
> local/include/NTL -Iinclude src/convert.c
>
> ... followed by more files compiled without the correct flags. I see more
> compilations in other directories later in the build log with the same
> problem.
scons is weird, it ignores most environment variables. I did add
hardcoded CFLAGS and CXXFLAGS (and LINKFLAGS). I do not have any
saved logs for the cython build flags, but it is passing the
proper flags (not sure if side effect of exporting CFLAGS and
CXXFLAGS).
Just in case, the SAGE64 is just to fool this:
if os.environ['SAGE64']=="yes":
env.Append( CFLAGS="-O2 -g -m64" )
env.Append( CXXFLAGS="-O2 -g -m64" )
env.Append( LINKFLAGS="-m64" )
so that os.environ['SAGE64'] is defined and does not cause
and exception due to being undefined.
> 4. The bundling issue still needs to be resolved.
I was left alone with the pexpect issue :-( Pexpect upstream did not
respond my query, and sagemath upstream did comment a lot about it,
and that at some point the new pexpect, basically completely rewritten
was significantly slower with sagemath.
The remaining issue is ipython. Hopefully by sagemath-5.7 it will be able
to use latest upstream ipython, otherwise may need to ping back about it.
> 5. The license check looks okay, except it flags one file with an Apache
> license. I think this is okay, because that file is only used for testing
> and does not contribute to the binary RPMs, right?
Need to verify when the selenium subdir was added (well, it appears
licensecheck only tells about it, but there are others), but it was
not when I did run licensecheck to generate the License tag. But the
notebook was significantly changed since then, and added several ASL
files indeed.
Updated now.
> 6. The license breakdown needs to be explained in a comment;
> See
> https://fedoraproject.org/wiki/Packaging:
> LicensingGuidelines#Multiple_Licensing_Scenarios
I did split/review as much as I could, and the directories in sagenb
where there was only one file with a license I assumed that license.
There is a significant breakdown already done in $SAGE_ROOT/COPYING.txt
but that includes what upstream sagemath bundles, so, only a small
portion actually applies to sagemath, that is release under GPLv2+
> 7. Since the main package installs an icon, it needs this:
> https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
Fixed.
> 8. %{_libdir}/sagemath/share is owned by the main package, but the main
> package puts no contents into it. It appears to be used only by the
> sagemath-data-foo packages, so shouldn't sagemath-data own it?
Corrected.
> 9. Empty directories in the sagemath-data-extcode package, some of which are
> the only items in their parent directories:
> - %{_libdir}/sagemath/share/extcode/MuPAD/user
> - %{_libdir}/sagemath/share/extcode/QEPCAD
> - %{_libdir}/sagemath/share/extcode/gap/user
> - %{_libdir}/sagemath/share/extcode/genus2reduction
> - %{_libdir}/sagemath/share/extcode/gnuplot
> - %{_libdir}/sagemath/share/extcode/kash/user
> - %{_libdir}/sagemath/share/extcode/macaulay2/user
> - %{_libdir}/sagemath/share/extcode/magma/ell_padic
> - %{_libdir}/sagemath/share/extcode/magma/padic_height
> - %{_libdir}/sagemath/share/extcode/maple/user
> - %{_libdir}/sagemath/share/extcode/mathematica/user
> - %{_libdir}/sagemath/share/extcode/matlab/user
> - %{_libdir}/sagemath/share/extcode/octave/user
> - %{_libdir}/sagemath/share/extcode/pari/user
> - %{_libdir}/sagemath/share/extcode/scilab/user
> - %{_libdir}/sagemath/share/extcode/sobj
These are supposed to be placehoders for customization. It is most
likely wrong because it is not per user. I left it there because it
matches upstream release.
I updated it to not create the empty directory trees.
> 10. The main package Requires: jmol, but then sagemath-notebook includes both
> JmolApplet.jar, from the jmol package, and vecmath.jar, from the vecmath
> package. What is going on here?
It could be changed to make sagemath-core (not the virtual sagemath
package) require jmol, but jmol should be required by the main/core package
as it can use the standard jmol in the command line interface.
The jars are actually symbolic links, to have them in the expected place
by the notebook:
$ ls -l /usr/lib64/python2.7/site-packages/sagenb/data/jmol/*.jar
lrwxrwxrwx 1 root root 30 Feb 9 15:17
/usr/lib64/python2.7/site-packages/sagenb/data/jmol/JmolApplet.jar ->
/usr/share/java/JmolApplet.jar
lrwxrwxrwx 1 root root 27 Feb 9 15:17
/usr/lib64/python2.7/site-packages/sagenb/data/jmol/vecmath.jar ->
/usr/share/java/vecmath.jar
> 11. Binary eggs need to be removed in %prep:
> - spkg/build/sagenb-0.10.2/src/sagenb/sagenb.egg-info
Fixed.
> 12. The complaint below about fonts is due to some TeX fonts in
> sagemath-notebook, which need to be handled according to the Fonts
> Policy:
> https://fedoraproject.org/wiki/Packaging:FontsPolicy
I changed the spec file to explicitly remove the fonts subdir.
But I am going to debug it to see if it works without those, of if needs
to package them separately. I believe the warning is about the .otf
files, and not the .eot and .svg ones (yes, it is:
// /usr/share/fedora-review/plugins/generic_should.py +637
for p in ['*.pfb', '*.pfa', '*.afm', '*.ttf', '*.otf']:
).
I did not test everything, so, but it most likely should be
working, or have some bad font rendering in some random place
(or just be using a system font).
> 13. Patches should have an explanatory comment:
>
> https://fedoraproject.org/wiki/Packaging:
> Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
Most were already commented, but added a description, and an url
link if applicable, to the few ones without a proper small description
in a comment.
> 14. Add -p to the cp command on line 827 of the spec file (for the "preserves
> timestamps" issue)
Fixed (actually used -a to match pattern in other cp calls)
> 15. I see some locale-specific files, but don't see this in use:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files
Initially I did remove those, but that ended up with a non working package.
Then I switched to use find_lang but it does not find the .mo files because
they are under a translations, not "locale" directory. After creating a
symbolic link named locale it still fails, because find_lang wants
locale/xx_YY/*.mo, and the .mo files are in
[translations|locale]/xx_YY/LC_MESSAGES/*.mo
Note sure if should add all the symlinks required to make find_lang work.
NOTE: not addressed yet.
> 16. Spelling: donwload_tarball -> download_tarball
Corrected. At least it was consistent, and was mispelled in all
usages of the macro :-)
> 17. I know that many of the rpmlint complaints are innocuous, but a few
> should
> be handled, including these:
> - sagemath-sagetex.x86_64: W: one-line-command-in-%post /usr/bin/mktexlsr
The sagemath-sagetex it tricky, because sagetex itself should be very
rarely updated, so, a simple sagemath-sagetex upgrade with sagemath-sagetex
in the same version would not need to rerun mktexlsr.
> - sagemath.src: W: strange-permission testjava.sh 0755L
Not sure about the .sh permission. I hope the change from -m755 to
-m0755 as argument to install should correct it.
> - sagemath.src:648: W: rpm-buildroot-usage %prep rm -rf %{buildroot}
I had that for debugging purposes and run a build --without[-=]prep,
but this is not be much useful nowadays, I removed it. Actually, if
doing a build using rpmbuild now it is required to remove manually
the buildroot directory, but it is a minor issue, and side effect
of all the magics done to make the build work in a buildroot environment.
> - sagemath.src:1305: W: macro-in-%changelog %{name}
Corrected.
> - sagemath.src:506: W: mixed-use-of-spaces-and-tabs (spaces: line 506,
> tab: line 3)
The mixed spaces and tabs is tricky, "correcting" it means modifying
the %description that uses spaces simulating a 4 spaces tabulation.
NOTE: not addressed yet.
> - sagemath-core.x86_64: W: unused-direct-shlib-dependency
> /usr/lib64/libcsage.so /lib64/libm.so.6
The unused direct shlib dependency is not easy, need to check all
.so python modules to see which one is problematic and add special
rules for it, anyway, should not be too hard to do to add an extra
patch to module_list.py to handle it.
NOTE: not addressed yet.
> I *think* that the other issues flagged by fedora-review aren't really
> issues,
> but check them anyway and let me know what you think.
The non working url is on purpose because downloading 300Mb+ for every
single test run of fedora-review is not cheap, but later it can be
made so, and it is easy to regenerate the srpm to toggle the:
# for quicker fedora-review tests without downloading the main tarball
%global download_tarball 0
variable.
Unless I am completely mistaken, ther is nothing that qualifies as
javadoc in the package, so the warning should be bogus, and a
side effect of the symlinks to jar files.
The dangling symlinks are for the .jar files from the "requires"
jmol and vecmath, so that it does not bundle those, and still
have the symlinks to the place the notebook wants them.
I will make extra checks to see if adding an explicit %lang
cures the warning about .mo files not in %lang, and this should
be acceptable to not use %find_lang as explained above (?!)
The unversioned .so files are masked out by __provides_exclude_from
and are private python modules, so the warning should be bogus.
The sage3d dangling symlink is indeed dangling. Need java3d
(non-free) and regenerate the jars (TODO), I will remove it from
any final release, but it is not by any interface, well, one
can try to make 3d plots using it, what will fail.
The other %{buildroot} usages can be patched to silence the
warning, that is, to use some other "temporary" directory
during build, because it needs to "install" some components
to rebuild everything in the buildroot environment, but
the most complex rebuild (the documentation) is in
%install anyway.
I will work a bit more, but already spent quite some time
to address most of the issues, so, if you want to rerun
it, it should be 90%+ of the issues addressed now :-)
--
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug
https://bugzilla.redhat.com/token.cgi?t=cYDFyvdDon&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
