After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769222 - Regression: Unable to compile Socket code against GLib 2.46
Regression: Unable to compile Socket code against GLib 2.46
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Bindings: GLib
0.32.x
Other All
: High major
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2016-07-27 11:49 UTC by Marcin Lewandowski
Modified: 2016-09-24 07:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
testcase for gio g_socket_create_source bug (759 bytes, text/x-vala)
2016-08-03 22:59 UTC, José Miguel Fonte
  Details
gio-2.0: Keep GLib.Socket API compatible with gio < 2.48 (7.73 KB, patch)
2016-09-16 18:57 UTC, Rico Tzschichholz
none Details | Review
gio-2.0: Keep GLib.Socket API compatible with gio < 2.48 (8.10 KB, patch)
2016-09-20 10:31 UTC, Rico Tzschichholz
committed Details | Review

Description Marcin Lewandowski 2016-07-27 11:49:30 UTC
Vala 0.32 ships with VAPI that adds DatagramBased API for UDP sockets in gio. It was added in GLib 2.48.

However, lack of conditionals in the VAPI makes compilation impossible against GLib 2.46 which does not contain DatagramBased class.
Comment 1 Al Thomas 2016-07-27 15:14:49 UTC
According to https://developer.gnome.org/gio/stable/GSocket.html Gsocket has these functions:

g_socket_receive_messages () - since 2.48
g_socket_send_messages () - since 2.44
g_socket_create_source () - since 2.22
g_socket_condition_check () - since 2.22
g_socket_condition_wait () - since 2.22

and from https://developer.gnome.org/gio/stable/GDatagramBased.html the GDatagramBasedInterface should implement the following methods:

receive_messages ()
send_messages ()
create_source ()
condition_check ()
condition_wait ()

Although the function names appear to show the interface is implemented, both receive_messages() and send_messages() have an additional parameter, gint64 timeout, that isn't present in the GSocket implementation.

I don't know of an elegant solution for versioning interfaces in GIR derived bindings. Is there one?
Comment 2 Marcin Lewandowski 2016-07-28 20:55:01 UTC
Possible workaround is copying gio-2.0.vapi from Vala 0.30 and adding it to the vapidir. It will have precedence over one shipped with the compiler.
Comment 3 José Miguel Fonte 2016-08-03 22:59:27 UTC
Created attachment 332692 [details]
testcase for gio g_socket_create_source bug

Also have a related problem. Some of my code which worked on vala-0.30 (Gtk 3.18) ceased to work (but compiles) on vala-0.32 (Gtk 3.20). Seems that the GSocket class method create_source does not exist in vala-0.32, only the GDatagramBased method it's used by the compiler. In C we can invoke g_source_create_source and/or g_datagram_based_create_source. In vala-0.32, g_source_create_source it's not used. This will make socket connections of TYPE_STREAM (TCP), fail as the returned source will be NULL.

I've attached a very simple example which will compile and work on vala-0.30 but will fail as soon as we get the connection and try to create a source via the socket.

Error message is:
ERROR:/home/[path deleted...]/vala-032-gio-bug/test-example.vala.c:90:_vala_main: assertion failed: (source != null)
Aborted (core dumped)

Check the enclosed file.
Comment 4 Marcin Lewandowski 2016-08-03 23:25:11 UTC
I think that current approach that vala compiler contains bundle of always-recent VAPIs is just invalid.

VAPI should ship with libraries.

