Bug 1327071

Summary: Review Request: libusnic_verbs - No-op libibverbs driver for the Cisco usNIC device
Product: [Fedora] Fedora Reporter: Honggang LI <honli>
Component: Package ReviewAssignee: Michal Schmidt <mschmidt>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: mschmidt, package-review
Target Milestone: ---Flags: mschmidt: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-04-25 23:53:02 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1315609    

Description Honggang LI 2016-04-14 08:42:58 UTC
Spec URL: http://people.redhat.com/honli/.4dbfd6a342c9ec5517f54ebc8d628aa4/libusnic_verbs.spec

SRPM URL: http://people.redhat.com/honli/.4dbfd6a342c9ec5517f54ebc8d628aa4/libusnic_verbs-2.0.1-1.fc25.src.rpm

Description: This is a dummy plugin for libibverbs for Cisco usNIC devices.

It's only purpose in life is to prevent libibverbs from noticing /sys
entries for Cisco usNIC devices and emitting a stderr warning that it
cannot find a userspace plugin to support that device.

Cisco does not support the userspace Verbs API for accessing its usNIC
devices.  The Libfabric API is provided for accessing Cisco usNIC
functionality (see http://libfabric.org/).

Fedora Account System Username: honli

Comment 2 Michal Schmidt 2016-04-14 09:57:30 UTC
> # This software is available to you under a choice of one of two
> # licenses.  You may choose to be licensed under the terms of the GNU
> # General Public License (GPL) Version 2, available from the file
> # COPYING in the main directory of this source tree, or the
> # BSD license below:
> #
> #     Redistribution and use in source and binary forms, with or
> #     without modification, are permitted provided that the following
> #     conditions are met:
> #
> #      - Redistributions of source code must retain the above
> #        copyright notice, this list of conditions and the following
> #        disclaimer.
> #
> #      - Redistributions in binary form must reproduce the above
> #        copyright notice, this list of conditions and the following
> #        disclaimer in the documentation and/or other materials
> #        provided with the distribution.
> #
> # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> # EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> # MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> # NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> # BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> # ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> # CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> # SOFTWARE.

Hm, what's with these license headers in spec files lately?
Most Fedora spec files do not have them, because they're implicitly covered
by the MIT license as specified in the Fedora contributor's agreement.
Oh I see. The spec file comes from the upstream tarball.
It's strange that they put in this license header, but did not add any
copyright header (who's the author? copyright holder? copyright
date?)

> Name:           libusnic_verbs

OK, the name follows the upstream name. On the other hand, with the "_verbs"
suffix it stands out like a sore thumb among other libibverbs plugins.
Whatever, I don't mind.

> Version:        2.0.1
> Release:        1%{?dist}
> Summary:        No-op libibverbs driver for the Cisco usNIC device
> License:        GPLv2 or BSD
> Url:            http://cisco.com/

https://github.com/cisco/libusnic_verbs would be a more useful URL.

> Source0:        https://github.com/cisco/%{name}/releases/download/v%{version}/%{name}-%{version}.tar.gz
> BuildRequires:  libibverbs-devel >= 1.2.0
> ExcludeArch:    s390 s390x

libibverbs itself has only "ExcludeArch: s390". Since libusnic_verbs is such
a simple dummy driver, it would most likely build fine on s390x.

Do you want to add?:
Provides: libibverbs-driver.%{_arch}

> %description
> This is a dummy plugin for libibverbs for Cisco usNIC devices.
> 
> It's only purpose in life is to prevent libibverbs from noticing /sys

s/It's/Its/  (a possessive, not contraction)

> entries for Cisco usNIC devices and emitting a stderr warning that it
> cannot find a userspace plugin to support that device.
> 
> Cisco does not support the userspace Verbs API for accessing its usNIC
> devices.  The Libfabric API is provided for accessing Cisco usNIC
> functionality (see http://libfabric.org/).
> 
> %prep
> %setup -q
> 
> %build
> %configure
> make %{?_smp_mflags}
> 
> %install
> %{make_install}
> # remove unpackaged files from the buildroot
> rm -f %{buildroot}%{_libdir}/*.la %{buildroot}%{_libdir}/libusnic_verbs.so
> 
> %files
> # All files are licensed in GPLv2.

Delete this comment. Is the software not dual-licensed GPLv2 / BSD?

> %{_libdir}/libusnic_verbs-rdmav2.so
> %{_sysconfdir}/libibverbs.d/usnic.driver
> %doc AUTHORS ChangeLog VERSION
> %license LICENSE

The LICENSE file refers to a file called COPYING, which should contain the
complete text of GPLv2. COPYING is missing in upstream. Please tell
upstream to add it.

> %changelog
> * Thu Apr 14 2016 Honggang Li <honli> - 2.0.1-1
> - Import libusnic_verbs for Fedora.

Comment 3 Honggang LI 2016-04-14 11:43:05 UTC
diff -up libusnic_verbs.spec.old libusnic_verbs.spec
--- libusnic_verbs.spec.old     2016-04-14 04:40:22.000000000 -0400
+++ libusnic_verbs.spec 2016-04-14 07:36:51.828223624 -0400
@@ -1,45 +1,18 @@
-# This software is available to you under a choice of one of two
-# licenses.  You may choose to be licensed under the terms of the GNU
-# General Public License (GPL) Version 2, available from the file
-# COPYING in the main directory of this source tree, or the
-# BSD license below:
-#
-#     Redistribution and use in source and binary forms, with or
-#     without modification, are permitted provided that the following
-#     conditions are met:
-#
-#      - Redistributions of source code must retain the above
-#        copyright notice, this list of conditions and the following
-#        disclaimer.
-#
-#      - Redistributions in binary form must reproduce the above
-#        copyright notice, this list of conditions and the following
-#        disclaimer in the documentation and/or other materials
-#        provided with the distribution.
-#
-# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
-# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
-# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
-# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
-# BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
-# ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
-# CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
-# SOFTWARE.
-
 Name:           libusnic_verbs
 Version:        2.0.1
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        No-op libibverbs driver for the Cisco usNIC device
+Provides:       libibverbs-driver.%{_arch}
 License:        GPLv2 or BSD
-Url:            http://cisco.com/
+Url:            https://github.com/cisco/libusnic_verbs
 Source0:        https://github.com/cisco/%{name}/releases/download/v%{version}/%{name}-%{version}.tar.gz
 BuildRequires:  libibverbs-devel >= 1.2.0
-ExcludeArch:    s390 s390x
+ExcludeArch:    s390
 
 %description
 This is a dummy plugin for libibverbs for Cisco usNIC devices.
 
-It's only purpose in life is to prevent libibverbs from noticing /sys
+Its only purpose in life is to prevent libibverbs from noticing /sys
 entries for Cisco usNIC devices and emitting a stderr warning that it
 cannot find a userspace plugin to support that device.
 
@@ -60,12 +33,17 @@ make %{?_smp_mflags}
 rm -f %{buildroot}%{_libdir}/*.la %{buildroot}%{_libdir}/libusnic_verbs.so
 
 %files
-# All files are licensed in GPLv2.
 %{_libdir}/libusnic_verbs-rdmav2.so
 %{_sysconfdir}/libibverbs.d/usnic.driver
 %doc AUTHORS ChangeLog VERSION
 %license LICENSE
 
 %changelog
+* Thu Apr 14 2016 Honggang Li <honli> - 2.0.1-2
+- Remove license header in the spec file.
+- Use a more useful source URL.
+- Enable building for s390x.
+- Remove license comment in file section.
+
 * Thu Apr 14 2016 Honggang Li <honli> - 2.0.1-1
 - Import libusnic_verbs for Fedora.

Comment 5 Michal Schmidt 2016-04-14 14:21:04 UTC
Package Review
==============

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


Issues:
=======
The COPYING file (full text of the GPLv2) is missing. The license header in
empty_usnic.c and the LICENSE file refer to it.
This should be fixed by upstream.
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

configure.ac uses AM_SILENT_RULES. Please call make with V=1 to make gcc
command lines visible in build.log.

Please use .tar.bz2 from upstream instead of .tar.gz. It's smaller.

Add a comment above the ExcludeArch:
# we don't have libibverbs on s390
or something similar.

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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[!]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
#### the COPYING file is missing
[x]: License field in the package spec file matches the actual license.
[x]: License file installed when any subpackage combination is installed.
[-]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[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]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
#### Underscores are usually disliked in package names, but an exception is in
     place for packages where it's already in the upstream name.
     https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Separators
[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.
[-]: Package is not known to require an ExcludeArch tag.
#### Justification: It does use ExcludeArch due to libibverbs itself using it.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 3 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: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[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]: 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]: 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:
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[-]: 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.
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[?]: 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]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[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:
[x]: Rpmlint is run on all installed packages.
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: libusnic_verbs-2.0.1-2.fc25.x86_64.rpm
          libusnic_verbs-debuginfo-2.0.1-2.fc25.x86_64.rpm
          libusnic_verbs-2.0.1-2.fc25.src.rpm
libusnic_verbs.x86_64: W: spelling-error Summary(en_US) libibverbs -> verbalizes
libusnic_verbs.x86_64: W: spelling-error Summary(en_US) usNIC -> snick
libusnic_verbs.x86_64: W: spelling-error %description -l en_US libibverbs -> verbalizes
libusnic_verbs.x86_64: W: spelling-error %description -l en_US usNIC -> snick
libusnic_verbs.x86_64: W: spelling-error %description -l en_US sys -> says, sis, syn
libusnic_verbs.x86_64: W: spelling-error %description -l en_US stderr -> std err, std-err, stander
libusnic_verbs.x86_64: W: spelling-error %description -l en_US userspace -> user space, user-space, users pace
libusnic_verbs.x86_64: W: non-conffile-in-etc /etc/libibverbs.d/usnic.driver
libusnic_verbs-debuginfo.x86_64: W: spelling-error Summary(en_US) libusnic -> libidinous
libusnic_verbs-debuginfo.x86_64: W: spelling-error %description -l en_US libusnic -> libidinous
libusnic_verbs.src: W: spelling-error Summary(en_US) libibverbs -> verbalizes
libusnic_verbs.src: W: spelling-error Summary(en_US) usNIC -> snick
libusnic_verbs.src: W: spelling-error %description -l en_US libibverbs -> verbalizes
libusnic_verbs.src: W: spelling-error %description -l en_US usNIC -> snick
libusnic_verbs.src: W: spelling-error %description -l en_US sys -> says, sis, syn
libusnic_verbs.src: W: spelling-error %description -l en_US stderr -> std err, std-err, stander
libusnic_verbs.src: W: spelling-error %description -l en_US userspace -> user space, user-space, users pace
libusnic_verbs.src:5: W: unversioned-explicit-provides libibverbs-driver.%{_arch}
libusnic_verbs.src: W: invalid-url Source0: https://github.com/cisco/libusnic_verbs/releases/download/v2.0.1/libusnic_verbs-2.0.1.tar.gz HTTP Error 403: Forbidden
3 packages and 0 specfiles checked; 0 errors, 19 warnings.




Requires
--------
libusnic_verbs (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libibverbs.so.1()(64bit)
    libibverbs.so.1(IBVERBS_1.0)(64bit)
    libibverbs.so.1(IBVERBS_1.1)(64bit)
    rtld(GNU_HASH)

libusnic_verbs-debuginfo (rpmlib, GLIBC filtered):



Provides
--------
libusnic_verbs:
    libibverbs-driver.x86_64
    libusnic_verbs
    libusnic_verbs(x86-64)
    libusnic_verbs-rdmav2.so()(64bit)

libusnic_verbs-debuginfo:
    libusnic_verbs-debuginfo
    libusnic_verbs-debuginfo(x86-64)



Unversioned so-files
--------------------
libusnic_verbs: /usr/lib64/libusnic_verbs-rdmav2.so

#### This is normal for a libibverbs plugin.

Source checksums
----------------
https://github.com/cisco/libusnic_verbs/releases/download/v2.0.1/libusnic_verbs-2.0.1.tar.gz :
  CHECKSUM(SHA256) this package     : 7f09c7c315ab59e8565b2ed3f540b6496e5378d83bf0554fbeb257408e3118e3
  CHECKSUM(SHA256) upstream package : 7f09c7c315ab59e8565b2ed3f540b6496e5378d83bf0554fbeb257408e3118e3


Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20
Command line :/usr/bin/fedora-review -b 1327071
Buildroot used: fedora-rawhide-x86_64
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

Comment 6 Honggang LI 2016-04-15 02:50:09 UTC
diff -up libusnic_verbs.spec.old libusnic_verbs.spec
--- libusnic_verbs.spec.old     2016-04-14 22:32:37.494347625 -0400
+++ libusnic_verbs.spec 2016-04-14 22:38:13.292391360 -0400
@@ -1,12 +1,13 @@
 Name:           libusnic_verbs
-Version:        2.0.1
-Release:        2%{?dist}
+Version:        2.0.2
+Release:        1%{?dist}
 Summary:        No-op libibverbs driver for the Cisco usNIC device
 Provides:       libibverbs-driver.%{_arch}
 License:        GPLv2 or BSD
 Url:            https://github.com/cisco/libusnic_verbs
-Source0:        https://github.com/cisco/%{name}/releases/download/v%{version}/%{name}-%{version}.tar.gz
+Source0:        https://github.com/cisco/%{name}/releases/download/v%{version}/%{name}-%{version}.tar.bz2
 BuildRequires:  libibverbs-devel >= 1.2.0
+# we don't have libibverbs on s390
 ExcludeArch:    s390
 
 %description
@@ -25,7 +26,7 @@ functionality (see http://libfabric.org/
 
 %build
 %configure
-make %{?_smp_mflags}
+make %{?_smp_mflags} V=1
 
 %install
 %{make_install}
@@ -36,9 +37,12 @@ rm -f %{buildroot}%{_libdir}/*.la %{buil
 %{_libdir}/libusnic_verbs-rdmav2.so
 %{_sysconfdir}/libibverbs.d/usnic.driver
 %doc AUTHORS ChangeLog VERSION
-%license LICENSE
+%license COPYING
 
 %changelog
+* Thu Apr 14 2016 Honggang Li <honli> - 2.0.2-1
+- Rebase to upstream 2.0.2 which fixes the license issue.
+
 * Thu Apr 14 2016 Honggang Li <honli> - 2.0.1-2
 - Remove license header in the spec file.
 - Use a more useful source URL.

Comment 8 Michal Schmidt 2016-04-15 08:26:48 UTC
No more issues. Thanks!
Approved.

Comment 9 Michal Schmidt 2016-04-15 08:48:30 UTC
I forgot to suggest to add "BuildRequires: gcc".
http://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B#BuildRequires_and_Requires

Comment 10 Honggang LI 2016-04-15 08:49:39 UTC
I will add it. Thanks.

Comment 11 Gwyn Ciesla 2016-04-15 12:28:57 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/libusnic_verbs

Comment 12 Fedora Update System 2016-04-17 13:08:34 UTC
libusnic_verbs-2.0.2-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-0c4160f251

Comment 13 Fedora Update System 2016-04-17 13:08:41 UTC
libusnic_verbs-2.0.2-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-2648a2fb05

Comment 14 Fedora Update System 2016-04-18 04:52:09 UTC
libusnic_verbs-2.0.2-1.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-2648a2fb05

Comment 15 Fedora Update System 2016-04-18 04:55:29 UTC
libusnic_verbs-2.0.2-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-0c4160f251

Comment 16 Fedora Update System 2016-04-25 23:53:00 UTC
libusnic_verbs-2.0.2-1.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

Comment 17 Fedora Update System 2016-08-29 18:55:13 UTC
libusnic_verbs-2.0.2-1.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.