Add function channel_of_descr_socket (windows has different handles for sockets and files) #176
Conversation
…or sockets and files)
|
(I don't know if this patch is expected to work outside windows or if it needs some modification) |
|
I would say the main question is if we want separate functions also in Unix, so that one could more easily write portable code. If so, then on Unix the two |
src/glib.ml
Outdated
| @@ -72,6 +72,8 @@ module Io = struct | |||
| type id | |||
| external channel_of_descr : Unix.file_descr -> channel | |||
| = "ml_g_io_channel_unix_new" | |||
| external channel_of_descr_socket : Unix.file_descr -> channel | |||
| = "ml_g_io_channel_unix_new_socket" | |||
There was a problem hiding this comment.
Shouldn't this rather be ml_g_io_channel_win32_new_socket ?
Note that the change is not visible in the interface, so this would not matter for CoqIDE.
There was a problem hiding this comment.
You mean to rename the C function? One can of course do this. The reason I named it this way is that this is still about Windows' emulation of the Unix socket system. But I have no strong feelings about the naming.
|
I've changed it quite a lot. Basically, since OCaml uses a common tagged data structure for sockets and handles, there is no need to have separate functions. |
|
I don't have a windows to test either |
|
@garrigue : I can test it. Are we talking about the PR branch or can I find the code somewhere else? |
|
I pushed the code to the pr branch. |
|
OK, give me a few days. I put it on my list for Friday. |
|
Sorry, I had to delay it to Monday, but it is still on my list ... |
|
Not sure what is going on, but I can't get the patch applied by opam. If I run I downloaded https://patch-diff.githubusercontent.com/raw/garrigue/lablgtk/pull/176.patch, added it to the opam file in the usual way (there was already one in my local patch repo - I just had to change the name). All looks good, no errors, opam lint says all is good, but the patch is not done. If I modify the name of the patch file, opam complains that the patch file is not there (as it should). Also my original patch works. But if I put in the patch of this PR nothing happens. I have not the slightest idea what is going on here. It might take a bit ... |
|
I think I found it - the patch produced by github (see link above) contains several separate patches for separate commits (4 in total) - apparently opam / patch does not support this. |
|
I split the patch file into 4 and added all 4 to opam - this also doesn't work (no errors, no patch applied). Can someone please squash this PR - maybe this fixes it. |
|
I don't think squashing would change the patch. |
|
Also why test with a patch instead of pointing the url at my branch? |
It is Coq Platform policy to only use official releases + patches which are kept directly in the Coq Platform repo. But this can be handled - I will just clone the repo (I am lazy and didn't do sp as yet) and I am sure I find a way to create the patch - worst case I put both versions in separate folders ... |
|
I squashed it in a local branch and created a patch from that - this did work. One obviously needs to adjust CoqIDE then - I guess we do this from Coq 8.20 on. We need to take care to link it to the proper lablgtk versions in opam then. I did a quick test of the resulting CoqIDE and it works fine. => good to go. |
That's what
is about no? |
Yes. I am not sure we want this, though. It is for sure cleaner to get rid of it and assuming everyone is using opam it should not be a problem to manage the version restrictions. But I have no strong feelings about this - adding the old function name as an alias would also be an option. |
|
Thinking about it: one would have to add an upper bound for the lablgtk version for all older versions of CoqIDE. I still would prefer this solution, but again no strong feelings. |
|
So is this going to be merged? |
|
Sorry the long delay, I got to do other things and forgot. |
CHANGES: 2024.06.10 [Jacques] * Prepare for release * Fix typos in README.md (garrigue/lablgtk#182) [Sylvain Chiron] 2024.05.20 [Jacques] * Typo in ml_gtk_text_buffer_get_iter_at_line_index. (garrigue/lablgtk#181) [Hugo Herbelin] 2024.05.14 [Jacques] * Support windows sockets in Glib.IO.channel_of_descr (garrigue/lablgtk#176) [with Michael Soegtrop] 2024.03.24 [Jacques] * fix pointer incompatibility in ml_gtk_style_context_list_classes (garrigue/lablgtk#178)
CHANGES: 2024.06.10 [Jacques] * Prepare for release * Fix typos in README.md (garrigue/lablgtk#182) [Sylvain Chiron] 2024.05.20 [Jacques] * Typo in ml_gtk_text_buffer_get_iter_at_line_index. (garrigue/lablgtk#181) [Hugo Herbelin] 2024.05.14 [Jacques] * Support windows sockets in Glib.IO.channel_of_descr (garrigue/lablgtk#176) [with Michael Soegtrop] 2024.03.24 [Jacques] * fix pointer incompatibility in ml_gtk_style_context_list_classes (garrigue/lablgtk#178)
This PR upstreams a patch which was carried in windows opam repos, eg https://github.com/coq/platform/blob/main/opam/opam-repository/packages/lablgtk3/lablgtk3.3.1.3/files/0001-Add-function-channel_of_descr_socket-windows-has-dif.patch
See also coq/coq#18607