[Bug 226036] Merge Review: liboil

[bugzilla at redhat.com]

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 at redhat.com>  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.