[Bug 1275888] Review Request: balde - a glib web microframework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1275888



--- Comment #7 from Michael Schwendt <bugs.michael@xxxxxxx> ---
Can't fully understand the excitement about the packaging so far.


* While the pkgconfig BuildRequires are mentioned in the guidelines, they don't
add much value compared with specifying the needed packages in BuildRequires
directly. Afterall, if the configure script really runs pkg-config queries to
look for stuff, the .pc files must be present. If the BuildRequires were
incorrect, the build would fail early which is not a big issue.


* It's surprising that the spec file does not contain any %changelog section
yet: https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs


* Consider running "rpmlint -i" on all (!) built packages, i.e. including the
src.rpm package file.


> balde-0.1.2.spec

You *really* do not want to rename the spec file for each version upgrade.
Certainly not within dist git.

It must be named "balde.spec" only:

  https://fedoraproject.org/wiki/Packaging:Guidelines#Spec_File_Naming

Perhaps you use a single directory for all your package versions of balde. You
may want to adjust your RPM macros to set up a unique source directory per
%version.


> Name: balde
> Group: Development/Libraries

The Group tag for base runtime library packages has been "System
Environment/Libraries" for many years.


> BuildRequires: autoconf
> BuildRequires: automake
> BuildRequires: libtool

None of these are needed.


> BuildRequires: doxygen

No doxygen generated docs are included, though.

$ rpm -qpd balde-devel-0.1.2-1.fc23.x86_64.rpm 
$ rpm -qpd balde-0.1.2-1.fc23.x86_64.rpm 
/usr/share/doc/balde/COPYING
/usr/share/doc/balde/README.md
$


> %package devel

https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package


>   CC     libbalde_la-app.lo
>   CC     libbalde_la-exceptions.lo
>   CC     libbalde_la-cgi.lo
>   CC     libbalde_la-resources.lo
>   CC     libbalde_la-routing.lo

Enable verbose build output with

  V=1 %make_build

so you get to see the used compiler/linker flags actually, which can be a
life-saver in case of problems when wrong flags and/or paths have been used
during building.

https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags


> tests

https://fedoraproject.org/wiki/Packaging:Guidelines#Test_Suites


> $ grep ^Req balde.pc 
> Requires: glib-2.0 >= 2.34, gio-2.0 >= 2.34, shared-mime-info

While the dep on glib and gio is plausible (balde headers include glib/gio
headers), the shared-mine-info dep is superfluous and very likely an artifact
of squeezing it into the @GLIB_DEP@ configure macro.


> %doc COPYING README.md

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text


> %{_bindir}/balde-template-gen

It belongs into the -devel package, doesn't it?


> %exclude %{_libdir}/libbalde.la
> %exclude %{_libdir}/libbalde_template.la

Caution! Prefer "rm -f" in %install for all files you *really* do not want to
include in any (sub-)package. Why? %exclude doesn't remove those files from the
buildroot, but only from the list of files that must be included. It would
still be possible to include them somewhere accidentally. And if "make install"
did not install these libtool archives, %exclude would cause the build to fail,
whereas "rm -f" would not. So, cleaning up the buildroot with "rm" is safer and
hence superior.

-- 
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://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]