Early fixes for MinGW build - draft PR
Hello, Worked some more on porting. Got rid of all but one compile warning for MinGW. update: fixed.
Here's the remaining warning:
src/internal_modules/roc_address/target_berkley/roc_address/socket_addr.cpp: In member function ‘bool roc::address::SocketAddr::is_multicast() const’:
src/internal_modules/roc_address/target_berkley/roc_address/socket_addr.cpp:150:16: warning: conversion to ‘long unsigned int’ from ‘long int’ may change the sign of the result [-Wsign-conversion]
150 | return IN_MULTICAST(core::ntoh32u(saddr_.addr4.sin_addr.s_addr));
| ^~~~~~~~~~~~
Tried about any kind of casts to no avail, and was surprised to see it was the other way around, as ntohs uint32_t return that would be cast to signed in the macro, also probably the macro expansion made the compiler point to "function" macro instead of parameter, but same attempts with casts of parameter in in function param were also no avail. Same as some attempts as replacing with !=0 and similar.
Now i suppose i i'd have only this one to be solved to consider those modifications to wothy of merge? Marking it as ready for review, and will start working on (supposed agree on priorities for MinGW32/Windows):
- (2) implement WIN32 native functions
- enable OpenFEC (well, not that a priority but as it's a single line of code away...)
- build roc-recv/roc-send tools
gh-292
Ok, i think it's ready for review, removing the 'draft mark'...
With all of those (plus copy of some files from posix) it successfully generates libroc_*.a:
- (generic, all builds) new URL for libatomic_ops releases
- socket_addr & socket_ops modifications
- include errno_to_str.h from wav_sink/source.cpp
- minor fixes in windows cond.cpp and errno_to_str (local var name mismatch, and const attribute conflicting)
- getting MinGW build to compile without -fpermissive: https://github.com/roc-streaming/roc-toolkit/issues/292#issuecomment-2985951346
- doing the socket_init/deinit() in Context c/dtor
Meanwhile i'll try to static-link this libroc_core.a in Windows roc-send, and try to cross-compile roc-recv/send to see if it works. Then i think as next priorities:
- clear the last few remaining warnings when compiling for MinGW
- working on item (2) porting posix code to native Windows API
:robot: Pull request description does not have a link to an issue. If there is a related issue, please add it to the description using any of the supported formats.
Ok, learning to use PR's, i believed it would contain only the commit for the URL, but looks like the PR includes all commits including ones added later, so it contains also the socket_addr and _ops which i intended to put on a separate PR...
Now i get all CI tests pass, i'll continue working on local machine, as i suppose for incremental approach you prefer that i don't push anymore stuff into this PR yet, and wait for this one to be reviewed and maybe fixed some more.
Hi, thanks for update!
I'll checkout changes next week, here are just 2 small remarks:
-
Current CI failure is unrelated to PR, you can rebase on fresh develop to fix it.
-
Seems that troubles with formatting are caused by this poor script, not by clang-format ("scons fmt" invokes both things).
I always wanted to rewrite it, so I did it now: 4d4abbbe67131069d2f5442d779276a6b1fc2be7
I hope rebasing on develop will fix your issues and you won't need dummy comments.
If I'm wrong and some problems are caused by clang-format, you can also use clang-format magic comment:
// clang-format off <something> // clang-format on
and once i've got it right, i think you'll agree it's better that i do another PR that would be equivalent in files modified but without extra commits like fixing things in previous ones to avoid cluttering branches with those ("playing back" the final modifications, and hopefully have all intermediate commits validate CI checks).
I'm fine with this approach, just FYI:
-
When merging PR, github allows to squash all commits into one, so it's not really necessary to create a separate PR just for squashing.
(Actually I use my own script for merging PRs, but it behaves the same way).
-
If you force-push to your PR's branch, github will automatically update PR, so it's also not necessary to create a new PR if you want to edit history by hand.
(Github PR is tied to fork name + branch name, not specific commit).
Hello, Worked some more on porting. Got rid of all but one compile warning for MinGW.
Here's the remaining warning:
src/internal_modules/roc_address/target_berkley/roc_address/socket_addr.cpp: In member function ‘bool roc::address::SocketAddr::is_multicast() const’:
src/internal_modules/roc_address/target_berkley/roc_address/socket_addr.cpp:150:16: warning: conversion to ‘long unsigned int’ from ‘long int’ may change the sign of the result [-Wsign-conversion]
150 | return IN_MULTICAST(core::ntoh32u(saddr_.addr4.sin_addr.s_addr));
| ^~~~~~~~~~~~
Tried about any kind of casts to no avail, and was surprised to see it was the other way around, as ntohs uint32_t return that would be cast to signed in the macro, also probably the macro expansion made the compiler point to "function" macro instead of parameter, but same attempts with casts of parameter in in function param were also no avail. Same as some attempts as replacing with !=0 and similar.
Now i suppose i i'd have only this one to be solved to consider those modifications to wothy of merge? Marking it as ready for review, and will start working on (supposed agree on priorities for MinGW32/Windows):
- (2) implement WIN32 native functions
- enable OpenFEC (well, not that a priority but as it's a single line of code away...)
- build roc-recv/roc-send tools