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.

* The HACKING file should be packaged, possibly in the %doc of devel

? The examples directory can go into the %doc of devel too.

? 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.

* 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.

* 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.

! Try to make use of the %{name} macro.

* 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?

* 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?



Adding Colin and Caolan to CC since they were the last two known maintainers. Sorry if this was not desired

ping?

re-ping? I added more previous maintainers to the CC. Please DO NOT IGNORE!

(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

Thank you Bastien,
There is this minor issue that is unfortunately a blocker:

The "%doc HACKING" line must come after "defattr(-,root,root,-)"

Please fix this so we can close the bug.

Thanks again!

Done in -3

All issues have been addressed. Thanks for the updates.

----------------------------------------------
This merge review (liboil) is APPROVED by oget
----------------------------------------------

Closing.