If this is not possible, there could be a versioned repository of VAPIs. IMO that would have been way more maintainable than current approach, where necessary fix to the compiler itself may break dependencies.
Comment 5 Al Thomas 2016-08-04 12:40:07 UTC
Marcin,(In reply to Marcin Lewandowski from comment #4)
> If this is not possible, there could be a versioned repository of VAPIs. IMO
> that would have been way more maintainable than current approach, where
> necessary fix to the compiler itself may break dependencies.

I'm not sure that the identification of this problem is a project organisation issue, but more a resource issue. The patch that went in and added the GDatagramBased interface was on 26 October 2015 ( https://git.gnome.org/browse/vala/commit/vapi/gio-2.0.vapi?id=cbd7b680395abcc5a2bea3d3f51455f247cd97c1 ). Since then there has been a development release (0.31.1) on 07 February 2016 and then the stable release 0.32 on 21 March 2016, nearly five months after the original patch. There are a steady stream of GIR based binding updates, usually a few every month ( see https://git.gnome.org/browse/vala/log/?qt=grep&q=gir-based+bindings ). Vala ( and GLib and GNOME projects in general ) follows a collaborative development model using free/open source licensing. So although these project are available without monetary cost, it should be recognised there are other "costs" involved. The more people periodically testing their builds against master or development releases then the more likely things like this will be picked up earlier.

As to this particular issue with the GDatagramBased interface, I'm thinking there is a mismatch between C and a strongly typed languages like Vala. In Vala interfaces provide polymorphism. So GDatagramBased would be a set of methods and properties any object could implement and still be passed around functions that accept an argument of GDatagramBased. I can understand that perfectly well for DtlsClientConnection and DtlsServerConnection. DTLS effectively being Transport Layer Security over UDP. What I can't understand is how GSocket can be seen as implementing GDatagramBased. This effectively means all sockets can be used for DTLS, even when they are stream based TCP sockets.
Comment 6 Marcin Lewandowski 2016-08-04 12:50:47 UTC
I am 100% sure that this is a project organisation issue.

I might want to compile code using fresh valac (as it provides some necessary bugfixes) for glib shipped with old ubuntu LTS release (because my clients won't upgrade). And vala should allow doing so. Expectation that all projects will constantly migrate to new version of libraries is naive. In some industries where reliability is a key it is even prohibited. 

Now I have two options: use old Vala, or backport VAPI from old Vala.

The fact that several months has passed does not change anything. In commercial reality I might be locked to *years* old libraries but I still might want to use new compiler. It has nothing to being free or proprierary.

Can you imagine that with new GCC you can compile only new GLib?

Please remind that this does not apply only to GLib. I might need to use any older version of the library.

I just don't think it's possible to maintain VAPI that will match all possible versions, especially with such large libraries as GLib. Wouldn't it be easier to have glib-2.0_2.46.vapi, glib-2.0_2.48.vapi and pick up the right one depending on --target-glib setting?
Comment 7 Al Thomas 2016-08-04 13:26:13 UTC
(In reply to Marcin Lewandowski from comment #6)
> I might want to compile code using fresh valac (as it provides some
> necessary bugfixes) for glib shipped with old ubuntu LTS release (because my
> clients won't upgrade). And vala should allow doing so. 
Vala 0.32 will build projects against GLib 2.32+. See https://git.gnome.org/browse/vala/commit/?id=0b297f4273a868c90b8e96adf52380839fcc38c8 
Please do not exaggerate this one issue in to something it is not.

> The fact that several months has passed does not change anything.
It does, because if it had been identified at the time it could have potentially been resolved before the stable release and so you would not be writing comments like this.

> Please remind that this does not apply only to GLib. I might need to use any
> older version of the library.
Libraries should maintain a stable API. Please focus on the issue related to this bug and understand the argument, that by adding an interface to GSocket this has potentially changed the API. The details are important here if you want to help resolve this issue. Or it could be a problem with vapigen by not including the base class methods.

> Wouldn't it
> be easier to have glib-2.0_2.46.vapi, glib-2.0_2.48.vapi and pick up the
> right one depending on --target-glib setting?
If you think this is a good idea, please gain community consensus for it and then work on patches. At present --target-glib is internal to valac and #if GLIB_2_XX in GLib VAPI. --target-glib does not exclude newly added interfaces when targeting old versions at present, you need to know not to use them.
Comment 8 Marcin Lewandowski 2016-08-04 13:37:12 UTC
(In reply to Al Thomas from comment #7)
> Libraries should maintain a stable API. 

But they not always do. And the question is how to handle this. You can't expect programming world to be ideal. IMO there should be a mechanism to handle that.

> Please focus on the issue related to
> this bug and understand the argument, that by adding an interface to GSocket
> this has potentially changed the API. 

I understand the argument. I see the point. But GLib was already released as it is and current approach to VAPI limits handling such case. It is obviously improper that GLib devs have changed the API but life goes on and IMO Vala should somehow adjust.
Comment 9 Al Thomas 2016-08-04 17:34:08 UTC
FWIW this is the response on IRC #gtk when asking why the GDatagramBased interface is implemented by GSocket:

[16:43]	al	thanks pmike, i dont' think GSocket should implement GDatagramBased interface. Not all sockets are DatagramBased, e.g. TCP.
[16:43]	al	There should be a new object GDatagramBasedSocket that inherits from GSocket and implements GDatagramBased. This will GError if try to create TCP socket with it. The changes to GSocket are causing problems with Vala. The idea of a conditional interface based on constructor argument is dubious in my view
[16:52]	pwithnall	al: It’s not a conditional interface, it’s an interface which will return an error if you call its methods on a stream-based socket
[16:52]	pwithnall	It’s no worse than socket() being a gateway to all sorts of different types of connections with totally different semantics
[16:53]	pwithnall	and I suspect less painful than having a DatagramBased and StreamBased copy of a lot of classes and interfaces
[16:53]	pwithnall	But to answer your original question, as pmike said: you cannot use a stream-based socket with GDtlsServerConnection
[16:54]	pwithnall	It will throw errors as soon as you try sending or receiving data
[16:54]	al	hmm, if it errors then it doesn't implement the method so it is not implementing the interface. my understanding was that GObject brought OOP methods to C. this seems like a corruption of that
[16:55]	al	and makes it difficult for bindings
[16:55]	al	what purpose does the interface serve?

On the Vala side there are two issues to fix at present with the binding for GSocket:

 1. This binding no longer compiles for GLib < 2.48
 2. Methods Socket.receive_messages (), Socket.send_messages (), Socket.create_source (), Socket.condition_check () and Socket.condition_wait () no longer appear to work correctly when used with non-datagram based connections, e.g. TCP, when using GLib 2.48

One solution would be to try is an #if GLIB_2_48 #else #endif around the new and old bindings. Making sure the g_socket_create_source(), etc. are added back in the new binding. This would hopefully work by adding to 
https://git.gnome.org/browse/vala/tree/vapi/metadata/Gio-2.0-custom.vala

Socket, however, is a large class and there are already adjustments made to it in https://git.gnome.org/browse/vala/tree/vapi/metadata/Gio-2.0.metadata

Another solution may be to remove GDatagramBased as an interface for Socket. The interface appears to offer no type safety benefits. Then cast Socket to GDatagramBased in the binding for methods such as DtlsClientConnection.
Comment 10 José Miguel Fonte 2016-08-05 00:19:26 UTC
Just tested changing the vapi file by hand with focus solely on the create_source method as a test case and succeeded. I'm not aware of how the vapi files are generated (automatic generation via vapi-gen etc?) or rules to generate them but seems that something went wrong with the addition of the interface DatagramBased.

On the vala-0.32 vapi file, all the DatagramBased methods are declared 'abstract' and they are not abstract, they have implementation in C, eg: g_socket_create_source vs g_datagram_based_create_source, which means that we can invoke both methods on a instance by direct cast or using the 'as' cast:

(socket as Socket).create_source (...) or socket.create_source (...) vs 
(socket as DatagramBased).create_source (...)

The Socket.create_source/g_socket_create_method signature also changed with the return type being a GLib.Source (0.32  glib 2.48) instead of a GLib.SourceSocket (0.30 glib 2.46).

Not sure how the devs will change this, but as Al has mentioned, there are several topics related with this "bug". 

As a last resort, manually changing the 0.32 vapi file, or using the 0.30 one as a workaround will work but situations may arise where this step which may throw erratic behavior. 

Trying to change the file manually seems more safe but prone to human induced errors.
Comment 11 José Miguel Fonte 2016-08-05 00:25:09 UTC
Forgot to say that 0.32 vapi files do not list the methods mentioned previously by Al and Marcin. They only show on the interface are are declared abstract as mentioned. Manually changing means, adding the methods on th GLib.Socket, check the method prototype/signature and removing abstract keyword from the interface methods.
Comment 12 Al Thomas 2016-08-05 15:01:24 UTC
Compiling the supplied test example with Vala master and GLib 2.46 gives only C compiler errors:

test-example.vala.c:87:45: error: ‘GDatagramBased’ undeclared (first use in this function)
   _tmp8_ = g_datagram_based_create_source ((GDatagramBased*) _tmp7_, G_IO_IN, NULL);
test-example.vala.c:87:60: error: expected expression before ‘)’ token
   _tmp8_ = g_datagram_based_create_source ((GDatagramBased*) _tmp7_, G_IO_IN, NULL);

So Vala is fine compiling the code, it is the symbol resolution from the C compiler. This suggests keeping the interface in 0.32 will still work for Vala compilation with GLib < 2.48. It is the symbols in GLib < 2.48 that don't exist. So long as Vala doesn't generate C code with these C symbols everything should be fine. The resolution could be simply to mark the methods of GDatagramBased interface as virtual instead of abstract. See https://wiki.gnome.org/Projects/Vala/Bindings#Abstract.2BAC8-Virtual_Distinction

If no one else gets there first I may try playing around with this next week. If anyone else wants to try adjusting the metadata and using vapigen to re-build the VAPI, then the Gio-2.0.gir is here: https://github.com/nemequ/vala-girs/blob/master/gir-1.0/Gio-2.0.gir
Comment 13 Michele Dionisio 2016-09-15 22:53:17 UTC
my pull request on this issue:
https://github.com/GNOME/vala/pull/7
Comment 14 Rico Tzschichholz 2016-09-16 18:57:07 UTC
Created attachment 335730 [details] [review]
gio-2.0: Keep GLib.Socket API compatible with gio < 2.48
Comment 15 Al Thomas 2016-09-17 16:29:24 UTC
Can interested parties please test this by using:
https://git.gnome.org/browse/vala/tree/vapi/gio-2.0.vapi
Comment 16 Al Thomas 2016-09-17 17:12:27 UTC
Scrub the last comment, patch has not been applied to master.
Test procedure should be:

git clone git://git.gnome.org/vala
cd vala
curl https://bug769222.bugzilla-attachments.gnome.org/attachment.cgi?id=335730 --output gio-20-Keep-GLibSocket-API-compatible-with-gio--24.patch
git apply gio-20-Keep-GLibSocket-API-compatible-with-gio--24.patch

Then use the new vapi/gio-2.0.vapi by copying it into your VAPI directory
Comment 17 Rico Tzschichholz 2016-09-20 10:31:43 UTC
Created attachment 335917 [details] [review]
gio-2.0: Keep GLib.Socket API compatible with gio < 2.48
Comment 18 Rico Tzschichholz 2016-09-24 07:00:50 UTC
Attachment 335917 [details] pushed as 28dd295 - gio-2.0: Keep GLib.Socket API compatible with gio < 2.48