Merge lp:~michihenning/unity-scopes-api/miscellaneous-fixes into lp:unity-scopes-api

[Michi Henning]

Bunch of miscellaneous fixes:

- Added warning for non-virtual destructors.
- Disabled warning with #pragma for the missing virtual destructor in AbstractScopeBase.
- Tidied up the way compiler flags are set in CMakeLists.txt for clarity.
- Added missing dependencies to demos, so we can't run with an out-of-date registry or scoperunner.
- Fixed a number of warnings when building the developer doc.
- Minor doc fixes.
- Minor stylistic code fixes.
- Added missing tmp_dir_initialized_ initializer to ScopeBaseImpl.
- Removed redundant lock in QueryCtrlImpl constructor.

These are fixes that I picked up from the branches that went on hold because of the ABI break, but that contained other unrelated fixes.

Main change is that we now get a warning for non-virtual base class constructors. I've disabled the warning we would normally get from the missing AbstractScopeBase destructor by adding a #pragma, so we still get a clean build. The other fixes are all minor, except for the missing tmp_dir_initialized_ initializer
.
Bunch of miscellaneous fixes:

- Added warning for non-virtual destructors.
- Disabled warning with #pragma for the missing virtual destructor in AbstractScopeBase.
- Tidied up the way compiler flags are set in CMakeLists.txt for clarity.
- Added missing dependencies to demos, so we can't run with an out-of-date registry or scoperunner.
- Fixed a number of warnings when building the developer doc.
- Minor doc fixes.
- Minor stylistic code fixes.
- Added missing tmp_dir_initialized_ initializer to ScopeBaseImpl.
- Removed redundant lock in QueryCtrlImpl constructor.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:276
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-ci/508/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-vivid-amd64-ci/33/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-vivid-armhf-ci/33/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-vivid-i386-ci/33/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-ci/508/rebuild

review: Needs Fixing (continuous-integration)

[lp:~michihenning/unity-scopes-api/miscellaneous-fixes]

277. By Michi Henning on 2015-01-14

Merged dependent branch.

278. By Michi Henning on 2015-01-15

Removed pragmas to suppress warnings for non-virtual base destructor because
those warnings will disappear once Marcus's child scopes change is merged.

279. By Michi Henning on 2015-01-20

Merged trunk.

Unmerged revisions

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2014-12-09 15:26:12 +0000
+++ CMakeLists.txt	2015-01-14 02:25:50 +0000
@@ -147,15 +147,18 @@
 # setting of LINK_FLAGS below).
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility=default")
 
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -pedantic -Wall -Wextra")
+set(C_AND_CXX_WARNINGS "-pedantic -Wall -Wextra")
 
 # Some additional warnings not included by the general flags set above.
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wcast-align -Wcast-qual -Wctor-dtor-privacy -Wformat -Wold-style-cast -Wredundant-decls -Wswitch-default")
-
-# TODO: Once AbstractScopeBase gets its virtual destructor, turn on the line below (bug #1360266).
-#set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wnon-virtual-dtor")
-
-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pedantic -Wall -Wextra") # Can't use -std=c++11 with C compiler
+set(EXTRA_C_WARNINGS "-Wcast-align -Wcast-qual -Wformat -Wredundant-decls -Wswitch-default")
+set(EXTRA_CXX_WARNINGS "-Wnon-virtual-dtor -Wctor-dtor-privacy -Wold-style-cast")
+
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${C_AND_CXX_WARNINGS}")
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${EXTRA_C_WARNINGS}")
+
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 ${C_AND_CXX_WARNINGS}")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_C_WARNINGS}")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CXX_WARNINGS}")
 
 # -fno-permissive causes warnings with clang, so we only enable it for gcc
 if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")

=== modified file 'HACKING'
--- HACKING	2014-10-09 02:52:15 +0000
+++ HACKING	2015-01-14 02:25:50 +0000
@@ -220,7 +220,7 @@
 Autopkg test suite
 ------------------
 
-Scopes comes with an autopkg smoke test.
+Scopes come with an autopkg smoke test.
 
 http://packaging.ubuntu.com/html/auto-pkg-test.html
 

=== modified file 'demo/CMakeLists.txt'
--- demo/CMakeLists.txt	2014-04-29 03:14:49 +0000
+++ demo/CMakeLists.txt	2015-01-14 02:25:50 +0000
@@ -9,7 +9,9 @@
 
 add_executable(scopes-client client.cpp)
 # Add_dependencies should be used sparingly. In this case we need the global
-# header to be generated before we start building the client binary.
+# header to be generated before we start building the client binary, and
+# we need an up-to-date registry to run it.
 add_dependencies(scopes-client globalheader)
