Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226036 --- Comment #4 from Bastien Nocera <bnocera@xxxxxxxxxx> 2009-06-10 10:46:09 EDT --- (In reply to comment #1) > I reviewed this package. There are some issues, questions and suggestions that > I'll bring into your attention. > > * The latest version is not packaged! Please update to the latest version. Latest version is in devel > * The HACKING file should be packaged, possibly in the %doc of devel Done > ? The examples directory can go into the %doc of devel too. The code in examples are too complicated to be used stand-alone. > ? Some files in m4 and the file "missing" indicates GPLv2+ license.But I don't > think these make their way into the package, so I guess BSD is good enough. OK > * I don't think glib2-devel is required to build the package. I built an > identical package in mock without BR'in glib2-devel. Some of the examples > require glib2-devel. So if you are going to package those examples you'll need > to require glib2-devel in the devel subpackage. It's optional, but used. From configure.ac: PKG_CHECK_MODULES(GLIB, glib-2.0, HAVE_GLIB=yes, HAVE_GLIB=no) AC_SUBST(GLIB_LIBS) AC_SUBST(GLIB_CFLAGS) AC_ARG_ENABLE(glib, AC_HELP_STRING([--disable-glib],[disable usage of glib]), [case "${enableval}" in yes) HAVE_GLIB=yes ;; no) HAVE_GLIB=no ;; *) AC_MSG_ERROR(bad value ${enableval} for --disable-glib) ;; esac]) AM_CONDITIONAL(HAVE_GLIB, test "x$HAVE_GLIB" = "xyes") > * A package must own all directories that it creates. If it does not create a > directory that it uses, then it should require a package which does create that > directory. The directory %{_datadir}/gtk-doc/html/ is created but not owned. So > the devel package should require "gtk-doc" which is the rightful owner of that > directory. Done > ! Try to make use of the %{name} macro. Done in the few places where it makes sense. > * From the SPEC file: > # multi-jobbed make makes the build fail: > # ./build_prototypes_doc >liboilfuncs-doc.h > # /bin/sh: ./build_prototypes_doc: No such file or directory > make %{?_smp_mflags} > Isn't there a contradiction here? What is that commented-out section for? I removed the comment. > * From the SPEC file: > # Disable Altivec, so that liboil doesn't SIGILL on non-Altivec PPCs > # See https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=252179#c15 > #sed -i 's/CFLAGS="$CFLAGS "-maltivec""/CFLAGS="$CFLAGS "-fno-tree-vectorize > -Wa,-maltivec""/' configure > #sed -i 's/LIBOIL_CFLAGS -maltivec/LIBOIL_CFLAGS -fno-tree-vectorize > -Wa,-maltivec/' configure > Do we still need these lines in the SPEC file? Nope, removed. Should all be fixed in 0.3.16-2 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review