Fixed IPv6 SocketFamily with wrong value on MacOS and Windows

xanathar commented 3 years ago

This is a fix for https://github.com/gtk-rs/sys/issues/178 .

Added a "manual" override for Gio.SocketFamily in conf/gir-gio.toml
Added the appropriate values (using libc) in gio-sys/src/manual.rs. The values for AF_INVALID and AF_UNIX are specified as integers as the corresponding values are not exported as constants in libc; that said, I've checked and their values are 0 and 1 in Windows, MacOS and Linux and, I presume, pretty much every other OS being the two earliest values
Regenerated gio-sys/src/lib.rs accordingly (also included in the PR)

xanathar commented 3 years ago

Converted to draft after CI failed.

xanathar commented 3 years ago

I think I fixed the build of this (I hoped I could get away without setting up a windows machine with gvsbuild but... no I couldn't :) ).

Now the CI failing in code which is outside of the scope of this PR (apparently glib::timeout_add_local changed from taking an integer to std::time::Duration); is this something known or a weird side effect of the PR changes?

sdroege commented 3 years ago

Now the CI failing in code which is outside of the scope of this PR (apparently glib::timeout_add_local changed from taking an integer to std::time::Duration); is this something known or a weird side effect of the PR changes?

That's fixed by https://github.com/gtk-rs/examples/pull/325 . Sorry for that!

xanathar commented 3 years ago

Cool thank you. Tomorrow I'll rebase, add the other two enums and undraft it.

sdroege commented 3 years ago

Great, thanks for your work on this :)

xanathar commented 3 years ago

I checked the sources of gioenums.h and I'm not so convinced anymore that moving (all) the other enums is a good idea (or, at the very least, every single case seems to have it's own nuances :( ).

Below is a summary of socket related enums, but TL;DR:

GSocketFamily needs to be fixed because AF_INET6 is different from OS to OS. This PR now has a manual override for this.
GSocketMsgFlags can be ported to use platform dependent constants, even though all the OSs use the same values for those 4 ints. This PR now has a manual override for this.
GSocketProtocol and GSocketType are probably best left untouched; details below.

If the CI says its "ok", I'll undraft this.

GSocketFamily

GSocketFamily is clearly defined to use system values, so I think what we've done so far is good:

typedef enum {
  G_SOCKET_FAMILY_INVALID,
  G_SOCKET_FAMILY_UNIX = GLIB_SYSDEF_AF_UNIX,
  G_SOCKET_FAMILY_IPV4 = GLIB_SYSDEF_AF_INET,
  G_SOCKET_FAMILY_IPV6 = GLIB_SYSDEF_AF_INET6
} GSocketFamily;

GSocketMsgFlags

GSocketMsgFlags is another case where there are platform specific values:

typedef enum /*< flags >*/
{
  G_SOCKET_MSG_NONE,
  G_SOCKET_MSG_OOB = GLIB_SYSDEF_MSG_OOB,
  G_SOCKET_MSG_PEEK = GLIB_SYSDEF_MSG_PEEK,
  G_SOCKET_MSG_DONTROUTE = GLIB_SYSDEF_MSG_DONTROUTE
} GSocketMsgFlags;

So I added GSocketMsgFlags to the PR.

GSocketType

GSocketType is defined, on the other hand, to use consecutive values, and there are a few switch statements in gsocket.c on the specific values so it's better left as is:

typedef enum
{
  G_SOCKET_TYPE_INVALID,
  G_SOCKET_TYPE_STREAM,
  G_SOCKET_TYPE_DATAGRAM,
  G_SOCKET_TYPE_SEQPACKET
} GSocketType;

GSocketProtocol

Finally, GSocketProtocol is a mixed bag; it uses hardwired values in gioenums.h which are afaict technically platform specific values; apparently, though, all the platforms I have checked (Linux, Windows, MacOS and FreeBSD) have the same values, so glib gets away with it. There are no switch statements, but the hardwired values are used when dispatching G_SOCKET_PROTOCOL_DEFAULT. I'd rather leave them with the numeric values they got from parsing gioenums.h.

typedef enum {
  G_SOCKET_PROTOCOL_UNKNOWN = -1,
  G_SOCKET_PROTOCOL_DEFAULT = 0,
  G_SOCKET_PROTOCOL_TCP     = 6,
  G_SOCKET_PROTOCOL_UDP     = 17,
  G_SOCKET_PROTOCOL_SCTP    = 132
} GSocketProtocol;

xanathar commented 3 years ago

On the two CI failures:

One is due to make making automated modifications to Cargo.toml files. Most of them are added/removed newlines, but one is odd:

diff --git a/gdkx11-sys/Cargo.toml b/gdkx11-sys/Cargo.toml
index 427c997..ff5c931 100644
--- a/gdkx11-sys/Cargo.toml
+++ b/gdkx11-sys/Cargo.toml
@@ -62,4 +62,4 @@ tempfile = "3"
 v3_16 = []
 v3_24 = ["v3_16"]
 v3_24_2 = ["v3_24", "cairo"]
-dox = ["v3_24_2", "cairo"]
+dox = []

I can definitely commit the autogenerated Cargo.toml files, but I am bit worried by the above change (I lack the context to evaluate it properly).

The other CI failure might have been an http transfer error in curl? The file size is wildly different when I try the same commands locally:

$ curl -LO https://github.com/EPashkin/gtk-bootstrap/releases/download/gtk-3.24.0-1/deps.txz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   634  100   634    0     0    762      0 --:--:-- --:--:-- --:--:--   762
100 33.9M  100 33.9M    0     0   355k      0  0:01:37  0:01:37 --:--:--  361k

sdroege commented 3 years ago

Below is a summary of socket related enums, but TL;DR:

I agree with that, thanks for investigating :tada:

sdroege commented 3 years ago

On the two CI failures:

These also fail in master apparently, so unrelated to what you did here and need to be investigated independently :)

GuillaumeGomez commented 3 years ago

I think a fix on the toml-rs crate was released, which now adds the previously missing empty lines. If so, that'd be so cool!

GuillaumeGomez commented 3 years ago

Merging then. Thanks!

xanathar commented 3 years ago

Thanks to you both!

sdroege commented 3 years ago

I'll do a bugfix release for this later/tomorrow

gtk-rs / sys