+add_dependencies(scopes-client scoperegistry)
 target_link_libraries(scopes-client ${UNITY_SCOPES_LIB} ${OTHER_LIBS})
 install(TARGETS scopes-client DESTINATION bin)

=== modified file 'demo/scopes/scope-A/CMakeLists.txt'
--- demo/scopes/scope-A/CMakeLists.txt	2014-06-17 00:53:21 +0000
+++ demo/scopes/scope-A/CMakeLists.txt	2015-01-14 02:25:50 +0000
@@ -4,7 +4,9 @@
 add_library(scope-A MODULE scope-A.cpp)
 # Add_dependencies should be used sparingly. In this case we need the global
 # header to be generated before we start building the client binary.
+# We also need the scoperegistry and scoperunner to be up-to-date.
 add_dependencies(scope-A globalheader)
+add_dependencies(scope-A scoperegistry scoperunner)
 set_target_properties(scope-A PROPERTIES
                       LINK_FLAGS "${ldflags} -Wl,--version-script,${symbol_map}")
 set_target_properties(scope-A PROPERTIES LINK_DEPENDS ${symbol_map})

=== modified file 'demo/scopes/scope-B/CMakeLists.txt'
--- demo/scopes/scope-B/CMakeLists.txt	2013-11-19 02:51:46 +0000
+++ demo/scopes/scope-B/CMakeLists.txt	2015-01-14 02:25:50 +0000
@@ -1,3 +1,4 @@
 add_library(scope-B MODULE scope-B.cpp)
+add_dependencies(scope-B scoperegistry scoperunner)
 
 configure_file(scope-B.ini.in scope-B.ini)

=== modified file 'demo/scopes/scope-C/CMakeLists.txt'
--- demo/scopes/scope-C/CMakeLists.txt	2013-11-19 02:51:46 +0000
+++ demo/scopes/scope-C/CMakeLists.txt	2015-01-14 02:25:50 +0000
@@ -1,3 +1,4 @@
 add_library(scope-C MODULE scope-C.cpp)
+add_dependencies(scope-C scoperegistry scoperunner)
 
 configure_file(scope-C.ini.in scope-C.ini)

=== modified file 'demo/scopes/scope-D/CMakeLists.txt'
--- demo/scopes/scope-D/CMakeLists.txt	2013-11-19 02:51:46 +0000
+++ demo/scopes/scope-D/CMakeLists.txt	2015-01-14 02:25:50 +0000
@@ -1,3 +1,4 @@
 add_library(scope-D MODULE scope-D.cpp)
+add_dependencies(scope-D scoperegistry scoperunner)
 
 configure_file(scope-D.ini.in scope-D.ini)

=== modified file 'demo/scopes/scope-N/CMakeLists.txt'
--- demo/scopes/scope-N/CMakeLists.txt	2013-11-19 02:51:46 +0000
+++ demo/scopes/scope-N/CMakeLists.txt	2015-01-14 02:25:50 +0000
@@ -1,3 +1,4 @@
 add_library(scope-N MODULE scope-N.cpp)
+add_dependencies(scope-N scoperegistry scoperunner)
 
 configure_file(scope-N.ini.in scope-N.ini)

=== modified file 'demo/scopes/scope-S/CMakeLists.txt'
--- demo/scopes/scope-S/CMakeLists.txt	2013-11-19 02:51:46 +0000
+++ demo/scopes/scope-S/CMakeLists.txt	2015-01-14 02:25:50 +0000
@@ -1,3 +1,4 @@
 add_library(scope-S MODULE scope-S.cpp)
+add_dependencies(scope-S scoperegistry scoperunner)
 
 configure_file(scope-S.ini.in scope-S.ini)

=== modified file 'demo/stand-alone/CMakeLists.txt'
--- demo/stand-alone/CMakeLists.txt	2014-04-17 06:08:05 +0000
+++ demo/stand-alone/CMakeLists.txt	2015-01-14 02:25:50 +0000
@@ -3,7 +3,9 @@
 add_executable(stand-alone-client stand-alone-client.cpp)
 
 # Add_dependencies should be used sparingly. In this case we need the global
-# header to be generated before we start building the client binary.
+# header to be generated before we start building the client binary, and
+# we need an up-to-date registry to run it.
 add_dependencies(stand-alone-client globalheader)
+add_dependencies(stand-alone-client scoperegistry)
 add_definitions(-DDEMO_RUNTIME_PATH="${CMAKE_CURRENT_BINARY_DIR}/Runtime.ini")
 target_link_libraries(stand-alone-client ${UNITY_SCOPES_LIB} ${OTHER_LIBS})

