Bug 70377 - [1.0] don't strip ABI versions from interface names
Summary: [1.0] don't strip ABI versions from interface names
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords: patch
Depends on: 69431
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-11 15:35 UTC by Simon McVittie
Modified: 2013-10-29 07:32 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
[tp-glib next] Don't strip ABI versions from generated code (558.08 KB, patch)
2013-10-11 15:54 UTC, Simon McVittie
Details | Splinter Review
[idle next] Adapt for use of ABI versions in generated code (27.97 KB, patch)
2013-10-11 15:55 UTC, Simon McVittie
Details | Splinter Review
[haze next] Adapt for use of ABI versions in generated code (23.62 KB, patch)
2013-10-11 15:56 UTC, Simon McVittie
Details | Splinter Review
[MC next] Adapt for ABI versions in telepathy-glib codegen (19.03 KB, patch)
2013-10-16 17:12 UTC, Simon McVittie
Details | Splinter Review
[Salut next] Update for telepathy-glib codegen putting ABI versions in names (71.92 KB, patch)
2013-10-16 17:12 UTC, Simon McVittie
Details | Splinter Review
[gabble] Adapt for use of ABI versions in generated code (141.83 KB, patch)
2013-10-25 15:10 UTC, Guillaume Desmottes
Details | Splinter Review
[rakia] Adapt for use of ABI versions in generated code (12.31 KB, patch)
2013-10-25 15:19 UTC, Guillaume Desmottes
Details | Splinter Review
[logger] Adapt for use of ABI versions in generated code (990 bytes, patch)
2013-10-28 09:10 UTC, Guillaume Desmottes
Details | Splinter Review
[logger] tests/lib: sync with tp-glib next (22.86 KB, patch)
2013-10-28 09:11 UTC, Guillaume Desmottes
Details | Splinter Review
[tp-fs] Adapt for use of ABI versions in generated code (27.70 KB, patch)
2013-10-28 09:27 UTC, Guillaume Desmottes
Details | Splinter Review
[tp-fs] call-handler: use tp_call_channel_accept_async() (862 bytes, patch)
2013-10-28 16:15 UTC, Guillaume Desmottes
Details | Splinter Review

Description Simon McVittie 2013-10-11 15:35:59 UTC
To make 0.99.2 easier to reach, telepathy-glib generates tp_cli_channel_type_call_call_hangup() instead of tp_cli_channel_type_call1_call_hangup(), and so on.

We should include the "1" in non-core interfaces so that we can actually version them.

Motivation for doing this now: I want to move a bunch of our non-spec interfaces into the main spec. Since many of them have names like .DRAFT, they need renaming *anyway*, so we might as well version them properly, even in 0.x; but if I do that and merge it into 1.0 before fixing this, I'll have to delete their ABI versions and then reinstate them later.
Comment 1 Simon McVittie 2013-10-11 15:54:22 UTC
Created attachment 87462 [details] [review]
[tp-glib next] Don't strip ABI versions from generated code

---

 90 files changed, 2881 insertions(+), 2894 deletions(-)

This was the least interesting patch I've done for a long time.

The interesting part is in tools/.
Comment 2 Simon McVittie 2013-10-11 15:55:39 UTC
Created attachment 87463 [details] [review]
[idle next] Adapt for use of ABI versions in generated code
Comment 3 Simon McVittie 2013-10-11 15:56:03 UTC
Created attachment 87464 [details] [review]
[haze next] Adapt for use of ABI versions in generated code
Comment 4 Simon McVittie 2013-10-11 15:57:50 UTC
Also needs patches for the rest of the usual suspects:

Empathy (unless it only uses high-level API)
Farstream (unless it only uses high-level API)
Folks (unless it only uses high-level API)
Logger (unless it only uses high-level API)

