Bug 1012392 - Review Request: nanomsg - nanomsg is a socket library that provides several common communication patterns
Review Request: nanomsg - nanomsg is a socket library that provides several c...
Status: CLOSED DUPLICATE of bug 1123511
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
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-09-26 07:53 EDT by Zoltan Boszormenyi
Modified: 2015-07-21 08:59 EDT (History)
5 users (show)

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


Attachments (Terms of Use)

  None (edit)
Description Zoltan Boszormenyi 2013-09-26 07:53:42 EDT
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 08:01:49 EDT
Koj
Comment 2 Zoltan Boszormenyi 2013-09-26 08:03:08 EDT
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 08:03:50 EDT
$ 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 09:08:50 EDT
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 11:38:49 EDT
(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 12:25:05 EDT
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 09:56:49 EST
ping!
Comment 8 Zoltan Boszormenyi 2013-12-17 07:05:58 EST
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 08:47:31 EST
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 06:34:30 EDT
(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 17:02:14 EDT
The latest release is 0.3 [1].

[1] http://nanomsg.org/download.html
Comment 13 Japheth Cleaver 2014-06-20 13:49:16 EDT
0.4 was released on 6/10

http://download.nanomsg.org/nanomsg-0.4-beta.tar.gz
Comment 14 Japheth Cleaver 2014-07-17 22:55:28 EDT
*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-17 23:09:53 EDT
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 06:39:15 EDT
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 06:41:47 EDT
Sorry to hear about that.

I will talk with Japheth and continue this.
Comment 18 Christopher Meng 2014-07-25 23:35:48 EDT

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

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