Bug 1123511 - Review Request: nanomsg - A fast, scalable, and easy to use socket library
Review Request: nanomsg - A fast, scalable, and easy to use socket library
Status: CLOSED DEFERRED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
: 1012392 (view as bug list)
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2014-07-25 17:36 EDT by Japheth Cleaver
Modified: 2016-07-29 09:25 EDT (History)
12 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-07-29 09:25:55 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Japheth Cleaver 2014-07-25 17:36:02 EDT
Spec URL: http://terabithia.org/rpms/misc/f20/nanomsg.spec
SRPM URL: http://terabithia.org/rpms/misc/f20/nanomsg-0.4-0.2.beta.fc20.src.rpm
Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=7162656


Hello,

With the closure of BZ#1012392 (a different contributor's review request for nanomsg), I've decided to pick up the ball and submit an RPM for review myself.


Description:  nanomsg is a socket library that provides several common communication patterns. It aims to make the networking layer fast, scalable, and easy to use. The communication patterns, also called "scalability protocols", are basic blocks for building distributed systems. By combining them you can create a vast array of distributed applications.



This project seems to be gaining in popularity within its niche (message queuing libraries), and I'd been investigating using it within another (non-Fedora-bundled) OSS project. It feels like this package would be a useful addition to Fedora.


Fedora Account System Username: cleaver

As this is my first official package submission, I would need sponsorship to become the package maintainer... however I'd like to see some version of nanomsg in Fedora regardless.


About me:
I've been using Fedora and its predecessor since the Cartman days and have been building RPMs regularly since around 2004. Various BZ's I've been involved with are at https://www.google.com/search?q="Japheth+Cleaver"&sitesearch=bugzilla.redhat.com
Comment 1 Christopher Meng 2014-07-25 23:35:48 EDT
*** Bug 1012392 has been marked as a duplicate of this bug. ***
Comment 2 Michael Schwendt 2014-07-26 09:59:09 EDT
A couple of minor issues:


> BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)

> %defattr(-,root,root)

> %clean
> rm -fR %{buildroot}

These are only needed for EL5, so unless you want to build for EL5, consider dropping them:

https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions
https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean


> %if %{with static}
> 	%configure --enable-doc --enable-debug --enable-static
> %else
> 	%configure --enable-doc --enable-debug --disable-static
> %endif

IMO, the following would be smarter (since you would not need to modify two %configure calls whenever you wanted to change the options):

  %configure --enable-doc --enable-debug \
      %{?_with_static:--enable-static}%{!?_with_static:--disable-static}


> make %{?_smp_mflags}

The build.log output is non-verbose (=silent). One cannot see which preprocessor/compiler flags and parameters are active.

One work-around would be to call the configure script with option --disable-silent-rules, another would be to call make with V=1.


Doing that, it turns out the package is built without Fedora's global flags:
https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags



> %files devel
> %doc ChangeLog COPYING README

> %files utils
> %doc ChangeLog COPYING README

Both subpackages don't need to duplicate these %doc files, because they (unlike the -doc subpkg) depend on the base package already. Plus, note that many users prefer changelog files to be included in the base runtime package anyway.


