dbus-cpp

Merge lp:~mandel/dbus-cpp/expose_executor into lp:dbus-cpp

  1. expose_executor
  2. Merge into trunk
Proposed by Manuel de la Peña
Status: Merged
Approved by: Thomas Voß
Approved revision: 66
Merged at revision: 62
Proposed branch: lp:~mandel/dbus-cpp/expose_executor
Merge into: lp:dbus-cpp
Diff against target: 183 lines (+38/-11)
6 files modified
debian/changelog (+6/-0)
debian/libdbus-cpp3.symbols.32bit (+2/-1)
debian/libdbus-cpp3.symbols.64bit (+2/-1)
include/core/dbus/asio/executor.h (+9/-0)
src/core/dbus/asio/executor.cpp (+10/-3)
tests/executor_test.cpp (+9/-6)
To merge this branch: bzr merge lp:~mandel/dbus-cpp/expose_executor
Reviewer Review Type Date Requested Status
Thomas Voß (community) Approve
PS Jenkins bot continuous-integration Approve
Łukasz Zemczak packaging Approve
Review via email: mp+222200@code.launchpad.net

Commit message

Allow to pass the io_service to the executor so that the deveoper has more freedom with the asio code.

To post a comment you must log in.
lp:~mandel/dbus-cpp/expose_executor updated
59. By Manuel de la Peña

Merge with trunk.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:59
http://jenkins.qa.ubuntu.com/job/dbus-cpp-ci/166/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-amd64-ci/6/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-armhf-ci/6/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-i386-ci/6/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/dbus-cpp-ci/166/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Thomas Voß (thomas-voss) :
review: Needs Fixing
lp:~mandel/dbus-cpp/expose_executor updated
60. By Manuel de la Peña

Updaed symbol files.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:60
http://jenkins.qa.ubuntu.com/job/dbus-cpp-ci/167/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-amd64-ci/7/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-armhf-ci/7/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-i386-ci/7/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/dbus-cpp-ci/167/rebuild

review: Needs Fixing (continuous-integration)
lp:~mandel/dbus-cpp/expose_executor updated
61. By Manuel de la Peña

Remove duplicated symbol.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:61
http://jenkins.qa.ubuntu.com/job/dbus-cpp-ci/168/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-amd64-ci/8
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-armhf-ci/8/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-i386-ci/8/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/dbus-cpp-ci/168/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

So, I found the reason for the symbols failure. It seems that dpkg-gensymbols (and related) is just stupid. The problem is in this line of debian/libdbus-cpp3.symbols.32bit:

+ (c++)"core::dbus::asio::make_executor(std::shared_ptr<core::dbus::Bus> const&, boost::asio::io_service &)@Base" 0replaceme

It should be:

+ (c++)"core::dbus::asio::make_executor(std::shared_ptr<core::dbus::Bus> const&, boost::asio::io_service&)@Base" 0replaceme

Please notice that there *cannot* be a whitespace between the typename and the pointer/reference sign. This actually causes a SYMBOL MISMATCH. So, dpkg-gensymbols is not whitespace-tolerant...

review: Needs Fixing
lp:~mandel/dbus-cpp/expose_executor updated
62. By Manuel de la Peña

Increas version number due to the new symbols.

63. By Manuel de la Peña

Remove extra space.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:63
http://jenkins.qa.ubuntu.com/job/dbus-cpp-ci/169/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-amd64-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-armhf-ci/9
        deb: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-armhf-ci/9/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-i386-ci/9

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/dbus-cpp-ci/169/rebuild

review: Approve (continuous-integration)
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

+1 now! dpkg... we are so disappoint!

review: Approve (packaging)
lp:~mandel/dbus-cpp/expose_executor updated
64. By Manuel de la Peña

Do not increase the version by 1.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:64
http://jenkins.qa.ubuntu.com/job/dbus-cpp-ci/170/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-amd64-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-armhf-ci/10
        deb: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-armhf-ci/10/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-i386-ci/10

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/dbus-cpp-ci/170/rebuild

review: Approve (continuous-integration)
lp:~mandel/dbus-cpp/expose_executor updated
65. By Manuel de la Peña

Merged with turnk.

66. By Manuel de la Peña

Do not use a pointer but a static ref. Update the tests to work so that a new io_service is used per TEST_F

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:66
http://jenkins.qa.ubuntu.com/job/dbus-cpp-ci/179/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-amd64-ci/19
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-armhf-ci/19
        deb: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-armhf-ci/19/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-cpp-utopic-i386-ci/19

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/dbus-cpp-ci/179/rebuild

review: Approve (continuous-integration)
Revision history for this message
Thomas Voß (thomas-voss) wrote :

