[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 #6 from Dan Williams <dcbw@xxxxxxxxxx> ---
(In reply to Michael Schwendt from comment #5)
> "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

Changed.

> A test-build:
> 
> > configure: WARNING: unrecognized options: --with-tests

Fixed.

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

The documentation is pre-generated to ensure that multilib builds don't
conflict.  So the documentation is pre-built, but won't be built during the
package build.

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

I'll poke upstream on it.

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

Fixed.

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

Requires removed; relying on auto deps.

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

Fixed to "V=1 make"

> 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

Poked upstream, they fixed it, new tarball.

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

Poked upstream, they fixed it, new tarball.

> > 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. ;)

Upstream is discussing how to add the manpages.

> > %package utils
> > License: GPLv2+
> 
> GPLv2 and GPLv2+, since the mbim-network script contains a GPLv2 preamble
> with no "or later" clause.

It was unintentional.  Poked upstream, they fixed it, new tarball.

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

Fixed.

spec and RPM in same location again.  Thanks!

-- 
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=08GaUbaPMm&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]