Merge lp:~kuchtam/mir/mir into lp:mir

[Michał Kuchta]

Implementation of bug#1195540 - "[enhancement] Make able to get version information from client / server APIs"

Implementation of bug#1195540 - "[enhancement] Make able to get version information from client / server APIs"

[Daniel van Vugt (vanvugt)]

Thanks for doing this.

I believe it's actually the version of libmirclient that we want to return. Not the version of libmirserver. So you can do that just by returning:
  MIR_CLIENT_API_VERSION
or:
  MIR_VERSION_NUMBER(MIR_VERSION_MAJOR, MIR_VERSION_MINOR, MIR_VERSION_PATCH)

review: Needs Fixing

[Brandon Schaefer (brandontschaefer)]

Also if you're doing c++ calls that can possibly throw we need to surround that with try/catch to avoid leaking a throw to the C API.

+unsigned mir_connection_get_server_version(const MirConnection* connection)

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

I think you can safely skip Brandon's comment because it's not relevant to the final solution. All we need is:

unsigned mir_get_client_api_version()
{
    return MIR_CLIENT_API_VERSION;
}

And yes, those are different things. One describes the version of the headers you are using and the other describes the version of the libmirclient binary that you are using. Usually they will be equal, but the point of bug 1195540 is to cover cases where you want to dynamically bind to new API functions at runtime.

[Brandon Schaefer (brandontschaefer)]

Very true

review: Abstain

[Andreas Pokorny (andreas-pokorny)]

You need to add a commit message to the merge request so that CI will run with the branch and allow autolanding.

[lp:~kuchtam/mir/mir]

3973. By Michal Kuchta <email address hidden> on 2017-02-01

Fix after review

3974. By Michal Kuchta <email address hidden> on 2017-02-01

Fix after review - merge

[Michał Kuchta (kuchtam)]

Thank you for your comments.

I’ve made corrections according your comments: -r 3974.

[Daniel van Vugt (vanvugt)]

Thanks. But MIR_CLIENT_API_VERSION is already defined (starting in Mir v0.26.0 released a few hours ago). Get it from mir_toolkit/version.h

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

That also means the new function is part of Mir 0.27 (or 1.0 or whatever), so this needs moving:

63 mir_window_request_persistent_id_sync;
64 + mir_get_client_api_version;
65 } MIR_CLIENT_0.25;

[Michał Kuchta (kuchtam)]

I have committed correction and it is available on separate branch: https://code.launchpad.net/~kuchtam/mir/bug_1195540_after_review/+merge/316287

[Daniel van Vugt (vanvugt)]

OK, I'll reject this branch and look at the other one instead.

Unmerged revisions

3974. By Michal Kuchta <email address hidden> on 2017-02-01

Fix after review - merge

3973. By Michal Kuchta <email address hidden> on 2017-02-01

Fix after review

=== modified file 'include/client/mir_toolkit/mir_connection.h'
--- include/client/mir_toolkit/mir_connection.h	2017-01-31 19:33:32 +0000
+++ include/client/mir_toolkit/mir_connection.h	2017-02-02 00:05:14 +0000
@@ -473,6 +473,13 @@
     MirErrorCallback callback,
     void* context);
 
+/**
+ * Returns client API version
+ *
+ * \return  The client API version
+ */
+unsigned mir_get_client_api_version();
+
 #ifdef __cplusplus
 }
 /**@}*/

=== modified file 'include/core/mir_toolkit/mir_version_number.h'
--- include/core/mir_toolkit/mir_version_number.h	2017-01-18 02:29:37 +0000
+++ include/core/mir_toolkit/mir_version_number.h	2017-02-02 00:05:14 +0000
@@ -34,4 +34,17 @@
 #define MIR_VERSION_NUMBER(major,minor,micro) \
     (((major) << 22) + ((minor) << 12) + (micro))
 
+/**
+ * MIR_CLIENT_API_VERSION
+ * \param major [in]    The major version (eg: 0 for version 0.26)
+ * \param minor [in]    The minor version (eg: 26 for version 0.26)
+ *
+ * Returns the combined version information as a single 32-bit value.
+ *
+ * Can be use to write a client that is not dependent on the newer
+ * functionality but can utilize it.
+ */
+#define MIR_CLIENT_API_VERSION(major,minor) \
+    ((major << 16) + minor)
+
 #endif /* MIR_VERSION_NUMBER_H_ */

=== modified file 'src/client/mir_connection_api.cpp'
--- src/client/mir_connection_api.cpp	2017-01-24 19:42:48 +0000
+++ src/client/mir_connection_api.cpp	2017-02-02 00:05:14 +0000
@@ -414,6 +414,11 @@
     }
 }
 
+unsigned mir_get_client_api_version()
+{
+    return MIR_CLIENT_API_VERSION(MIR_VERSION_MAJOR, MIR_VERSION_MINOR);
+}
+
 void mir_connection_apply_session_display_config(MirConnection* connection, MirDisplayConfig const* display_config)
 try
 {

=== modified file 'src/client/symbols.map'
--- src/client/symbols.map	2017-02-01 18:29:26 +0000
+++ src/client/symbols.map	2017-02-02 00:05:14 +0000
@@ -583,6 +583,7 @@
     mir_window_get_preferred_orientation;
     mir_window_request_persistent_id;
     mir_window_request_persistent_id_sync;
+    mir_get_client_api_version;
 } MIR_CLIENT_0.25;
 
 MIR_CLIENT_0.27 {  # New functions in Mir 0.27 or 1.0

=== modified file 'tests/acceptance-tests/test_client_header_version.cpp'
--- tests/acceptance-tests/test_client_header_version.cpp	2017-01-17 05:58:36 +0000
+++ tests/acceptance-tests/test_client_header_version.cpp	2017-02-02 00:05:14 +0000
@@ -80,3 +80,15 @@
                                  MIR_VERSION_MINOR,
                                  MIR_VERSION_MICRO), MIR_CLIENT_API_VERSION);
 }
+
+TEST(ClientApiVersion, mir_client_api_version_is_sane)
+{
+
+    EXPECT_TRUE(0x00010000 >= MIR_CLIENT_API_VERSION(1, 0));
+
+    EXPECT_TRUE(0x00000001 >= MIR_CLIENT_API_VERSION(0, 1));
+
+    EXPECT_TRUE(0x00010001 >= MIR_CLIENT_API_VERSION(1, 1));
+
+    EXPECT_TRUE(0x0002ffff >= MIR_CLIENT_API_VERSION(1, 65536));
+}