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

[Michał Kuchta]

Bug #1195540: [enhancement] Make able to get version information from client / server APIs - corrections after review

Bug #1195540: [enhancement] Make able to get version information from client / server APIs - corrections after review

[Daniel van Vugt (vanvugt)]

Cool, thanks.

Approved, but could be improved with:
  * Better documentation mentioning you need to compare the result to MIR_VERSION_NUMBER(x,y,z)
  * Remove extra blank link from the cpp file.
  * Acceptance tests to verify the function returns MIR_CLIENT_API_VERSION (when called within the same source tree).

Although I now realize the use of dlsym() mentioned in the bug may negate the need for all this at all, it doesn't hurt...

review: Approve

[Cemil Azizoglu (cemil-azizoglu)]

+1 on the acceptance test. But it's trivial in this case so feel free to top approve if you decide not to indulge.

review: Approve

[Michał Kuchta (kuchtam)]

Fixed in revision: 4009

[Daniel van Vugt (vanvugt)]

Thanks, but this should be "==" or "EXPECT_EQ"

+ MIR_CLIENT_API_VERSION_PATCH) >= mir_get_client_api_version());

review: Needs Fixing

[Michał Kuchta (kuchtam)]

Fixed in rev: 4010

[Daniel van Vugt (vanvugt)]

review: Approve

[Daniel van Vugt (vanvugt)]

As mentioned above, I now realize we may never need this function. Simply using dlsym() by itself should suffice. But the bug was open, and it's a great practice exercise.

Just waiting to see if any of ~mir-team objects to this.

[Daniel van Vugt (vanvugt)]

Michał,

If nobody has already mentioned it to you, please also complete this:
  https://www.ubuntu.com/legal/contributors/submit

And for the Canonical Project Manager field, enter:
  Kevin Gunn (<email address hidden>)

More information:
  https://www.ubuntu.com/legal/contributors

[Michał Kuchta (kuchtam)]

Hi Daniel,

I have signed document.

I think that this function can be useful because it gives you simple and
quick information about which lib version is use now.

Of course you can do this by dlsym - but each time you must check if
returned address is correct

and when API has big number of functions it can be uncomfortable.

Best Regards,

Michał

2017-02-08 4:09 GMT+01:00 Daniel van Vugt <email address hidden>:

> Michał,
>
> If nobody has already mentioned it to you, please also complete this:
> https://www.ubuntu.com/legal/contributors/submit
>
> And for the Canonical Project Manager field, enter:
> Kevin Gunn (<email address hidden>)
>
> More information:
> https://www.ubuntu.com/legal/contributors
> --
> https://code.launchpad.net/~kuchtam/mir/bug_1195540_after_
> review/+merge/316287
> You are the owner of lp:~kuchtam/mir/bug_1195540_after_review.
>

[Cemil Azizoglu (cemil-azizoglu)]

Huh I wonder why jenkins hasn't run on this? Oh well.

review: Approve

[Mir CI Bot (mir-ci-bot)]

review: Approve (continuous-integration)

=== 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-06 07:16:22 +0000
@@ -473,6 +473,15 @@
     MirErrorCallback callback,
     void* context);
 
+/**
+ * Returns client API version
+ *
+ * Result of the function should be compared to result of MIR_VERSION_NUMBER
+ *
+ * \return  The client API version
+ */
+unsigned mir_get_client_api_version();
+
 #ifdef __cplusplus
 }
 /**@}*/

=== 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-06 07:16:22 +0000
@@ -414,6 +414,11 @@
     }
 }
 
+unsigned mir_get_client_api_version()
+{
+    return MIR_CLIENT_API_VERSION;
+}
+
 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-02 19:17:46 +0000
+++ src/client/symbols.map	2017-02-06 07:16:22 +0000
@@ -601,4 +601,5 @@
   global:
     mir_connection_apply_session_input_config;
     mir_connection_set_base_input_config;
+    mir_get_client_api_version;
 } MIR_CLIENT_0.26.1;

=== modified file 'tests/acceptance-tests/test_client_library.cpp'
--- tests/acceptance-tests/test_client_library.cpp	2017-02-03 22:37:56 +0000
+++ tests/acceptance-tests/test_client_library.cpp	2017-02-06 07:16:22 +0000
@@ -1225,3 +1225,10 @@
     mir_buffer_stream_release_sync(stream);
     mir_connection_release(connection);
 }
+
+TEST_F(ClientLibrary, client_api_version)
+{
+    ASSERT_TRUE( MIR_VERSION_NUMBER(MIR_CLIENT_API_VERSION_MAJOR,
+                                    MIR_CLIENT_API_VERSION_MINOR,
+                                    MIR_CLIENT_API_VERSION_PATCH) == mir_get_client_api_version());
+}