Spec URL: http://www.mellanox.com/downloads/Accelerator/libvma.spec SRPM URL: http://www.mellanox.com/downloads/Accelerator/libvma-7.0.13-1.fc23.src.rpm Description: libvma is dynamically linked user space library for enhancing the performance of TCP and UDP networking-heavy applications over RDMA-capable NICs. It allows application written over standard socket API to run with full network stack bypass from user-space which result in latency reduction and increased throughput and packet rate Fedora Account System Username: avnerbh
Hello Avner, you're not yet in the "packager" group, so I'm marking this review request as needing a sponsor. I will do a review.
Just a few things I noticed (not yet a full review): - You're using too many macros for my taste. Some of them are entirely unused. - Some spec file tags are unnecessary these days (Group, BuildRoot, the %clean step, %defattr). - I don't like messing with limits.conf in %post like that. If you really need to reconfigure the limits, consider shipping a fragment file in /etc/security/limits.d/
A new branch named "fedora" was created under https://github.com/Mellanox/libvma Please switch to fedora branch by selecting the branch name on the upper left "branch:" button. All packaging related development will be handled on this branch before being merged into the master branch.
alexva's scratch build of libvma-7.0.14-1.fc24.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=13075948
Hi Michal, Following your feedback we’ve updated the libvma.spec file: http://www.mellanox.com/downloads/Accelerator/libvma.spec and created a new libvma source RPM file: http://www.mellanox.com/downloads/Accelerator/libvma-7.0.14-1.fc24.src.rpm Please review. Thanks, AlexV
Branch "fedora" under https://github.com/Mellanox/libvma was merged into master branch. All packaging related code is under master branch
> ExcludeArch: %{arm} %{ix86} s390x ppc64 ppc64le Every use of ExcludeArch should have a comment explaining the reason. > %description > libvma is a LD_PRELOAD-able library that boosts performance > of TCP and UDP traffic Missing a dot to end the sentence. > Part of Mellanox's enhanced services Does it mean libvma works only with Mellanox hardware? If yes, please rephrase it in a clearer way. If it does not mean that, then does it mean anything at all? > Allows application written over standard socket API > to run over Infiniband/Ethernet from user space with full network stack bypass > and get better throughput, latency and packets/sec rate. It allows applications [...] > %pre > if [ `grep "^[^#]" /etc/security/limits.conf /etc/security/limits.d/* 2> /dev/null |grep memlock|grep unlimited | wc -l` -le 0 ]; then Looks like it could be simplified to: if ! grep -qs '^[^#].*memlock.*unlimited' /etc/security/limits.conf /etc/security/limits.d/*; then Anyway, do you really need to emit the message? It is unusual for a scriptlet in Fedora to emit any output whatsoever. > %files > %{_libdir}/%{name}*.so.* > %{_libdir}/%{name}.so You may want to add a comment here saying you need libvma.so in the main package (as opposed to -devel) so that 'LD_PRELOAD=libvma.so <command>' works. > %doc README.txt journal.txt VMA_VERSION I see your upstream tarball contains a COPYING file, that you're not including in the package. Ideally it would contain the full text of the GPLv2 (Would you please make that change upstream? Currently it has only a copy of the header as it appears in the source files.) and be included in the package using the %license tag (not %doc).
(In reply to Michal Schmidt from comment #7) > > ExcludeArch: %{arm} %{ix86} s390x ppc64 ppc64le > > Every use of ExcludeArch should have a comment explaining the reason. I will fix it. > > %description > > libvma is a LD_PRELOAD-able library that boosts performance > > of TCP and UDP traffic > > Missing a dot to end the sentence. I will fix it. > > Part of Mellanox's enhanced services > > Does it mean libvma works only with Mellanox hardware? > If yes, please rephrase it in a clearer way. > If it does not mean that, then does it mean anything at all? libvma can be supported by RDMA capable devices that support IBV_QPT_RAW_PACKET QPs. I will rephrase the description. > > Allows application written over standard socket API > > to run over Infiniband/Ethernet from user space with full network stack bypass > > and get better throughput, latency and packets/sec rate. > > It allows applications [...] I will fix it. > > %pre > > if [ `grep "^[^#]" /etc/security/limits.conf /etc/security/limits.d/* 2> /dev/null |grep memlock|grep unlimited | wc -l` -le 0 ]; then > Looks like it could be simplified to: > if ! grep -qs '^[^#].*memlock.*unlimited' /etc/security/limits.conf > /etc/security/limits.d/*; then > > Anyway, do you really need to emit the message? > It is unusual for a scriptlet in Fedora to emit any output whatsoever. We print this message in order to notify about system configuration change (memlock limit). I agree that this is not crucial and can be removed. > > %files > > %{_libdir}/%{name}*.so.* > > %{_libdir}/%{name}.so > > You may want to add a comment here saying you need libvma.so in the main > package > (as opposed to -devel) so that 'LD_PRELOAD=libvma.so <command>' works. Good idea, I will add this comment. > > %doc README.txt journal.txt VMA_VERSION > > I see your upstream tarball contains a COPYING file, that you're not > including in the package. > Ideally it would contain the full text of the GPLv2 > (Would you please make that change upstream? > Currently it has only a copy of the header as it appears in the source > files.) I am checking if we can make this change. > and be included in the package using the %license tag (not %doc). I will add the COPYING file to the package under the %license tag.
alexva's scratch build of libvma-7.0.14-1.fc24.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=13185983
alexva's scratch build of libvma-7.0.14-1.fc24.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=13187585
alexva's scratch build of libvma-7.0.14-1.fc24.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=13190047
alexva's scratch build of libvma-7.0.14-1.fc24.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=13191401
alexva's scratch build of libvma-7.0.14-2.fc24.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=13215203
Hi Michal, I've updated the package: * Wed Mar 2 2016 Alex Vainman <alexv> - 7.0.14-2 - Added reasoning for archs exclusion - Package description improvement - Removal of the pre scriplet - Added COPYING and LICENSE files to the package Notice: a new packaged "LICENSE" file contains the full GPLv2 license text as you requested. Spec URL: http://www.mellanox.com/downloads/Accelerator/libvma.spec SRPM URL: http://www.mellanox.com/downloads/Accelerator/libvma-7.0.14-2.fc24.src.rpm
[We're getting close to completion of this review, so I'm reassigning this to Neil Horman who has package sponsor powers and tentatively agreed to sponsor you. I remain on the CC list.] I see you excluded most of the architectures, just because libvma was not tested on them. I think your standards for inclusion are too high :-) For example, very few Fedora packagers test their packages on i686 these days, but they don't exclude that arch. But whatever, I'll leave that choice to you. Note that you misspelled "libvma" as "libmva" in the comments. I realized the COPYING file (just like the headers in the source files) is somewhat contradictory: /* * Copyright (C) Mellanox Technologies Ltd. 2001-2013. ALL RIGHTS RESERVED. * * This software product is a proprietary product of Mellanox Technologies Ltd. * (the "Company") and all right, title, and interest in and to the software product, * including all associated intellectual property rights, are and shall * remain exclusively with the Company. The above paragraph makes it look like the source code was never meant to leak outside the Company. Maybe I'm misunderstanding it. * * This software is made available under either the GPL v2 license or a commercial license. OK, a relief! To clear any doubts, would it be possible to apply the GPLv2 the way the FSF recommends it?: http://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html#SEC4 Just make it "version 2 of the License" without the "or (at your option) any later version". * If you wish to obtain a commercial license, please contact Mellanox at support. */ OK, nothing wrong with a small ad. This line can stay. In %build I recommend running make with V=1 so that the compiler command lines are visible in build.log. Here's edited output from the fedora-review tool: Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated Issues: ======= (fedora-review reported an installation error, which was a false positive. It also complained about *.so in the main package, but we agreed this is necessary in this case.) ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Header files in -devel subpackage, if present. [x]: ldconfig called in %post and %postun if required. [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: "BSD (3 clause)", "BSD (2 clause)", "GPL (v3 or later)", "Unknown or generated". 277 files have unknown license. Detailed output of licensecheck in /home/michich/1298665-libvma/licensecheck.txt ### The "unknown license" is GPLv2 - the source files use a different license header than the one recommended by FSF. The GPLv3 files are src/vma/config_parser.[ch] - they are generated by GNU Bison and they permit the use in non-GPLv3 software by the usual Bison license exception. [x]: License file installed when any subpackage combination is installed. [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. [!]: %config files are marked noreplace or the reason is justified. Note: No (noreplace) in %config (noreplace) /etc/security/limits.d/30 -libvma-limits.conf ### I'm not sure if fedora-review is stricter than rpmbuild about the whitespace, but you'd better remove the space. [x]: Package contains desktop file if it is a GUI application. [x]: 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. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [x]: 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. [x]: 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. ### Well, it does use ExcludeArch, but the use is explained. So OK. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 122880 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]: 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: [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 libvma- debuginfo [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Scriptlets must be sane, if used. [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]: 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: [!]: Rpmlint is run on all installed packages. 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. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: libvma-7.0.14-2.fc25.x86_64.rpm libvma-devel-7.0.14-2.fc25.x86_64.rpm libvma-utils-7.0.14-2.fc25.x86_64.rpm libvma-debuginfo-7.0.14-2.fc25.x86_64.rpm libvma-7.0.14-2.fc25.src.rpm libvma.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libvma.so libvma-devel.x86_64: W: no-documentation libvma-utils.x86_64: W: no-documentation libvma-utils.x86_64: W: no-manual-page-for-binary vma_stats 5 packages and 0 specfiles checked; 0 errors, 4 warnings. Requires -------- libvma-devel (rpmlib, GLIBC filtered): libvma(x86-64) libvma-debuginfo (rpmlib, GLIBC filtered): libvma (rpmlib, GLIBC filtered): /sbin/ldconfig config(libvma) ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libdl.so.2()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libibverbs.so.1()(64bit) libibverbs.so.1(IBVERBS_1.0)(64bit) libibverbs.so.1(IBVERBS_1.1)(64bit) libm.so.6()(64bit) libnl-3.so.200()(64bit) libnl-3.so.200(libnl_3)(64bit) libnl-route-3.so.200()(64bit) libnl-route-3.so.200(libnl_3)(64bit) libpthread.so.0()(64bit) librdmacm.so.1()(64bit) librdmacm.so.1(RDMACM_1.0)(64bit) librt.so.1()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.8)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) libvma.so.7()(64bit) pam rtld(GNU_HASH) libvma-utils (rpmlib, GLIBC filtered): libc.so.6()(64bit) libdl.so.2()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libm.so.6()(64bit) libpthread.so.0()(64bit) librt.so.1()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) libvma(x86-64) rtld(GNU_HASH) Provides -------- libvma-devel: libvma-devel libvma-devel(x86-64) libvma-debuginfo: libvma-debuginfo libvma-debuginfo(x86-64) libvma: config(libvma) libvma libvma(x86-64) libvma.so.7()(64bit) libvma-utils: libvma-utils libvma-utils(x86-64) Unversioned so-files -------------------- libvma: /usr/lib64/libvma.so Source checksums ---------------- http://www.mellanox.com/downloads/Accelerator/libvma-7.0.14.tar.gz : CHECKSUM(SHA256) this package : c13a6576e3d94cff9e5456d982623253fa5e07051b01de4223301f2adcb9cc59 CHECKSUM(SHA256) upstream package : c13a6576e3d94cff9e5456d982623253fa5e07051b01de4223301f2adcb9cc59 AutoTools: Obsoleted m4s found ------------------------------ AC_PROG_LIBTOOL found in: libvma-7.0.14/configure.ac:28 Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20 Command line :/usr/bin/fedora-review -b 1298665 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
(In reply to Michal Schmidt from comment #15) > [We're getting close to completion of this review, so I'm reassigning this > to Neil Horman who has package sponsor powers and tentatively agreed > to sponsor you. I remain on the CC list.] > > I see you excluded most of the architectures, just because libvma was > not tested on them. I think your standards for inclusion are too high :-) > For example, very few Fedora packagers test their packages on i686 these > days, but they don't exclude that arch. > But whatever, I'll leave that choice to you. I see, we will reconsider this. > Note that you misspelled "libvma" as "libmva" in the comments. > I will fix the spelling. > > I realized the COPYING file (just like the headers in the source files) > is somewhat contradictory: > /* > * Copyright (C) Mellanox Technologies Ltd. 2001-2013. ALL RIGHTS RESERVED. > * > * This software product is a proprietary product of Mellanox Technologies > Ltd. > * (the "Company") and all right, title, and interest in and to the software > product, > * including all associated intellectual property rights, are and shall > * remain exclusively with the Company. > > The above paragraph makes it look like the source code was never meant > to leak outside the Company. Maybe I'm misunderstanding it. > > * > * This software is made available under either the GPL v2 license or a > commercial license. > > OK, a relief! > > To clear any doubts, would it be possible to apply the GPLv2 the way > the FSF recommends it?: > http://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html#SEC4 > Just make it "version 2 of the License" > without the "or (at your option) any later version". > I am checking if we can make this change. > * If you wish to obtain a commercial license, please contact Mellanox at > support. > */ > > OK, nothing wrong with a small ad. This line can stay. > > > In %build I recommend running make with V=1 so that the compiler command > lines > are visible in build.log. > OK, I will add it. > > Here's edited output from the fedora-review tool: > > Package Review > ============== > > Legend: > [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated > > > Issues: > ======= > (fedora-review reported an installation error, which was a false positive. > It also complained about *.so in the main package, but we agreed this is > necessary in this case.) > > ===== MUST items ===== > > C/C++: > [x]: Package does not contain kernel modules. > [x]: Package contains no static executables. > [x]: Header files in -devel subpackage, if present. > [x]: ldconfig called in %post and %postun if required. > [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: "BSD (3 clause)", "BSD (2 clause)", "GPL (v3 or later)", > "Unknown or generated". 277 files have unknown license. Detailed > output of licensecheck in > /home/michich/1298665-libvma/licensecheck.txt > > ### The "unknown license" is GPLv2 - the source files use a different > license header than the one recommended by FSF. > The GPLv3 files are src/vma/config_parser.[ch] - they are generated > by GNU Bison and they permit the use in non-GPLv3 software by > the usual Bison license exception. > > [x]: License file installed when any subpackage combination is installed. > [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. > [!]: %config files are marked noreplace or the reason is justified. > Note: No (noreplace) in %config (noreplace) /etc/security/limits.d/30 > -libvma-limits.conf > > ### I'm not sure if fedora-review is stricter than rpmbuild about > the whitespace, but you'd better remove the space. > OK, I will fix this. > [x]: Package contains desktop file if it is a GUI application. > [x]: 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. > [x]: Package does not generate any conflict. > [x]: Package obeys FHS, except libexecdir and /usr/target. > [x]: 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. > [x]: 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. > > ### Well, it does use ExcludeArch, but the use is explained. So OK. > > [x]: Large documentation must go in a -doc subpackage. Large could be size > (~1MB) or number of files. > Note: Documentation size is 122880 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]: 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: > [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 libvma- > debuginfo > [?]: Package functions as described. > [x]: Latest version is packaged. > [x]: Package does not include license text files separate from upstream. > [x]: Scriptlets must be sane, if used. > [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]: 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: > [!]: Rpmlint is run on all installed packages. > 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. > [x]: Spec file according to URL is the same as in SRPM. > > > Rpmlint > ------- > Checking: libvma-7.0.14-2.fc25.x86_64.rpm > libvma-devel-7.0.14-2.fc25.x86_64.rpm > libvma-utils-7.0.14-2.fc25.x86_64.rpm > libvma-debuginfo-7.0.14-2.fc25.x86_64.rpm > libvma-7.0.14-2.fc25.src.rpm > libvma.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libvma.so > libvma-devel.x86_64: W: no-documentation > libvma-utils.x86_64: W: no-documentation > libvma-utils.x86_64: W: no-manual-page-for-binary vma_stats > 5 packages and 0 specfiles checked; 0 errors, 4 warnings. > > > > > Requires > -------- > libvma-devel (rpmlib, GLIBC filtered): > libvma(x86-64) > > libvma-debuginfo (rpmlib, GLIBC filtered): > > libvma (rpmlib, GLIBC filtered): > /sbin/ldconfig > config(libvma) > ld-linux-x86-64.so.2()(64bit) > libc.so.6()(64bit) > libdl.so.2()(64bit) > libgcc_s.so.1()(64bit) > libgcc_s.so.1(GCC_3.0)(64bit) > libibverbs.so.1()(64bit) > libibverbs.so.1(IBVERBS_1.0)(64bit) > libibverbs.so.1(IBVERBS_1.1)(64bit) > libm.so.6()(64bit) > libnl-3.so.200()(64bit) > libnl-3.so.200(libnl_3)(64bit) > libnl-route-3.so.200()(64bit) > libnl-route-3.so.200(libnl_3)(64bit) > libpthread.so.0()(64bit) > librdmacm.so.1()(64bit) > librdmacm.so.1(RDMACM_1.0)(64bit) > librt.so.1()(64bit) > libstdc++.so.6()(64bit) > libstdc++.so.6(CXXABI_1.3)(64bit) > libstdc++.so.6(CXXABI_1.3.8)(64bit) > libstdc++.so.6(CXXABI_1.3.9)(64bit) > libvma.so.7()(64bit) > pam > rtld(GNU_HASH) > > libvma-utils (rpmlib, GLIBC filtered): > libc.so.6()(64bit) > libdl.so.2()(64bit) > libgcc_s.so.1()(64bit) > libgcc_s.so.1(GCC_3.0)(64bit) > libm.so.6()(64bit) > libpthread.so.0()(64bit) > librt.so.1()(64bit) > libstdc++.so.6()(64bit) > libstdc++.so.6(CXXABI_1.3)(64bit) > libstdc++.so.6(CXXABI_1.3.9)(64bit) > libvma(x86-64) > rtld(GNU_HASH) > > > > Provides > -------- > libvma-devel: > libvma-devel > libvma-devel(x86-64) > > libvma-debuginfo: > libvma-debuginfo > libvma-debuginfo(x86-64) > > libvma: > config(libvma) > libvma > libvma(x86-64) > libvma.so.7()(64bit) > > libvma-utils: > libvma-utils > libvma-utils(x86-64) > > > > Unversioned so-files > -------------------- > libvma: /usr/lib64/libvma.so > > Source checksums > ---------------- > http://www.mellanox.com/downloads/Accelerator/libvma-7.0.14.tar.gz : > CHECKSUM(SHA256) this package : > c13a6576e3d94cff9e5456d982623253fa5e07051b01de4223301f2adcb9cc59 > CHECKSUM(SHA256) upstream package : > c13a6576e3d94cff9e5456d982623253fa5e07051b01de4223301f2adcb9cc59 > > > AutoTools: Obsoleted m4s found > ------------------------------ > AC_PROG_LIBTOOL found in: libvma-7.0.14/configure.ac:28 > > > Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20 > Command line :/usr/bin/fedora-review -b 1298665 > 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
>[We're getting close to completion of this review, so I'm reassigning this >to Neil Horman who has package sponsor powers and tentatively agreed >to sponsor you. I remain on the CC list.] Thanks Michal! Neil, nice to e-meet you :) Please let me know if I can do anything in order to promote this task. >I realized the COPYING file (just like the headers in the source files) >is somewhat contradictory: >/* > * Copyright (C) Mellanox Technologies Ltd. 2001-2013. ALL RIGHTS RESERVED. > * > * This software product is a proprietary product of Mellanox Technologies Ltd. > * (the "Company") and all right, title, and interest in and to the software product, > * including all associated intellectual property rights, are and shall > * remain exclusively with the Company. > >The above paragraph makes it look like the source code was never meant >to leak outside the Company. Maybe I'm misunderstanding it. > > * > * This software is made available under either the GPL v2 license or a commercial license. > >OK, a relief! > >To clear any doubts, would it be possible to apply the GPLv2 the way >the FSF recommends it?: >http://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html#SEC4 >Just make it "version 2 of the License" >without the "or (at your option) any later version". I’ve checked this matter with our legal department. Since libvma package is dual licensed: GPLv2 or proprietary license, we think that the wording in the COPYING and in the sources files is correct and accurate. Notice that this approach is common in the industry, see for example the license of MYSQL: http://www.mysql.com/about/legal/licensing/oem/ PS: I will send an updated package soon. Thanks, AlexV
alex, I appreciate you looking into it, but I don't believe that the legal dual licensing is going to be acceptable for fedora packaging (or Red Hat for that matter). I'm going to ask Red Hat Legal to clarify this but it seems to me that, while dual licensing is certainly a legal approach to take to open source code, both licenses must be open source compatible for us to pacakge the code in Fedora or RHEL. That is to say, you may have a package that contains both BSD and GPLv2 licensed code, and thats fine, as long as you clearly delineate which code is licensed in which way. However, you have a dual license situation in which code may be licensed under GPLv2 or some proprietary license. While the former is ok, the latter is not distributable, and you have several files in here that are very ambiguous, as they do not specify which license they are under. If they are all 100% GPLv2 licensed, then thats great, but its not a dual license situation then, its a single license, and the proprietary language needs to be removed. If there proprietary code here, then thats a problem of a different sort.
(In reply to Neil Horman from comment #19) > alex, I appreciate you looking into it, but I don't believe that the legal > dual licensing is going to be acceptable for fedora packaging (or Red Hat > for that matter). I'm going to ask Red Hat Legal to clarify this but it > seems to me that, while dual licensing is certainly a legal approach to take > to open source code, both licenses must be open source compatible for us to > pacakge the code in Fedora or RHEL. Neil, I don't think the dual license (GPLv2 or proprietary) is a problem for redistribution, but it can be problem w.r.t. contributing patches to upstream. The software is dual-licensed in the sense that the recipient can choose either one of the two licenses (GPLv2 or proprietary). So we can ignore the proprietary option and distribute the software under the terms of GPLv2. So far so good. Of course asking Legal is the right thing to do if you have any doubts. The only problem I can see is if we (or any other Free Software developer) develop a patch and send it back to Mellanox. If we contribute a patch under GPLv2, Mellanox will likely refuse to merge it, because then they couldn't distribute the resulting work under the proprietary license. Alex, how do you expect to handle patch contributions? Are you going to require copyright assignment agreements?
Thank you Neil and Michal for the comments. After intensive consulting we agreed to change libvma license: to GPLv2 or BSD (dual license). I think that this change should resolve the discussed issue and any ambiguities related to licensing. We are going to release a new libvma version with this change. As soon as the new version is ready I will send an updated package for review.
Michal, thats a question for legal to answer. Dual Licensing can mean any number of things, including your interpretation, or others in which certain files are licensed exclusively under a specific license (DPDK does this, electing GPLv2 for some files, and BSD for others, and a proprietary license for yet others). Fedora typically treats a dual license scenario on a per-file basis (each file selects which of the dual licenses applies to it). Regardless, any non-open source compatible license requires legal approval to be packaged and distributed. However, its a moot point, since Mellanox is being kind enough to fix the packaging to be dual licensed BSD and GPLv2, which solves the problem for us, as both of those are compatible. alex, for your reference, here are the fedora license guidelines: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines Should they help guide your conversion. Thank you for taking the time to clear this additional hurdle. Please post a new spec and srpm here when you have one available.
Thank you Neil for the pointer. I will post the new spec and srpm as soon as they are ready.
Hi Neil/Michal, I've updated the package: * Sun Mar 13 2016 Alex Vainman <alexv> - 8.0.1-1 - New upstream release - Move to dual license: GPLv2 or BSD - ExcludeArch update - Removal of extra space in: config(noreplace) {_sysconfdir}/security/limits.d/30-libvma-limits.conf - Add V=1 to make Spec URL: http://www.mellanox.com/downloads/Accelerator/libvma.spec SRPM URL: http://www.mellanox.com/downloads/Accelerator/libvma-8.0.1-1.fc24.src.rpm
Ok, this looks good to me. There are still two files in the tests subdirectory that claim to be proprietary licenses, but they don't seem to get included in the binary rpm, so I think thats ok. ACK from me. Avner, I presume you still need to get sponsored in the feodora system, correct? I can do that for you. Just to confirm avnerbh+mellanox is the email you registered for a fedora account with, correct?
ping, I need to know what fedora account system email/username you are using so that I can approve you as a pacakger
Hi Neil, Following are my accounts details Fedora account ============== Account Name: ophirmunk Full Name: Ophir Munk Email: ophirmu redhat.bugzilla.com =================== Real Name: Ophir Munk Email: ophirmu Please approve me as a packager
done, please complete the packaging process here: https://fedoraproject.org/wiki/Package_Review_Process To create the package in dist-git.
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/libvma
Checked with 'fedpkg clone libvma', package had been created in dist-git. libvma-8.0.1-1.fc24/25 had been built. http://koji.fedoraproject.org/koji/buildinfo?buildID=746017 http://koji.fedoraproject.org/koji/buildinfo?buildID=746001