Bug 1055721

Summary: Review Request: qpid-dispatch - Dispatch router for Qpid
Product: [Fedora] Fedora Reporter: Darryl L. Pierce <dpierce>
Component: Package ReviewAssignee: Christopher Meng <i>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: i, knakayam, nakayamakenjiro, package-review, tross, ville.skytta
Target Milestone: ---Flags: i: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: qpid-dispatch-0.1-4.fc20 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-02-14 07:55:55 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:
Attachments:
Description Flags
Don't discard user CFLAGS, append to them instead none

Description Darryl L. Pierce 2014-01-20 19:54:45 UTC
Spec URL: http://mcpierce.fedorapeople.org/rpms/qpid-dispatch.spec
SRPM URL: http://mcpierce.fedorapeople.org/rpms/qpid-dispatch-0.1-1.fc20.src.rpm
Description: Dispatch router for Qpid
Fedora Account System Username: mcpierce

Comment 1 Christopher Meng 2014-01-21 01:41:19 UTC
Looks like these sysmted macros are not used:

https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd

One request, can you write those %files and %p* section below %prep~%check? Snafu...

Comment 2 Darryl L. Pierce 2014-01-22 15:58:32 UTC
(In reply to Christopher Meng from comment #1)
> Looks like these sysmted macros are not used:
> 
> https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd

Added them, and also will be adding them to other packages I maintain.

> One request, can you write those %files and %p* section below %prep~%check?
> Snafu...

Done.

Updated spec:  http://mcpierce.fedorapeople.org/rpms/qpid-dispatch.spec
Updated SRPM:  http://mcpierce.fedorapeople.org/rpms/qpid-dispatch-0.1-1.fc20.1.src.rpm
Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6440136

Comment 3 Christopher Meng 2014-01-23 05:36:30 UTC
Still a mess...Would you like to move all subpackage above %prep?

1. BuildRequires: python-devel

-->

BuildRequires: python2-devel

If you want to push it to EPEL, better add conditional lines but not simply revert back this change.

2. Remove unneeded: 

%check


%clean

3. Requires(post): systemd-units
Requires(preun): systemd-units
Requires(postun): systemd-units

PLease use modern style:

https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd

4. doc subpackage should be noarch.

5. Rpmlint
-------
Checking: libqpid-dispatch-0.1-1.fc21.1.i686.rpm
          libqpid-dispatch-devel-0.1-1.fc21.1.i686.rpm
          qpid-dispatch-router-0.1-1.fc21.1.i686.rpm
          qpid-dispatch-router-docs-0.1-1.fc21.1.i686.rpm
          qpid-dispatch-tools-0.1-1.fc21.1.i686.rpm
          qpid-dispatch-0.1-1.fc21.1.src.rpm
libqpid-dispatch.i686: W: shared-lib-calls-exit /usr/lib/libqpid-dispatch.so.0.1 exit
libqpid-dispatch.i686: W: no-documentation
libqpid-dispatch.i686: E: non-executable-script /usr/lib/qpid-dispatch/python/qpid_dispatch_internal/tools/display.py 0644L /usr/bin/env
libqpid-dispatch-devel.i686: W: no-documentation
qpid-dispatch-router.i686: W: invalid-url URL: http://qpid.apache.org/ timed out
qpid-dispatch-router.i686: W: only-non-binary-in-usr-lib
qpid-dispatch-tools.i686: W: no-manual-page-for-binary qdtest
6 packages and 0 specfiles checked; 1 errors, 6 warnings.

6. Are you the upstream? If so please patch them:


*No copyright* Apache (v2.0)
----------------------------
qpid-dispatch-0.1/bin/test.sh
qpid-dispatch-0.1/config.sh
qpid-dispatch-0.1/include/qpid/dispatch.h
qpid-dispatch-0.1/include/qpid/dispatch/agent.h
qpid-dispatch-0.1/include/qpid/dispatch/alloc.h
qpid-dispatch-0.1/include/qpid/dispatch/amqp.h
qpid-dispatch-0.1/include/qpid/dispatch/bitmask.h
qpid-dispatch-0.1/include/qpid/dispatch/buffer.h
qpid-dispatch-0.1/include/qpid/dispatch/compose.h
qpid-dispatch-0.1/include/qpid/dispatch/config.h
qpid-dispatch-0.1/include/qpid/dispatch/container.h
qpid-dispatch-0.1/include/qpid/dispatch/ctools.h
qpid-dispatch-0.1/include/qpid/dispatch/dispatch.h
qpid-dispatch-0.1/include/qpid/dispatch/error.h
qpid-dispatch-0.1/include/qpid/dispatch/hash.h
qpid-dispatch-0.1/include/qpid/dispatch/iovec.h
qpid-dispatch-0.1/include/qpid/dispatch/iterator.h
qpid-dispatch-0.1/include/qpid/dispatch/log.h
qpid-dispatch-0.1/include/qpid/dispatch/message.h
qpid-dispatch-0.1/include/qpid/dispatch/parse.h
qpid-dispatch-0.1/include/qpid/dispatch/python_embedded.h
qpid-dispatch-0.1/include/qpid/dispatch/router.h
qpid-dispatch-0.1/include/qpid/dispatch/server.h
qpid-dispatch-0.1/include/qpid/dispatch/threading.h
qpid-dispatch-0.1/include/qpid/dispatch/timer.h
qpid-dispatch-0.1/include/qpid/dispatch/user_fd.h
qpid-dispatch-0.1/python/qpid_dispatch_internal/config/__init__.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/config/parser.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/config/schema.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/router/__init__.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/router/configuration.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/router/data.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/router/engine.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/router/link.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/router/mobile.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/router/neighbor.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/router/node.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/router/path.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/router/routing.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/stubs/__init__.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/stubs/ioadapter.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/stubs/logadapter.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/tools/__init__.py
qpid-dispatch-0.1/python/qpid_dispatch_internal/tools/display.py
qpid-dispatch-0.1/router/src/main.c
qpid-dispatch-0.1/src/agent.c
qpid-dispatch-0.1/src/alloc.c
qpid-dispatch-0.1/src/alloc_private.h
qpid-dispatch-0.1/src/amqp.c
qpid-dispatch-0.1/src/bitmask.c
qpid-dispatch-0.1/src/buffer.c
qpid-dispatch-0.1/src/compose.c
qpid-dispatch-0.1/src/compose_private.h
qpid-dispatch-0.1/src/config.c
qpid-dispatch-0.1/src/config_private.h
qpid-dispatch-0.1/src/container.c
qpid-dispatch-0.1/src/dispatch.c
qpid-dispatch-0.1/src/dispatch_private.h
qpid-dispatch-0.1/src/hash.c
qpid-dispatch-0.1/src/iovec.c
qpid-dispatch-0.1/src/iterator.c
qpid-dispatch-0.1/src/log.c
qpid-dispatch-0.1/src/log_private.h
qpid-dispatch-0.1/src/message.c
qpid-dispatch-0.1/src/message_private.h
qpid-dispatch-0.1/src/parse.c
qpid-dispatch-0.1/src/posix/threading.c
qpid-dispatch-0.1/src/python_embedded.c
qpid-dispatch-0.1/src/router_agent.c
qpid-dispatch-0.1/src/router_node.c
qpid-dispatch-0.1/src/router_private.h
qpid-dispatch-0.1/src/router_pynode.c
qpid-dispatch-0.1/src/server.c
qpid-dispatch-0.1/src/server_private.h
qpid-dispatch-0.1/src/timer.c
qpid-dispatch-0.1/src/timer_private.h
qpid-dispatch-0.1/src/work_queue.c
qpid-dispatch-0.1/src/work_queue.h
qpid-dispatch-0.1/tests/alloc_test.c
qpid-dispatch-0.1/tests/compose_test.c
qpid-dispatch-0.1/tests/field_test.c
qpid-dispatch-0.1/tests/message_test.c
qpid-dispatch-0.1/tests/parse_test.c
qpid-dispatch-0.1/tests/router_engine_test.py
qpid-dispatch-0.1/tests/run_unit_tests.c
qpid-dispatch-0.1/tests/run_unit_tests_size.c
qpid-dispatch-0.1/tests/server_test.c
qpid-dispatch-0.1/tests/system_tests_one_router.py
qpid-dispatch-0.1/tests/test_case.h
qpid-dispatch-0.1/tests/timer_test.c
qpid-dispatch-0.1/tests/tool_test.c

Comment 4 Darryl L. Pierce 2014-01-23 16:01:06 UTC
(In reply to Christopher Meng from comment #3)
> Still a mess...Would you like to move all subpackage above %prep?

I moved all the subpackages up above prep/build/install and put headers in. I kind of like the way it looks, and will look at more examples of packages to see what's the common format to make it easier to maintain.
 
> 1. BuildRequires: python-devel
> 
> -->
> 
> BuildRequires: python2-devel
> 
> If you want to push it to EPEL, better add conditional lines but not simply
> revert back this change.

I'm not a fan of conditions in the specfile since I don't merge across branches, only cherry-picking changes. I'll update the specfile for each branch (unless there's a guideline that requires conditionals).

> 2. Remove unneeded: 
> 
> %check
> 
> 
> %clean

Done

> 3. Requires(post): systemd-units
> Requires(preun): systemd-units
> Requires(postun): systemd-units
> 
> PLease use modern style:
> 
> https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd

Done

> 4. doc subpackage should be noarch.

Done

> 5. Rpmlint
> -------
> Checking: libqpid-dispatch-0.1-1.fc21.1.i686.rpm
>           libqpid-dispatch-devel-0.1-1.fc21.1.i686.rpm
>           qpid-dispatch-router-0.1-1.fc21.1.i686.rpm
>           qpid-dispatch-router-docs-0.1-1.fc21.1.i686.rpm
>           qpid-dispatch-tools-0.1-1.fc21.1.i686.rpm
>           qpid-dispatch-0.1-1.fc21.1.src.rpm
> libqpid-dispatch.i686: W: shared-lib-calls-exit
> /usr/lib/libqpid-dispatch.so.0.1 exit
> libqpid-dispatch.i686: W: no-documentation
> libqpid-dispatch.i686: E: non-executable-script
> /usr/lib/qpid-dispatch/python/qpid_dispatch_internal/tools/display.py 0644L
> /usr/bin/env
> libqpid-dispatch-devel.i686: W: no-documentation
> qpid-dispatch-router.i686: W: invalid-url URL: http://qpid.apache.org/ timed
> out
> qpid-dispatch-router.i686: W: only-non-binary-in-usr-lib
> qpid-dispatch-tools.i686: W: no-manual-page-for-binary qdtest
> 6 packages and 0 specfiles checked; 1 errors, 6 warnings.
> 
> 6. Are you the upstream? If so please patch them:

I'm on the team, yes.

> *No copyright* Apache (v2.0)
> ----------------------------

The tool used to check the files is wrong. They all have copyright headers in them.

> qpid-dispatch-0.1/bin/test.sh
> qpid-dispatch-0.1/config.sh
> qpid-dispatch-0.1/include/qpid/dispatch.h
> qpid-dispatch-0.1/include/qpid/dispatch/agent.h
> qpid-dispatch-0.1/include/qpid/dispatch/alloc.h
> qpid-dispatch-0.1/include/qpid/dispatch/amqp.h
> qpid-dispatch-0.1/include/qpid/dispatch/bitmask.h
> qpid-dispatch-0.1/include/qpid/dispatch/buffer.h
> qpid-dispatch-0.1/include/qpid/dispatch/compose.h
> qpid-dispatch-0.1/include/qpid/dispatch/config.h
> qpid-dispatch-0.1/include/qpid/dispatch/container.h
> qpid-dispatch-0.1/include/qpid/dispatch/ctools.h
> qpid-dispatch-0.1/include/qpid/dispatch/dispatch.h
> qpid-dispatch-0.1/include/qpid/dispatch/error.h
> qpid-dispatch-0.1/include/qpid/dispatch/hash.h
> qpid-dispatch-0.1/include/qpid/dispatch/iovec.h
> qpid-dispatch-0.1/include/qpid/dispatch/iterator.h
> qpid-dispatch-0.1/include/qpid/dispatch/log.h
> qpid-dispatch-0.1/include/qpid/dispatch/message.h
> qpid-dispatch-0.1/include/qpid/dispatch/parse.h
> qpid-dispatch-0.1/include/qpid/dispatch/python_embedded.h
> qpid-dispatch-0.1/include/qpid/dispatch/router.h
> qpid-dispatch-0.1/include/qpid/dispatch/server.h
> qpid-dispatch-0.1/include/qpid/dispatch/threading.h
> qpid-dispatch-0.1/include/qpid/dispatch/timer.h
> qpid-dispatch-0.1/include/qpid/dispatch/user_fd.h
> qpid-dispatch-0.1/python/qpid_dispatch_internal/config/__init__.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/config/parser.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/config/schema.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/router/__init__.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/router/configuration.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/router/data.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/router/engine.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/router/link.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/router/mobile.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/router/neighbor.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/router/node.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/router/path.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/router/routing.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/stubs/__init__.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/stubs/ioadapter.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/stubs/logadapter.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/tools/__init__.py
> qpid-dispatch-0.1/python/qpid_dispatch_internal/tools/display.py
> qpid-dispatch-0.1/router/src/main.c
> qpid-dispatch-0.1/src/agent.c
> qpid-dispatch-0.1/src/alloc.c
> qpid-dispatch-0.1/src/alloc_private.h
> qpid-dispatch-0.1/src/amqp.c
> qpid-dispatch-0.1/src/bitmask.c
> qpid-dispatch-0.1/src/buffer.c
> qpid-dispatch-0.1/src/compose.c
> qpid-dispatch-0.1/src/compose_private.h
> qpid-dispatch-0.1/src/config.c
> qpid-dispatch-0.1/src/config_private.h
> qpid-dispatch-0.1/src/container.c
> qpid-dispatch-0.1/src/dispatch.c
> qpid-dispatch-0.1/src/dispatch_private.h
> qpid-dispatch-0.1/src/hash.c
> qpid-dispatch-0.1/src/iovec.c
> qpid-dispatch-0.1/src/iterator.c
> qpid-dispatch-0.1/src/log.c
> qpid-dispatch-0.1/src/log_private.h
> qpid-dispatch-0.1/src/message.c
> qpid-dispatch-0.1/src/message_private.h
> qpid-dispatch-0.1/src/parse.c
> qpid-dispatch-0.1/src/posix/threading.c
> qpid-dispatch-0.1/src/python_embedded.c
> qpid-dispatch-0.1/src/router_agent.c
> qpid-dispatch-0.1/src/router_node.c
> qpid-dispatch-0.1/src/router_private.h
> qpid-dispatch-0.1/src/router_pynode.c
> qpid-dispatch-0.1/src/server.c
> qpid-dispatch-0.1/src/server_private.h
> qpid-dispatch-0.1/src/timer.c
> qpid-dispatch-0.1/src/timer_private.h
> qpid-dispatch-0.1/src/work_queue.c
> qpid-dispatch-0.1/src/work_queue.h
> qpid-dispatch-0.1/tests/alloc_test.c
> qpid-dispatch-0.1/tests/compose_test.c
> qpid-dispatch-0.1/tests/field_test.c
> qpid-dispatch-0.1/tests/message_test.c
> qpid-dispatch-0.1/tests/parse_test.c
> qpid-dispatch-0.1/tests/router_engine_test.py
> qpid-dispatch-0.1/tests/run_unit_tests.c
> qpid-dispatch-0.1/tests/run_unit_tests_size.c
> qpid-dispatch-0.1/tests/server_test.c
> qpid-dispatch-0.1/tests/system_tests_one_router.py
> qpid-dispatch-0.1/tests/test_case.h
> qpid-dispatch-0.1/tests/timer_test.c
> qpid-dispatch-0.1/tests/tool_test.c


Updated spec:  http://mcpierce.fedorapeople.org/rpms/qpid-dispatch.spec
Updated SRPM:  http://mcpierce.fedorapeople.org/rpms/qpid-dispatch-0.1-1.fc20.2.src.rpm
Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6445071

Comment 5 Christopher Meng 2014-01-24 02:11:13 UTC
PACKAGE APPROVED!

Comment 6 Darryl L. Pierce 2014-01-24 13:24:55 UTC
(In reply to Christopher Meng from comment #5)
> PACKAGE APPROVED!

Excellent! Thank you.

New Package SCM Request
=======================
Package Name: qpid-dispatch
Short Description: Dispatch router for AMQP 1.0.
Owners: mcpierce
Branches: f19 f20 el5 el6 el7
InitialCC:

Comment 7 Gwyn Ciesla 2014-01-24 13:38:46 UTC
Git done (by process-git-requests).

Comment 8 Fedora Update System 2014-01-24 16:01:03 UTC
qpid-dispatch-0.1-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/qpid-dispatch-0.1-2.fc20

Comment 9 Fedora Update System 2014-01-24 16:55:33 UTC
qpid-dispatch-0.1-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/qpid-dispatch-0.1-2.fc19

Comment 10 Fedora Update System 2014-01-25 02:26:42 UTC
qpid-dispatch-0.1-2.fc20 has been pushed to the Fedora 20 testing repository.

Comment 11 Ville Skyttä 2014-01-27 19:41:32 UTC
Created attachment 856247 [details]
Don't discard user CFLAGS, append to them instead

Careful, reviewer: package is not built with $RPM_OPT_FLAGS. Fix attached, please apply upstream as well.

Honoring proper Fedora CFLAGS causes a build failure due to -Werror being used so additional fixing will be required:

[...]/server_test.c: In function 'ufd_handler':
[...]/server_test.c:99:18: error: ignoring return value of 'write', declared with attribute warn_unused_result [-Werror=unused-result]
             write(fd[1], "X", 1);

Comment 12 Darryl L. Pierce 2014-01-27 19:48:48 UTC
(In reply to Ville Skyttä from comment #11)
> Created attachment 856247 [details]
> Don't discard user CFLAGS, append to them instead
> 
> Careful, reviewer: package is not built with $RPM_OPT_FLAGS. Fix attached,
> please apply upstream as well.
> 
> Honoring proper Fedora CFLAGS causes a build failure due to -Werror being
> used so additional fixing will be required:
> 
> [...]/server_test.c: In function 'ufd_handler':
> [...]/server_test.c:99:18: error: ignoring return value of 'write', declared
> with attribute warn_unused_result [-Werror=unused-result]
>              write(fd[1], "X", 1);

I'm going to create a new BZ for this and associate it with an upstream JIRA.

Comment 13 Fedora Update System 2014-01-30 20:27:51 UTC
qpid-dispatch-0.1-3.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/qpid-dispatch-0.1-3.fc20

Comment 14 Fedora Update System 2014-01-30 20:37:27 UTC
qpid-dispatch-0.1-3.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/qpid-dispatch-0.1-3.fc19

Comment 15 Fedora Update System 2014-02-05 15:26:16 UTC
qpid-dispatch-0.1-4.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/qpid-dispatch-0.1-4.fc20

Comment 16 Fedora Update System 2014-02-05 15:41:42 UTC
qpid-dispatch-0.1-4.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/qpid-dispatch-0.1-4.fc19

Comment 17 Fedora Update System 2014-02-14 07:55:55 UTC
qpid-dispatch-0.1-4.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 18 Fedora Update System 2014-02-14 07:57:47 UTC
qpid-dispatch-0.1-4.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.