=== modified file 'doc/Doxyfile-devel.in'
--- doc/Doxyfile-devel.in	2014-06-16 03:26:21 +0000
+++ doc/Doxyfile-devel.in	2015-01-14 02:25:50 +0000
@@ -1758,7 +1758,7 @@
 # DOT_GRAPH_MAX_NODES then the graph will not be shown at all. Also note
 # that the size of a graph can be further restricted by MAX_DOT_GRAPH_DEPTH.
 
-DOT_GRAPH_MAX_NODES    = 50
+DOT_GRAPH_MAX_NODES    = 100
 
 # The MAX_DOT_GRAPH_DEPTH tag can be used to set the maximum depth of the
 # graphs generated by dot. A depth value of 3 means that only nodes reachable

=== modified file 'include/unity/scopes/AbstractScopeBase.h'
--- include/unity/scopes/AbstractScopeBase.h	2014-11-03 05:31:30 +0000
+++ include/unity/scopes/AbstractScopeBase.h	2015-01-14 02:25:50 +0000
@@ -36,6 +36,11 @@
 See unity::scopes::ScopeBase for documentation of the member functions.
 */
 
+// TODO: Once we can make an ABI-breaking change, add a virtual destructor and remove the pragma.
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wnon-virtual-dtor"
+
 class AbstractScopeBase
 {
 public:
@@ -58,6 +63,8 @@
 /// @endcond
 };
 
+#pragma GCC diagnostic pop
+
 } // namespace scopes
 
 } // namespace unity

=== modified file 'include/unity/scopes/Category.h'
--- include/unity/scopes/Category.h	2014-11-03 05:31:30 +0000
+++ include/unity/scopes/Category.h	2015-01-14 02:25:50 +0000
@@ -42,7 +42,7 @@
    \brief A set of related results returned by a scope
    and displayed within a single pane in the Unity dash.
 
-   To create a Category, use ReplyProxy::register_category.
+   To create a Category, use SearchReplyProxy::register_category.
    \see ResultItem
 */
 class Category

=== modified file 'include/unity/scopes/ScopeBase.h'
--- include/unity/scopes/ScopeBase.h	2014-12-01 08:20:37 +0000
+++ include/unity/scopes/ScopeBase.h	2015-01-14 02:25:50 +0000
@@ -136,6 +136,11 @@
 call to stop() in a timely manner.
 */
 
+// TODO: Once we can make an ABI-breaking change, remove the pragma.
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wnon-virtual-dtor"
+
 class ScopeBase : public AbstractScopeBase
 {
 public:
@@ -342,6 +347,8 @@
     /// @endcond
 };
 
+#pragma GCC diagnostic pop
+
 } // namespace scopes
 
 } // namespace unity

=== modified file 'include/unity/scopes/internal/CategoryRegistry.h'
--- include/unity/scopes/internal/CategoryRegistry.h	2014-11-03 05:31:30 +0000
+++ include/unity/scopes/internal/CategoryRegistry.h	2015-01-14 02:25:50 +0000
@@ -46,13 +46,15 @@
     CategoryRegistry() = default;
 
     /**
-    \brief Deserializes category from a variant_map and registers it. Throws if category with same id exists.
+    \brief Deserializes category from a variant_map and registers it.
+    \throws unity::InvalidArgumentException if a category with the same id exists already.
     \return category instance
     */
     Category::SCPtr register_category(VariantMap const& variant_map);
 
     /**
-    \brief Creates category from supplied parameters. Throws if category with same id exists.
+    \brief Creates category from supplied parameters.
+    \throws unity::InvalidArgumentException if a category with the same id exists already.
     \return category instance
     */
     Category::SCPtr register_category(std::string const& id, std::string const& title, std::string const& icon, CannedQuery::SCPtr const& query, CategoryRenderer const& renderer_template);
@@ -65,7 +67,7 @@
 
     /**
     \brief Register an existing category instance with this registry.
-    Throws if category with sane id exists.
+    \throws unity::InvalidArgumentException if a category with the same id exists already.
     */
     void register_category(Category::SCPtr category);
 

