[Bug 1229903] Review Request: NetworkManager-sstp - NetworkManager VPN plugin for SSTP

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

 



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



--- Comment #4 from Christopher Meng <i@xxxxxxxx> ---
(In reply to Marcin Zajaczkowski from comment #3)
> You can always become a co-maintainer of NetworkManager-sstp and have the
> another package to (co-)maintain :).

I will give up ;)

-----------------------------------------------------------------

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated



===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[-]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "GPL (v2 or later)". Detailed output of licensecheck:

GPL (v2 or later)
-----------------
NetworkManager-sstp-0.9.10/auth-dialog/main.c
NetworkManager-sstp-0.9.10/ltmain.sh
NetworkManager-sstp-0.9.10/properties/advanced-dialog.c
NetworkManager-sstp-0.9.10/properties/advanced-dialog.h
NetworkManager-sstp-0.9.10/properties/import-export.c
NetworkManager-sstp-0.9.10/properties/import-export.h
NetworkManager-sstp-0.9.10/properties/nm-sstp.c
NetworkManager-sstp-0.9.10/properties/nm-sstp.h
NetworkManager-sstp-0.9.10/src/nm-ppp-status.h
NetworkManager-sstp-0.9.10/src/nm-sstp-pppd-plugin.c
NetworkManager-sstp-0.9.10/src/nm-sstp-service-defines.h
NetworkManager-sstp-0.9.10/src/nm-sstp-service.c
NetworkManager-sstp-0.9.10/src/nm-sstp-service.h

[x]: License file installed when any subpackage combination is installed.
[x]: Package requires other packages for directories it uses.
     Note: No known owner of /etc/dbus-1/system.d
[x]: Package must own all directories that it creates.
     Note: Directories without known owners: /etc/dbus-1/system.d,
     /usr/lib/NetworkManager
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: The spec file handles locales properly.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 61440 bytes in 8 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Rpmlint is run on all rpms the build produces.
     Note: No rpmlint messages.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: %config files are marked noreplace or the reason is justified.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[!]: Buildroot is not present
     Note: Invalid buildroot found: %{_tmppath}/%{name}-%{version}-root
     See: http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
[x]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     NetworkManager-sstp-gnome , NetworkManager-sstp-debuginfo
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[!]: Rpmlint is run on all installed packages.
     Note: Mock build failed
     See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint
[!]: Package should not use obsolete m4 macros
     Note: Some obsoleted macros found, see the attachment.
     See: https://fedorahosted.org/FedoraReview/wiki/AutoTools
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.


Installation errors
-------------------
INFO: mock.py version 1.2.12 starting (python version = 3.4.3)...
Start: init plugins
INFO: selinux disabled
Finish: init plugins
Start: run
Start: chroot init
INFO: calling preinit hooks
INFO: enabled root cache
INFO: enabled dnf cache
Start: cleaning dnf metadata
Finish: cleaning dnf metadata
INFO: enabled ccache
Mock Version: 1.2.12
INFO: Mock Version: 1.2.12
Finish: chroot init
INFO: installing package(s):
/home/rpmaker/Desktop/NetworkManager-sstp/results/NetworkManager-sstp-0.9.10-3.fc24.i686.rpm
/home/rpmaker/Desktop/NetworkManager-sstp/results/NetworkManager-sstp-gnome-0.9.10-3.fc24.i686.rpm
/home/rpmaker/Desktop/NetworkManager-sstp/results/NetworkManager-sstp-debuginfo-0.9.10-3.fc24.i686.rpm
/home/rpmaker/Desktop/NetworkManager-sstp/results/NetworkManager-sstp-debuginfo-0.9.10-3.fc24.i686.rpm
ERROR: Command failed. See logs for output.
 # /usr/bin/dnf --installroot /var/lib/mock/fedora-rawhide-i386/root/
--releasever 24 install
/home/rpmaker/Desktop/NetworkManager-sstp/results/NetworkManager-sstp-0.9.10-3.fc24.i686.rpm
/home/rpmaker/Desktop/NetworkManager-sstp/results/NetworkManager-sstp-gnome-0.9.10-3.fc24.i686.rpm
/home/rpmaker/Desktop/NetworkManager-sstp/results/NetworkManager-sstp-debuginfo-0.9.10-3.fc24.i686.rpm
/home/rpmaker/Desktop/NetworkManager-sstp/results/NetworkManager-sstp-debuginfo-0.9.10-3.fc24.i686.rpm


Rpmlint
-------
Checking: NetworkManager-sstp-0.9.10-3.fc24.i686.rpm
          NetworkManager-sstp-gnome-0.9.10-3.fc24.i686.rpm
          NetworkManager-sstp-debuginfo-0.9.10-3.fc24.i686.rpm
          NetworkManager-sstp-0.9.10-3.fc24.src.rpm
4 packages and 0 specfiles checked; 0 errors, 0 warnings.




Requires
--------
NetworkManager-sstp-debuginfo (rpmlib, GLIBC filtered):