Gabble (please do Bug #69431 first)
MC
Rakia
Salut
Comment 5 Guillaume Desmottes 2013-10-11 21:21:47 UTC
Comment on attachment 87462 [details] [review]
[tp-glib next] Don't strip ABI versions from generated code

Review of attachment 87462 [details] [review]:
-----------------------------------------------------------------

++
Comment 6 Guillaume Desmottes 2013-10-11 21:22:16 UTC
Comment on attachment 87463 [details] [review]
[idle next] Adapt for use of ABI versions in generated code

Review of attachment 87463 [details] [review]:
-----------------------------------------------------------------

++ assuming tests pass
Comment 7 Guillaume Desmottes 2013-10-11 21:22:35 UTC
Comment on attachment 87464 [details] [review]
[haze next] Adapt for use of ABI versions in generated code

Review of attachment 87464 [details] [review]:
-----------------------------------------------------------------

++ assuming tests pass
Comment 8 Simon McVittie 2013-10-16 17:12:00 UTC
Created attachment 87743 [details] [review]
[MC next] Adapt for ABI versions in telepathy-glib codegen

---

Same idea.
Comment 9 Simon McVittie 2013-10-16 17:12:50 UTC
Created attachment 87744 [details] [review]
[Salut next] Update for telepathy-glib codegen putting ABI versions in  names
Comment 10 Guillaume Desmottes 2013-10-16 17:37:26 UTC
Comment on attachment 87743 [details] [review]
[MC next] Adapt for ABI versions in telepathy-glib codegen

Review of attachment 87743 [details] [review]:
-----------------------------------------------------------------

++
Comment 11 Guillaume Desmottes 2013-10-16 17:37:45 UTC
Comment on attachment 87744 [details] [review]
[Salut next] Update for telepathy-glib codegen putting ABI versions in  names

Review of attachment 87744 [details] [review]:
-----------------------------------------------------------------

++
Comment 12 Guillaume Desmottes 2013-10-25 13:25:48 UTC
I merged all these patches to 'next'.
Comment 13 Guillaume Desmottes 2013-10-25 15:10:30 UTC
Created attachment 88113 [details] [review]
[gabble] Adapt for use of ABI versions in generated code
Comment 14 Guillaume Desmottes 2013-10-25 15:19:18 UTC
Created attachment 88114 [details] [review]
[rakia] Adapt for use of ABI versions in generated code
Comment 15 Guillaume Desmottes 2013-10-28 09:10:47 UTC
Created attachment 88215 [details] [review]
[logger] Adapt for use of ABI versions in generated code
Comment 16 Guillaume Desmottes 2013-10-28 09:11:02 UTC
Created attachment 88216 [details] [review]
[logger] tests/lib: sync with tp-glib next
Comment 17 Guillaume Desmottes 2013-10-28 09:27:27 UTC
Created attachment 88221 [details] [review]
[tp-fs] Adapt for use of ABI versions in generated code
Comment 19 Simon McVittie 2013-10-28 10:33:11 UTC
Thanks, checking...
Comment 20 Simon McVittie 2013-10-28 10:54:38 UTC
Comment on attachment 88113 [details] [review]
[gabble] Adapt for use of ABI versions in generated code

Review of attachment 88113 [details] [review]:
-----------------------------------------------------------------

Gabble in "rather large" shock...

Looks good.

::: src/conn-aliasing.c
@@ +1053,4 @@
>        g_value_take_string (val, alias);
>  
>        tp_contacts_mixin_set_contact_attribute (attributes_hash,
> +        handle, TP_IFACE_CONNECTION_INTERFACE_ALIASING1"/alias",

Definitely not a blocker (quite the opposite: please merge first!) but it would be good to grep these projects for "/alias" and use TP_TOKEN_CONNECTION_INTERFACE_ALIASING1_ALIAS.

::: src/conn-avatars.c
@@ +612,4 @@
>              g_value_set_string (val, "");
>  
>            tp_contacts_mixin_set_contact_attribute (attributes_hash, handle,
> +            TP_IFACE_CONNECTION_INTERFACE_AVATARS1"/token", val);

... and grep for "/token" too

::: src/conn-client-types.c
@@ +148,4 @@
>        val = tp_g_value_slice_new_take_boxed (G_TYPE_STRV, types);
>  
>        tp_contacts_mixin_set_contact_attribute (attributes_hash, handle,
> +          TP_IFACE_CONNECTION_INTERFACE_CLIENT_TYPES1 "/client-types", val);

... and "/client-types"

::: src/conn-contact-info.c
@@ +1057,4 @@
>                        TP_ARRAY_TYPE_CONTACT_INFO_FIELD_LIST, contact_info);
>  
>                tp_contacts_mixin_set_contact_attribute (attributes_hash,
> +                      contact, TP_IFACE_CONNECTION_INTERFACE_CONTACT_INFO1"/info",

... and "/info"

::: src/conn-location.c
@@ +622,4 @@
>                TP_HASH_TYPE_STRING_VARIANT_MAP, location);
>  
>            tp_contacts_mixin_set_contact_attribute (attributes_hash,
> +              handle, TP_IFACE_CONNECTION_INTERFACE_LOCATION1"/location", val);

and "/location"

::: src/connection.c
@@ +3429,4 @@
>  
>        tp_contacts_mixin_set_contact_attribute (attributes_hash,
>            handle,
> +          TP_IFACE_CONNECTION_INTERFACE_CONTACT_CAPABILITIES1"/capabilities",

this too

::: src/media-factory.c
@@ +840,4 @@
>    if (G_UNLIKELY (qc_gtalk_p2p == 0))
>      {
>        qc_gtalk_p2p = g_quark_from_static_string (
> +          TP_IFACE_CHANNEL_TYPE_CALL1 "/gtalk-p2p");

I forget, do we have tokens for these too?
Comment 21 Simon McVittie 2013-10-28 10:56:47 UTC
Comment on attachment 88114 [details] [review]
[rakia] Adapt for use of ABI versions in generated code

Review of attachment 88114 [details] [review]:
-----------------------------------------------------------------

Looks good.

What I said about "/alias" for Gabble applies here too.
Comment 22 Simon McVittie 2013-10-28 10:57:00 UTC
Comment on attachment 88215 [details] [review]
[logger] Adapt for use of ABI versions in generated code

Review of attachment 88215 [details] [review]:
-----------------------------------------------------------------

Sure.
Comment 23 Simon McVittie 2013-10-28 11:02:14 UTC
Comment on attachment 88216 [details] [review]
[logger] tests/lib: sync with tp-glib next

Review of attachment 88216 [details] [review]:
-----------------------------------------------------------------

Sure.
Comment 24 Simon McVittie 2013-10-28 11:12:33 UTC
Comment on attachment 88221 [details] [review]
[tp-fs] Adapt for use of ABI versions in generated code

Review of attachment 88221 [details] [review]:
-----------------------------------------------------------------

Sure

::: examples/call-handler.c
@@ +578,4 @@
>  
>    tp_handle_channels_context_accept (handler_context);
>  
> +  tp_cli_channel_type_call1_call_accept (proxy, -1,

Certainly not a blocker, but shouldn't this be using tp_call_channel_accept_async() since we have it?

@@ +632,5 @@
>    tp_base_client_add_handler_capabilities_varargs (client,
> +    TP_IFACE_CHANNEL_TYPE_CALL1 "/video/h264",
> +    TP_IFACE_CHANNEL_TYPE_CALL1 "/shm",
> +    TP_IFACE_CHANNEL_TYPE_CALL1 "/ice",
> +    TP_IFACE_CHANNEL_TYPE_CALL1 "/gtalk-p2p",

Not a blocker, but, tokens would be good.
Comment 25 Guillaume Desmottes 2013-10-28 14:45:56 UTC
All merged, I'll look at the extra comments now.
Comment 26 Guillaume Desmottes 2013-10-28 16:09:31 UTC
I insta merged the patches to use TOKEN macros, except some in Gabble because of bug #70962
Comment 27 Guillaume Desmottes 2013-10-28 16:15:40 UTC
Created attachment 88236 [details] [review]
[tp-fs] call-handler: use tp_call_channel_accept_async()
Comment 28 Simon McVittie 2013-10-28 19:05:47 UTC
Comment on attachment 88236 [details] [review]
[tp-fs] call-handler: use tp_call_channel_accept_async()

Review of attachment 88236 [details] [review]:
-----------------------------------------------------------------

::: examples/call-handler.c
@@ +578,4 @@
>  
>    tp_handle_channels_context_accept (handler_context);
>  
> +  tp_call_channel_accept_async (TP_CALL_CHANNEL (proxy), NULL, NULL);

Assuming it works, ++
Comment 29 Guillaume Desmottes 2013-10-29 07:32:56 UTC
All merged; closing.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.