=== modified file 'include/unity/scopes/internal/smartscopes/SSScopeObject.h'
--- include/unity/scopes/internal/smartscopes/SSScopeObject.h	2014-11-20 05:32:53 +0000
+++ include/unity/scopes/internal/smartscopes/SSScopeObject.h	2015-01-14 02:25:50 +0000
@@ -52,9 +52,9 @@
 
     // Remote operation implementations
     MWQueryCtrlProxy search(CannedQuery const& q,
-                                  SearchMetadata const& hints,
-                                  MWReplyProxy const& reply,
-                                  InvokeInfo const& info) override;
+                            SearchMetadata const& hints,
+                            MWReplyProxy const& reply,
+                            InvokeInfo const& info) override;
 
     MWQueryCtrlProxy activate(Result const& result,
                               ActionMetadata const& hints,

=== modified file 'src/scopes/internal/JsonCppNode.cpp'
--- src/scopes/internal/JsonCppNode.cpp	2014-08-18 08:12:22 +0000
+++ src/scopes/internal/JsonCppNode.cpp	2015-01-14 02:25:50 +0000
@@ -276,7 +276,7 @@
     return std::make_shared<JsonCppNode>(value_node);
 }
 
-JsonNodeInterface::SPtr JsonCppNode::get_node(uint node_index) const
+JsonNodeInterface::SPtr JsonCppNode::get_node(unsigned int node_index) const
 {
     if (root_.type() != Json::arrayValue)
     {

=== modified file 'src/scopes/internal/Logger.cpp'
--- src/scopes/internal/Logger.cpp	2014-12-01 12:41:49 +0000
+++ src/scopes/internal/Logger.cpp	2015-01-14 02:25:50 +0000
@@ -114,7 +114,7 @@
     }
 }
 
-Logger::operator src::severity_channel_logger_mt<>&()
+Logger::operator boost::log::sources::severity_channel_logger_mt<>&()
 {
     return logger_;
 }

=== modified file 'src/scopes/internal/QueryCtrlImpl.cpp'
--- src/scopes/internal/QueryCtrlImpl.cpp	2014-11-20 05:47:58 +0000
+++ src/scopes/internal/QueryCtrlImpl.cpp	2015-01-14 02:25:50 +0000
@@ -48,7 +48,6 @@
     // inform the reply object belonging to this query that the query is finished.
     assert(reply_proxy);
 
-    lock_guard<mutex> lock(mutex_);
     ready_ = ctrl_proxy != nullptr;
     cancelled_ = false;
 }

=== modified file 'src/scopes/internal/ReplyImpl.cpp'
--- src/scopes/internal/ReplyImpl.cpp	2014-11-20 05:29:23 +0000
+++ src/scopes/internal/ReplyImpl.cpp	2015-01-14 02:25:50 +0000
@@ -94,7 +94,7 @@
         {
             fwd()->finished(CompletionDetails(CompletionDetails::OK));  // Oneway, can't block
         }
-        catch (std::exception const& e)
+        catch (std::exception const&)
         {
             // No logging here because this may happen after the run time is destroyed.
         }

=== modified file 'src/scopes/internal/ScopeBaseImpl.cpp'
--- src/scopes/internal/ScopeBaseImpl.cpp	2014-11-13 02:31:13 +0000
+++ src/scopes/internal/ScopeBaseImpl.cpp	2015-01-14 02:25:50 +0000
@@ -39,6 +39,7 @@
     : scope_dir_initialized_(false)
     , cache_dir_initialized_(false)
     , app_dir_initialized_(false)
+    , tmp_dir_initialized_(false)
     , registry_initialized_(false)
     , settings_db_initialized_(false)
 {

=== modified file 'src/scopes/internal/SearchMetadataImpl.cpp'
--- src/scopes/internal/SearchMetadataImpl.cpp	2014-09-02 15:47:35 +0000
+++ src/scopes/internal/SearchMetadataImpl.cpp	2015-01-14 02:25:50 +0000
@@ -19,8 +19,8 @@
 #include <unity/scopes/internal/SearchMetadataImpl.h>
 #include <unity/scopes/internal/Utils.h>
 #include <unity/scopes/ScopeExceptions.h>
+
 #include <unity/UnityExceptions.h>
-#include <sstream>
 
 namespace unity
 {

=== modified file 'src/scopes/utility/internal/BufferedResultForwarderImpl.cpp'
--- src/scopes/utility/internal/BufferedResultForwarderImpl.cpp	2014-12-01 09:47:04 +0000
+++ src/scopes/utility/internal/BufferedResultForwarderImpl.cpp	2015-01-14 02:25:50 +0000
@@ -72,7 +72,7 @@
     // scope author tells us that results for this forwarder are now ready
     // to be displayed (or the query has finished); set the 'ready' flag
     // and if previous forwarder is also ready, then push and disable
-    // futher buffering.
+    // further buffering.
     if (!ready_.exchange(true))
     {
         if (previous_ready_ || !has_previous_)