NetworkManager-sstp-gnome (rpmlib, GLIBC filtered):
    NetworkManager-sstp
    libatk-1.0.so.0
    libc.so.6
    libcairo-gobject.so.2
    libcairo.so.2
    libdbus-1.so.3
    libdbus-glib-1.so.2
    libgdk-3.so.0
    libgdk_pixbuf-2.0.so.0
    libgio-2.0.so.0
    libglib-2.0.so.0
    libgobject-2.0.so.0
    libgtk-3.so.0
    libnm-glib-vpn.so.1
    libnm-glib.so.4
    libnm-util.so.2
    libpango-1.0.so.0
    libpangocairo-1.0.so.0
    libpthread.so.0
    nm-connection-editor
    rtld(GNU_HASH)

NetworkManager-sstp (rpmlib, GLIBC filtered):
    NetworkManager
    config(NetworkManager-sstp)
    dbus
    gnome-keyring
    gtk3
    libatk-1.0.so.0
    libc.so.6
    libcairo-gobject.so.2
    libcairo.so.2
    libdbus-1.so.3
    libdbus-1.so.3(LIBDBUS_1_3)
    libdbus-glib-1.so.2
    libgdk-3.so.0
    libgdk_pixbuf-2.0.so.0
    libgio-2.0.so.0
    libglib-2.0.so.0
    libgobject-2.0.so.0
    libgtk-3.so.0
    libnm-glib-vpn.so.1
    libnm-glib.so.4
    libnm-gtk.so.0
    libnm-util.so.2
    libpango-1.0.so.0
    libpangocairo-1.0.so.0
    libsecret-1.so.0
    libsstp_api-0.so
    ppp
    rtld(GNU_HASH)
    shared-mime-info
    sstp-client



Provides
--------
NetworkManager-sstp-debuginfo:
    NetworkManager-sstp-debuginfo
    NetworkManager-sstp-debuginfo(x86-32)

NetworkManager-sstp-gnome:
    NetworkManager-sstp-gnome
    NetworkManager-sstp-gnome(x86-32)

NetworkManager-sstp:
    NetworkManager-sstp
    NetworkManager-sstp(x86-32)
    config(NetworkManager-sstp)



Unversioned so-files
--------------------
NetworkManager-sstp: /usr/lib/pppd/2.4.7/nm-sstp-pppd-plugin.so
NetworkManager-sstp-gnome: /usr/lib/NetworkManager/libnm-sstp-properties.so

Source checksums
----------------
https://downloads.sourceforge.net/sstp-client/NetworkManager-sstp-0.9.10.tar.bz2
:
  CHECKSUM(SHA256) this package     :
10949bb97d0930df7b46de931fd302f607e4ca4b5137e2e046ec358188faee52
  CHECKSUM(SHA256) upstream package :
10949bb97d0930df7b46de931fd302f607e4ca4b5137e2e046ec358188faee52


AutoTools: Obsoleted m4s found
------------------------------
  AC_PROG_LIBTOOL found in: NetworkManager-sstp-0.9.10/configure.ac:17


Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20
Command line :/usr/bin/fedora-review -rvn
NetworkManager-sstp-0.9.10-3.fc21.src.rpm
Buildroot used: fedora-rawhide-i386
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R,
PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
------------------------------

1. Ignore `Installation errors`, since it's something wrong in system-wide, not
this package.

2. Fix all [!] items.

3. Drop changelog not from you. ALSO, leave a blank for each changelog entry.

4. Epoch:     1

This doesn't make sense at all.

If you submit something to Fedora, could you copy the spec form others
wholesale?

5. Drop all Group tags.

6. 
```
BuildRequires: gtk3-devel
BuildRequires: dbus-devel
BuildRequires: NetworkManager-devel >= 0.9.10
BuildRequires: NetworkManager-glib-devel
BuildRequires: sstp-client-devel
BuildRequires: glib2-devel
BuildRequires: ppp-devel
BuildRequires: libtool intltool gettext
BuildRequires: libsecret-devel
BuildRequires: libnm-gtk-devel

Requires: gtk3
Requires: dbus
Requires: NetworkManager >= 0.9.10
Requires: sstp-client
Requires: ppp
Requires: shared-mime-info
Requires: gnome-keyring
```

RPM is not dumb like past, drop eplicit requires unless RPM can't detect and
pull in.

Plus, the people who wrote the spec put wrong requires of gnome-specific to
main package, dependency mess of course.

7. %if 0%{?fedora} > 17

Well, are we in the same contemporary fedora lifecycle?

8. Requires: ppp

Not enough, once ppp bumps the version, this plugin will be broken.

9. %setup -q -n %{name}-%{version}

->

%setup -q

10. 

if [ ! -f configure ]; then
  ./autogen.sh
fi

Well, if you want to make autogen work, you at least need to BR autoconf
automake libtool. Instead of getting packaging complex and overcomplicated,
please drop it.

11. %description -n NetworkManager-sstp-gnome
This package contains software for integrating VPN capabilities with
the SSTP server with NetworkManager (GNOME files).

Please rework the description, `with...with...` sounds redundant and not
grammatical.

------------------------------
AGAIN, if you submit something to Fedora, could you copy the spec form others
wholesale?

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