LGTM.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2014-06-12 11:08:49 +0000
3+++ debian/changelog 2014-06-23 10:20:17 +0000
4@@ -1,3 +1,9 @@
5+dbus-cpp (3.1.0-0ubuntu1) UNRELEASED; urgency=medium
6+
7+ * Provide a new make_executor method to allow to pass the io_service.
8+
9+ -- Manuel de la Pena <manuel.delapena@canonical.com> Thu, 05 Jun 2014 13:53:45 +0200
10+
11 dbus-cpp (3.0.0+14.10.20140612-0ubuntu1) utopic; urgency=low
12
13 [ Manuel de la Peña ]
14
15=== modified file 'debian/libdbus-cpp3.symbols.32bit'
16--- debian/libdbus-cpp3.symbols.32bit 2014-05-05 08:38:36 +0000
17+++ debian/libdbus-cpp3.symbols.32bit 2014-06-23 10:20:17 +0000
18@@ -1,4 +1,5 @@
19 (c++)"core::dbus::asio::make_executor(std::shared_ptr<core::dbus::Bus> const&)@Base" 2.0.0+14.04.20140310
20+ (c++)"core::dbus::asio::make_executor(std::shared_ptr<core::dbus::Bus> const&, boost::asio::io_service&)@Base" 0replaceme
21 (c++)"core::dbus::Bus::access_signal_router()@Base" 2.0.0+14.04.20140310
22 (c++)"core::dbus::Bus::add_match(core::dbus::MatchRule const&)@Base" 2.0.0+14.04.20140310
23 (c++)"core::dbus::Bus::~Bus()@Base" 2.0.0+14.04.20140310
24@@ -200,4 +201,4 @@
25 (c++)"vtable for core::dbus::Fixture@Base" 2.0.0+14.04.20140310
26 (c++)"vtable for core::dbus::PendingCall@Base" 2.0.0+14.04.20140310
27 (c++)"vtable for core::dbus::types::ObjectPath::Errors::InvalidObjectPathStringRepresentation@Base" 2.0.0+14.04.20140310
28-#include "libdbus-cpp3.symbols.coverage"
29\ No newline at end of file
30+#include "libdbus-cpp3.symbols.coverage"
31
32=== modified file 'debian/libdbus-cpp3.symbols.64bit'
33--- debian/libdbus-cpp3.symbols.64bit 2014-05-05 08:38:36 +0000
34+++ debian/libdbus-cpp3.symbols.64bit 2014-06-23 10:20:17 +0000
35@@ -1,4 +1,5 @@
36 (c++)"core::dbus::asio::make_executor(std::shared_ptr<core::dbus::Bus> const&)@Base" 2.0.0+14.04.20140310
37+ (c++)"core::dbus::asio::make_executor(std::shared_ptr<core::dbus::Bus> const&, boost::asio::io_service&)@Base" 0replaceme
38 (c++)"core::dbus::Bus::access_signal_router()@Base" 2.0.0+14.04.20140310
39 (c++)"core::dbus::Bus::add_match(core::dbus::MatchRule const&)@Base" 2.0.0+14.04.20140310
40 (c++)"core::dbus::Bus::~Bus()@Base" 2.0.0+14.04.20140310
41@@ -200,4 +201,4 @@
42 (c++)"vtable for core::dbus::Fixture@Base" 2.0.0+14.04.20140310
43 (c++)"vtable for core::dbus::PendingCall@Base" 2.0.0+14.04.20140310
44 (c++)"vtable for core::dbus::types::ObjectPath::Errors::InvalidObjectPathStringRepresentation@Base" 2.0.0+14.04.20140310
45-#include "libdbus-cpp3.symbols.coverage"
46\ No newline at end of file
47+#include "libdbus-cpp3.symbols.coverage"
48
49=== modified file 'include/core/dbus/asio/executor.h'
50--- include/core/dbus/asio/executor.h 2013-11-27 18:57:42 +0000
51+++ include/core/dbus/asio/executor.h 2014-06-23 10:20:17 +0000
52@@ -22,6 +22,14 @@
53 #include <core/dbus/executor.h>
54 #include <core/dbus/visibility.h>
55
56+namespace boost
57+{
58+namespace asio
59+{
60+class io_service;
61+}
62+}
63+
64 namespace core
65 {
66 namespace dbus
67@@ -29,6 +37,7 @@
68 namespace asio
69 {
70 ORG_FREEDESKTOP_DBUS_DLL_PUBLIC Executor::Ptr make_executor(const Bus::Ptr& bus);
71+ORG_FREEDESKTOP_DBUS_DLL_PUBLIC Executor::Ptr make_executor(const Bus::Ptr& bus, boost::asio::io_service& io);
72 }
73 }
74 }
75
76=== modified file 'src/core/dbus/asio/executor.cpp'
77--- src/core/dbus/asio/executor.cpp 2014-03-05 06:58:15 +0000
78+++ src/core/dbus/asio/executor.cpp 2014-06-23 10:20:17 +0000
79@@ -331,7 +331,8 @@
80 }
81
82 public:
83- explicit Executor(const Bus::Ptr& bus) : bus(bus), work(io_service)
84+
85+ Executor(const Bus::Ptr& bus, boost::asio::io_service& io) : bus(bus), io_service(io), work(io_service)
86 {
87 if (!bus)
88 throw std::runtime_error("Precondition violated, cannot construct executor for null bus.");
89@@ -378,13 +379,19 @@
90
91 private:
92 Bus::Ptr bus;
93- boost::asio::io_service io_service;
94+ boost::asio::io_service& io_service;
95 boost::asio::io_service::work work;
96 };
97
98 ORG_FREEDESKTOP_DBUS_DLL_PUBLIC Executor::Ptr make_executor(const Bus::Ptr& bus)
99 {
100- return std::make_shared<core::dbus::asio::Executor>(bus);
101+ static boost::asio::io_service io;
102+ return std::make_shared<core::dbus::asio::Executor>(bus, io);
103+}
104+
105+ORG_FREEDESKTOP_DBUS_DLL_PUBLIC Executor::Ptr make_executor(const Bus::Ptr& bus, boost::asio::io_service& io)
106+{
107+ return std::make_shared<core::dbus::asio::Executor>(bus, io);
108 }
109
110 }
111
112=== modified file 'tests/executor_test.cpp'
113--- tests/executor_test.cpp 2014-03-05 06:58:15 +0000
114+++ tests/executor_test.cpp 2014-06-23 10:20:17 +0000
115@@ -30,6 +30,7 @@
116 #include <core/testing/cross_process_sync.h>
117 #include <core/testing/fork_and_run.h>
118
119+#include <boost/asio.hpp>
120 #include <gtest/gtest.h>
121
122 namespace dbus = core::dbus;
123@@ -54,6 +55,8 @@
124
125 struct Executor : public core::dbus::testing::Fixture
126 {
127+ protected:
128+ boost::asio::io_service io_service;
129 };
130
131 auto session_bus_config_file =
132@@ -67,13 +70,13 @@
133
134 TEST_F(Executor, ThrowsOnConstructionFromNullBus)
135 {
136- EXPECT_ANY_THROW(core::dbus::asio::make_executor(core::dbus::Bus::Ptr{}));
137+ EXPECT_ANY_THROW(core::dbus::asio::make_executor(core::dbus::Bus::Ptr{}, io_service));
138 }
139
140 TEST_F(Executor, DoesNotThrowForExistingBus)
141 {
142 auto bus = session_bus();
143- EXPECT_NO_THROW(bus->install_executor(core::dbus::asio::make_executor(bus)));
144+ EXPECT_NO_THROW(bus->install_executor(core::dbus::asio::make_executor(bus, io_service)));
145 }
146
147 TEST_F(Executor, ABusRunByAnExecutorReceivesSignals)
148@@ -85,7 +88,7 @@
149 {
150 core::testing::SigTermCatcher sc;
151 auto bus = session_bus();
152- bus->install_executor(dbus::asio::make_executor(bus));
153+ bus->install_executor(dbus::asio::make_executor(bus, io_service));
154 auto service = dbus::Service::add_service<test::Service>(bus);
155 auto skeleton = service->add_object_for_path(dbus::types::ObjectPath("/this/is/unlikely/to/exist/Service"));
156 skeleton->install_method_handler<test::Service::Method>(
157@@ -114,7 +117,7 @@
158 auto client = [this, expected_value, &cross_process_sync]() -> core::posix::exit::Status
159 {
160 auto bus = session_bus();
161- bus->install_executor(dbus::asio::make_executor(bus));
162+ bus->install_executor(dbus::asio::make_executor(bus, io_service));
163 std::thread t{[bus](){bus->run();}};
164
165 EXPECT_EQ(std::uint32_t(1), cross_process_sync.wait_for_signal_ready_for(std::chrono::milliseconds{500}));
166@@ -153,7 +156,7 @@
167 ChaosMonkey chaos_monkey;
168 core::testing::SigTermCatcher sc;
169 auto bus = session_bus();
170- bus->install_executor(dbus::asio::make_executor(bus));
171+ bus->install_executor(dbus::asio::make_executor(bus, io_service));
172 auto service = dbus::Service::add_service<test::Service>(bus);
173 auto skeleton = service->add_object_for_path(dbus::types::ObjectPath("/this/is/unlikely/to/exist/Service"));
174 skeleton->install_method_handler<test::Service::Method>(
175@@ -190,7 +193,7 @@
176 auto client = [this, expected_value, &cross_process_sync]() -> core::posix::exit::Status
177 {
178 auto bus = session_bus();
179- bus->install_executor(dbus::asio::make_executor(bus));
180+ bus->install_executor(dbus::asio::make_executor(bus, io_service));
181 std::thread t{[bus](){bus->run();}};
182
183 // If you encounter failures in the client, uncomment the following two lines

Subscribers

People subscribed via source and target branches