> %{_libdir}/pkgconfig/*.pc

Umm, ... it says "Version: 2.0.2" which is neither the package %version (0.4 beta) nor the lib SONAME version (0, 0.2.0). Anything known about this confusing versioning?
Comment 3 Japheth Cleaver 2014-07-27 14:14:40 EDT
Thanks for the comments. Some responses:

- I'd like to keep the EL builds a possibility, definitely.
- config line updated to use that macro pattern, thanks
- turns out the --enable-debug did more than simply not strip the binaries. I've removed that and bypassed stripping another way. I believe the options should be correct now.
- doc files updated

The pkgconfig Version is coming from the ABI version in https://github.com/nanomsg/nanomsg/blob/master/src/nn.h, ultimately. As for the SONAME itself, that's... interesting. I'm not sure on the history there. I'll look into it further.

I've updated the versions at:
http://terabithia.org/rpms/misc/f20/nanomsg-0.4-0.3.beta.fc20.src.rpm
http://terabithia.org/rpms/misc/f20/nanomsg.spec

Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=7200652
Comment 4 Japheth Cleaver 2014-07-29 16:54:33 EDT
Based on http://www.freelists.org/post/nanomsg/Brief-introduction-and-pkgconfig-vs-SONAME-question,4 and (e.g.) http://blog.asleson.org/index.php/2014/07/08/libtool-library-versioning-version-info-currentrevisionage/, this seems to be the expected (if slightly lexicographically confusing) behavior.

The ABI has not had anything deprecated yet, so it's still at SONAME 0. Package release version is unrelated.


Any other issues seen with this package?
Comment 5 Michael Schwendt 2014-07-30 06:39:28 EDT
Yes, "libtool versioning" is this,

  http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html#Updating-version-info

but while it is certainly possible to map the  current:revision:age  values into the release versioning scheme (e.g. 2:0:2 -> nanomsg-2.0.2), this has not been done here because the release is nanomsg-0.4-beta.

So, RPM package %version = 0.4 does not match the version in the pkgconfig .pc file. This also affects RPM BuildRequires on pkgconfig files ( https://fedoraproject.org/wiki/Packaging:PkgConfigBuildRequires ), pkgconfig based queries (see "pkg-config --help|grep VERS") and .pc file inter-dependencies.

I would have expected the nanonmsg release version to be the same as the .pc file version. Either 2.x.y or 0.x. With a leading 0 being better because it would match the SONAME version.

[...]

> turns out the --enable-debug did more than simply not strip the binaries.

Good catch. That's one of the reasons why verbose build output can be helpful.


> Any other issues seen with this package?

From "fedora-review -b 1123511":

| nanomsg-devel.x86_64: W: only-non-binary-in-usr-lib

False positive.


| [ ]: Large documentation must go in a -doc subpackage. Large could be size
|      (~1MB) or number of files.
|      Note: Documentation size is 296960 bytes in 3 files.

The currently small size of the -doc package makes it questionable. Just note that you are free to include those files in the -devel package. It could still be split off in the future in case the size would increase a lot.


| [ ]: Fully versioned dependency in subpackages if applicable.
|      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in nanomsg-
|      utils , nanomsg-doc

1) It's good that a normal -doc package does not depend on the base library package.

2) For nanomsg-utils, it's debatable. Some packagers refuse to add a base package Requires to such subpackages, because the automatic SONAME dep is present already and sufficient for up-to-date installations. The only benefit of the base package Requires would be to make the dep more strict, i.e. installation of nanomsg-utils linked with API additions would strictly require the corresponding nanomsg base package. An older lib release with the same SONAME would not suffice due to missing the new symbols.


[...]

Have you thought about attempting at doing a few reviews?

 * http://fedoraproject.org/PackageReviewStatus/
 * https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
Comment 6 Christopher Meng 2014-08-20 00:23:33 EDT
PING!
Comment 7 Fabian Affolter 2014-11-10 03:33:46 EST
Any progress?
Comment 8 Japheth Cleaver 2014-11-14 20:07:29 EST
Hello. I was actually waiting on the 0.5 beta release, which just dropped today.

Aside from the upstream fixes, there's no specific change here. SONAME was bumped to libnanomsg.so.0.2.1, with pkgconfig at 2.1.2 (per upstream nn.h generation).

I'd considered moving -doc back into the main package, but with the recent emphasis on pruning down size as much as humanly possible I suppose this is a decent future-proof for those cloud use cases.


I have not had a chance to do any other package reviews myself, unfortunately, but am certainly willing to look out for them.


I've updated the versions at:
http://terabithia.org/rpms/misc/f20/nanomsg-0.5-0.1.beta.fc20.src.rpm
http://terabithia.org/rpms/misc/f20/nanomsg.spec

Koji build logs: 
Rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=8148256
Fedora20: http://koji.fedoraproject.org/koji/taskinfo?taskID=8148325
EPEL7: http://koji.fedoraproject.org/koji/taskinfo?taskID=8148332
Comment 9 Michael Schwendt 2014-12-25 09:46:16 EST
Not much left:

* SONAME has not changed. It's still libnanomsg.so.0


* If you insist on creating a separate -doc package for only 39 files with a total size of 1,021,705 bytes, also consider adding "BuildArch: noarch" to that subpackage.



# pkg-config libnanomsg --libs
-lnanomsg -lanl -lrt 

Kinda odd to relink with libanl and librt since libnanomsg is linked with those two already, and the nanomsg API does not pull in anything for those two libs:

  # grep .*#include /usr/include/nanomsg/*
  /usr/include/nanomsg/nn.h:#include <errno.h>
  /usr/include/nanomsg/nn.h:#include <stddef.h>
  /usr/include/nanomsg/reqrep.h:#include "nn.h"



The testsuite succeeds with this release, provided that the spec file points the runtime linker at the freshly built lib:

--- nanomsg.spec.ORIG	2014-11-15 00:34:06.000000000 +0100
+++ nanomsg.spec	2014-12-25 15:42:38.301122955 +0100
@@ -115,7 +115,7 @@
 %check
 # conditional: currently failing tests: https://github.com/nanomsg/nanomsg/issues/281
 %if %{with check}
-make check DESTDIR="%{buildroot}"
+LD_LIBRARY_PATH=$(pwd)/.libs make check
 %endif

============================================================================
Testsuite summary for nanomsg 0.5-beta
============================================================================
# TOTAL: 31
# PASS:  31
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================
Comment 10 Fabian Affolter 2015-03-06 17:18:20 EST
Are there any plans to go with this?
Comment 11 Pavel Zhukov 2015-07-26 14:46:56 EDT
(In reply to Fabian Affolter from comment #10)
> Are there any plans to go with this?
Comment 12 Upstream Release Monitoring 2015-10-29 14:19:44 EDT
cleaver's scratch build of nanomsg-0.7-0.1.beta.fc20.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11629931
Comment 13 Japheth Cleaver 2015-10-29 14:29:34 EDT
(In reply to Upstream Release Monitoring from comment #12)
> cleaver's scratch build of nanomsg-0.7-0.1.beta.fc20.src.rpm for rawhide
> completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11629931

The "Currently fails checks: https://github.com/nanomsg/nanomsg/issues/281" line should be removed from the .spec file (since it now passes fine, so long as LD_LIBRARY_PATH is specified), and the dist tag indicates this was a rebuild of an f20 SRPM, however the functionality and file list should be correct.
Comment 14 Pavel Zhukov 2015-11-27 10:08:23 EST
0.8-beta has been released
Comment 15 Neal Gompa 2015-11-28 21:00:12 EST
@Japheth: Are you still trying to get this in? It'd be great if it was in Fedora.
Comment 16 Miroslav Suchý 2016-07-29 09:25:55 EDT
No response for past 8 months. Closing. If you want to continue, please reopen.

Note You need to log in before you can comment on or make changes to this bug.