Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #2255: complete socket.h; simplify & shorten code paths #2766

Conversation

LeeTibbert
Copy link
Contributor

@LeeTibbert LeeTibbert commented Aug 3, 2022

This PR brings socket.scala into nearly complete compliance with POSIX 2018 specification socket.h.
This implies implementing the sendmsg and recvmsg methods and creating MsgIoSocketTest.scala
to exercise them.

In addition, a number of socket code paths were simplified and shortened.

A number of items, including sockatmark(), socketpair(), and the CMSG* & SHUT_* macros have no direct
correspondents on Windows. A reasonable attempt was made to provide "stub" error values. A experienced
Windows developer could probably improve those shots-in-the-dark.

@LeeTibbert
Copy link
Contributor Author

This is ready for review but not merge.

I say "not merge" because the apparently unrelated CI failures
do not provide confidence.

The CI environment appears to be having problems downloading
Clang and/or llvm.

I will do a dummy commit by the light of day and after Seattle has
woken up. Hopefully, somebody at GitHub or the repository used
to provide Clang will fix the availability.

I will submit

@LeeTibbert
Copy link
Contributor Author

@WojciechMazur Sorry to interrupt, thought I might save you some time, when you get a chance
to look at SN. (Thank you for all the recent merges). It is August in Europe, and I know that
almost everybody is on holiday.

I think the underlying problem Clang/LLVM for at least multiarch is that llvm.org seems to
have gone to (unreleased) LLVM 15 as stable. At least that is what the build environment
is pulling down. It then executes an update-alternative, which, understandably, fails:

2022-08-03T13:06:36.8508276Z �[91mupdate-alternatives: error: alternative path /usr/bin/clang-14 doesn't exist
2022-08-03T13:06:36.9898125Z The command '/bin/sh -c update-alternatives --install /usr/bin/clang clang /usr/bin/clang-14 100' returned a non-zero code: 2

I suspect something similar is happening in the macOS environment. It is tricky to capture the failure
there. Almost always I just get the informative "Cancelled".

I hope this helps, when you get a chance, and saves you some time and life energy.

I have plenty of other SN work to do, so a broken CI gives me a good excuse
to take August off also.

@WojciechMazur
Copy link
Contributor

@LeeTibbert I've finished my summer break yesterday, I'll try to fix to CI this week, as also gives me a headache

@@ -64,7 +64,7 @@ object socket {
sa_family_t, // ss_family, // ss_family, sa_len is synthesized if needed
CUnsignedShort, // __opaquePadTo32
CUnsignedInt, // opaque, __opaquePadTo64
CArray[CUnsignedLongLong, _15] // __opaqueAlignStructure to 8 bytes
CArray[CUnsignedLongLong, _15] // __opaqueAlignStructure to 8 bytes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the inscrutable single character change with the even more
inscrutable commit message.

Yes, the change was needed, but, you are right, the need was not obvious
in the text.

CI was giving me problems. IIRC, these were later determined
to be changes in the CI environment itself.

I know of no way for a plebe such as myself to restart CI.
I hate doing forced git pushes on code which has been
out in the world, so I resorted to the indeed inscrutable
single character change.

Sorry if my opacity causes you to spend your limited time.

This was referenced Aug 28, 2022
@WojciechMazur WojciechMazur merged commit 7c947a5 into scala-native:main Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants