Bug 1012392

Summary: Review Request: nanomsg - nanomsg is a socket library that provides several common communication patterns
Product: [Fedora] Fedora Reporter: Zoltan Boszormenyi <zboszor>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: cleaver-redhat, i, karlthered, mail, tomspur
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-07-25 10:41:47 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:

Description Zoltan Boszormenyi 2013-09-26 11:53:42 UTC
Spec URL: http://boszormenyi.dyndns.biz/nanomsg.spec
SRPM URL: http://boszormenyi.dyndns.biz/nanomsg-0.2-2alpha.fc19.src.rpm
Description: Successor of ZeroMQ reimplemented in plain C
Fedora Account System Username: zboszor

This is my first package, I need a sponsor.

Although a very similar functionality is provided by zeromq and zeromq3 packages, this library is the successor. Indeed, the author of zeromq and nanomsg is the same, he decided to reimplement it in plain C to avoid dependency on the C++ runtime library.

Comment 1 Zoltan Boszormenyi 2013-09-26 12:01:49 UTC
Koj

Comment 2 Zoltan Boszormenyi 2013-09-26 12:03:08 UTC
koji scratch builds were completed for f20, f19, f18 and dist-6E-epel
dist-5E-epel failed for some reason very early

Comment 3 Zoltan Boszormenyi 2013-09-26 12:03:50 UTC
$ rpmlint -i -v nanomsg.spec
nanomsg.spec: I: checking-url http://download.nanomsg.org/nanomsg-0.2-alpha.tar.gz (timeout 10 seconds)
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

Comment 4 Christopher Meng 2013-09-26 13:08:50 UTC
CCing cuz I've packaged it.

Informal review:

1. Summary:	nanomsg is a socket library that provides several common communication patterns.

Bad.

Suggestion:

Summary:	A socket library which provides several common communication patterns

2. Remove BuildRoot tag.

3. It's nonsense to Requires:	pkgconfig in -devel subpackage.

4. Why do we need static libs? If you don't recommend using it, you can drop it, or there really have people like static linking?

5. devel subpackage has:

Requires:	%{name} = %{version}-%{release}

Should use:

Requires:       %{name}%{?_isa} = %{version}-%{release}

6. Put %check after %install

7. Remove rm -rf $RPM_BUILD_ROOT in %install

8. Why rm -f $RPM_BUILD_ROOT%{_infodir}/dir ?

9. Remove %clean section.

10. Remove %defattr(-,root,root,-)

11. Use %doc to define which are docs.

12. %dir %{_includedir}/%{name}
%{_includedir}/%{name}/*.h

Just %{_includedir}/%{name} is the best.

13. I don't think you need to say "It is licensed under MIT/X11 license." in %description, Fedora only allows OPEN SOURCE projects.

Comment 5 Zoltan Boszormenyi 2013-09-26 15:38:49 UTC
(In reply to Christopher Meng from comment #4)
> CCing cuz I've packaged it.
> 
> Informal review:
> 
> 1. Summary:	nanomsg is a socket library that provides several common
> communication patterns.
> 
> Bad.
> 
> Suggestion:
> 
> Summary:	A socket library which provides several common communication
> patterns

Done.

> 
> 2. Remove BuildRoot tag.

Done.

> 
> 3. It's nonsense to Requires:	pkgconfig in -devel subpackage.

Done.

> 4. Why do we need static libs? If you don't recommend using it, you can drop
> it, or there really have people like static linking?

Removed the static subpackage.

> 5. devel subpackage has:
> 
> Requires:	%{name} = %{version}-%{release}
> 
> Should use:
> 
> Requires:       %{name}%{?_isa} = %{version}-%{release}

Done.

> 6. Put %check after %install

Done.

> 7. Remove rm -rf $RPM_BUILD_ROOT in %install

Done.

> 8. Why rm -f $RPM_BUILD_ROOT%{_infodir}/dir ?

It was there because I used another Fedora package as a start. Removed.

> 
> 9. Remove %clean section.

Done.

> 10. Remove %defattr(-,root,root,-)

Done.

> 11. Use %doc to define which are docs.

There was already

%doc AUTHORS ChangeLog COPYING README

in the main package.

"make install" installs man pages into %{_mandir} and *.html pages
(generated from the same source as the man pages) into %{_docdir}/%{name}.
I marked the *.html as %doc in the devel subpackage.

> 12. %dir %{_includedir}/%{name}
> %{_includedir}/%{name}/*.h
> 
> Just %{_includedir}/%{name} is the best.

Done.

> 13. I don't think you need to say "It is licensed under MIT/X11 license." in
> %description, Fedora only allows OPEN SOURCE projects.

I removed this last statement, it's redundant with "License:" anyway.
I copied the description from http://nanomsg.org/index.html

The new version is at the same URL as above:

Spec URL: http://boszormenyi.dyndns.biz/nanomsg.spec
SRPM URL: http://boszormenyi.dyndns.biz/nanomsg-0.2-3alpha.fc19.src.rpm

Comment 6 Haïkel Guémar 2013-10-06 16:25:05 UTC
I was planning to package it but i got delayed by an event as fp.o ambassador (where i gave a talk on nanomsg:) )
Good point, it passed the scratch build test: http://koji.fedoraproject.org/koji/taskinfo?taskID=6029041

I'm a sponsor, i can take care of that part if you agree.
For a beginning, start by doing two informal reviews (please, put me in CC) and link them in this ticket.
I'll also drill you on some fedora packaging guidelines, either by mail or irc.

1. Group tags are not required anymore, remove them
2. Release tag is not handled correctly for a pre-release, it should be something like 
%global alphatag alpha
Release: O.3%{alphatag}
Please read the following guideline:
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages
3. for the url: http://nanomsg.org
4. Remove the requires on nanomsg in the nanocat subpackage, RPM can compute that dependency
5. i'd rather rename the nanocat subpackage into tools or utilities, other CLI might show up in the future.'
6. the description is way too long and not helpful for fedora users. Please *trim it* and make it *useful*.
7. Did you submit your patches upstream ? If you did, please provide a link to the upstream ticket, if you didn't, please send them.
When you're done, put a comment on top of the patch with the url of the upstream ticket.
8. for the devel package, put the %doc first, ordering lines in %files section improves spec readability
9. You shouldn't specify the compression format for man pages, if we change it later, your spec will break. As i have a FPC member who backed me on that point so i'll make it a MUST ;)
10. for the Summary, i suggest: "A socket library for high-performance message-passing application" which is a more descriptive one-liner

Comment 7 Christopher Meng 2013-11-12 14:56:49 UTC
ping!

Comment 8 Zoltan Boszormenyi 2013-12-17 12:05:58 UTC
Sorry it took me this long to get back to this task.

(In reply to Haïkel Guémar from comment #6)
> I was planning to package it but i got delayed by an event as fp.o
> ambassador (where i gave a talk on nanomsg:) )
> Good point, it passed the scratch build test:
> http://koji.fedoraproject.org/koji/taskinfo?taskID=6029041
> 
> I'm a sponsor, i can take care of that part if you agree.
> For a beginning, start by doing two informal reviews (please, put me in CC)
> and link them in this ticket.

I'll do that later today or tomorrow.

> I'll also drill you on some fedora packaging guidelines, either by mail or
> irc.
> 
> 1. Group tags are not required anymore, remove them

Done.

> 2. Release tag is not handled correctly for a pre-release, it should be
> something like 
> %global alphatag alpha
> Release: O.3%{alphatag}
> Please read the following guideline:
> https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-
> Release_packages

How about the git commit number and git date?
I rebased to the current GIT version.

> 3. for the url: http://nanomsg.org

Fixed.

> 4. Remove the requires on nanomsg in the nanocat subpackage, RPM can compute
> that dependency

Done.

> 5. i'd rather rename the nanocat subpackage into tools or utilities, other
> CLI might show up in the future.'

Done.

> 6. the description is way too long and not helpful for fedora users. Please
> *trim it* and make it *useful*.

Done.

> 7. Did you submit your patches upstream ? If you did, please provide a link
> to the upstream ticket, if you didn't, please send them.
> When you're done, put a comment on top of the patch with the url of the
> upstream ticket.

I sent them long ago, but they were not appropriate, since nanomsg is also build with Visual Studio (via cmake) and it doesn't understand certain constructs. My patches were GCC-only.

I have created a new patch, sent it upstream and created the issue:

https://github.com/nanomsg/nanomsg/issues/208

My mail reporting the problem and the patch is at:

http://www.freelists.org/post/nanomsg/Warning-fixes,7

> 8. for the devel package, put the %doc first, ordering lines in %files
> section improves spec readability

Done.

> 9. You shouldn't specify the compression format for man pages, if we change
> it later, your spec will break. As i have a FPC member who backed me on that
> point so i'll make it a MUST ;)

Done.

> 10. for the Summary, i suggest: "A socket library for high-performance
> message-passing application" which is a more descriptive one-liner

Done.

Spec URL: http://boszormenyi.dyndns.biz/nanomsg.spec
SRPM URL: http://boszormenyi.dyndns.biz/nanomsg-0.2-0.3.20131217git624c483.fc20.src.rpm

Comment 9 Zoltan Boszormenyi 2013-12-17 13:47:31 UTC
My dyndns address have been cancelled. New links:

Spec URL: https://www.dropbox.com/s/h414giiyzc5scai/nanomsg.spec
SRPM URL: https://www.dropbox.com/s/mu2qrn3czenf2pn/nanomsg-0.2-0.3.20131217git624c483.fc20.src.rpm

Comment 11 Christopher Meng 2014-03-13 10:34:30 UTC
(In reply to Zoltan Boszormenyi from comment #10)
> My informal reviews:
> https://bugzilla.redhat.com/show_bug.cgi?id=970393#c7
> https://bugzilla.redhat.com/show_bug.cgi?id=1036130#c9

You need to find a sponsor initiatively:

https://admin.fedoraproject.org/accounts/group/members/packager/*/sponsor

Comment 12 Fabian Affolter 2014-05-21 21:02:14 UTC
The latest release is 0.3 [1].

[1] http://nanomsg.org/download.html

Comment 13 Japheth Cleaver 2014-06-20 17:49:16 UTC
0.4 was released on 6/10

http://download.nanomsg.org/nanomsg-0.4-beta.tar.gz

Comment 14 Japheth Cleaver 2014-07-18 02:55:28 UTC
*ping*

I'd really love to see this included in Fedora. I'd be willing to take it (and this BZ) on if there's no other activity occurring.

Comment 15 Christopher Meng 2014-07-18 03:09:53 UTC
Last comment here:

Zoltan, if you can't repond in 1 week I will close it.

Japheth, I packaged this long time ago, and postponed by this needsponsor review. If you are a packager already, feel free to submit and I will review, if not I will do this.

Fedora always lacks some packages just because of such reviews, I'm annoyed now.

Thanks.

Comment 16 Zoltan Boszormenyi 2014-07-25 10:39:15 UTC
Sorry for being non-responsive, I had to deal with a lot of other issues.
I would be happy if someone else could pick this up, I can't invest time into this.

Comment 17 Christopher Meng 2014-07-25 10:41:47 UTC
Sorry to hear about that.

I will talk with Japheth and continue this.

Comment 18 Christopher Meng 2014-07-26 03:35:48 UTC

*** This bug has been marked as a duplicate of bug 1123511 ***