Bug 1055721 - Review Request: qpid-dispatch - Dispatch router for Qpid
Summary: Review Request: qpid-dispatch - Dispatch router for Qpid
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Meng
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-01-20 19:54 UTC by Darryl L. Pierce
Modified: 2015-06-22 00:08 UTC (History)
6 users (show)

Fixed In Version: qpid-dispatch-0.1-4.fc20
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-02-14 07:55:55 UTC
Type: ---
Embargoed:
i: fedora-review+
gwync: fedora-cvs+

Attachments (Terms of Use)
Don't discard user CFLAGS, append to them instead (457 bytes, patch)
2014-01-27 19:41 UTC, Ville Skyttä 		no flags Details | Diff
View All

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.
Note You need to log in before you can comment on or make changes to this bug.