[Bug 990272] Review Request: libmbim - library to control MBIM-speaking WWAN modems

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

 



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



--- Comment #5 from Michael Schwendt <bugs.michael@xxxxxxx> ---
"bump the version" likely refers to the "Release" tag:

https://fedoraproject.org/wiki/Packaging:FrequentlyMadeMistakes
| 
| Increase the "Release" tag every time you upload a new package
| to avoid confusion. The reviewer and other interested parties
| probably still have older versions of your SRPM lying around
| to check what has changed between the old and new packages;
| those get confused when the revision didn't change. 

It would be more helpful, if Christopher would give a rationale (or a link to
the Wiki) when asking for spec file changes.

[...]

A close look at the package:


> %global snapshot .git20130730
> Release: 1%{snapshot}%{?dist}

If following the guidelines, the date would come before "git":
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages

That's not really important for post-release snapshots and when strictly
keeping the same package versioning scheme, but it helps avoiding surprises,
such as comparing letters with numbers:

  $ rpmdev-vercmp 1.git20130830.fc19 1.20130730git.fc19
  1.git20130830.fc19 < 1.20130730git.fc19


A test-build:

> configure: WARNING: unrecognized options: --with-tests

?


> checking whether to build gtk-doc documentation... no
>    Documentation:           no

?


>    Maintainer mode:         yes

Indeed, it's one of the configure scripts that still enable AM_MAINTAINER_MODE.
Not an immediate problem, just pointing it out.


> checking for LIBMBIM_GLIB... no
> configure: error: Package requirements (glib-2.0 >= 2.32
>                   gobject-2.0
>                   gio-2.0
>                   gudev-1.0 >= 147) were not met:
> No package 'gudev-1.0' found

It's missing:

  # libgudev1-devel
  BuildRequires: pkgconfig(gudev-1.0) >= 147


> Requires: glib2%{?_isa}

So, if there is _no_ minimum version requirement, the conclusion is that you
should drop this explicit Requires completely in accordance with:
https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

There is an automatic arch-specific dependency on the libglib-2.0.so.0 SONAME.


> make %{?_smp_mflags}

Two options for making the build output more verbose (so one could actually see
what compiler flags are used or examine build logs with scripts). Either:

  V=1 make %{?_smp_mflags}
or
  %configure --disable-silent-rules ...


Relevant rpmlint output for the binary rpms:

> libmbim.x86_64: E: incorrect-fsf-address /usr/share/doc/libmbim-1.5.0/COPYING

https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address


> libmbim.x86_64: E: zero-length /usr/share/doc/libmbim-1.5.0/README

Not a big issue. You could drop it, but decide yourself whether you would
notice that a future upgrade would fill it with contents.


> libmbim-utils.x86_64: W: no-documentation
> libmbim-utils.x86_64: W: no-manual-page-for-binary mbimcli
> libmbim-utils.x86_64: W: no-manual-page-for-binary mbim-network

Just including these warnings to meet the review guidelines. ;)


> %package utils
> License: GPLv2+

GPLv2 and GPLv2+, since the mbim-network script contains a GPLv2 preamble with
no "or later" clause.

Perhaps that's intentional, perhaps not:
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Clarification


> glib2 also provides some gtk-doc, and while that's currently stored
> in glib2-doc it has switched around in the past between -devel (which
> is a BuildRequire) and -docs so I'm more comfortable explicitly
> owning the relevant dirs than all of them.

There is no reason to be concerned, because we install into an empty
%{buildroot}, and that one won't include any docs from glib2 build
requirements.

> %dir %{_datadir}/gtk-doc
> %dir %{_datadir}/gtk-doc/html
> %dir %{_datadir}/gtk-doc/html/libmbim-glib
> %{_datadir}/gtk-doc/html/libmbim-glib/*

  %{_datadir}/gtk-doc/

achieves exactly the same thing.

  $ rpmls -p libmbim-devel-1.5.0-1.git20130730.fc19.x86_64.rpm|grep ^d
  drwxr-xr-x  /usr/include/libmbim-glib
  drwxr-xr-x  /usr/share/gtk-doc
  drwxr-xr-x  /usr/share/gtk-doc/html
  drwxr-xr-x  /usr/share/gtk-doc/html/libmbim-glib

-- 
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=jvdixuILhd&a=cc_unsubscribe
_______________________________________________
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]