Skip to content

cmake: SerenityOS has thread-safe getaddrinfo#12095

Closed
vszakats wants to merge 3 commits intocurl:masterfrom
vszakats:cmake-serenity-getaddr-threadsafe
Closed

cmake: SerenityOS has thread-safe getaddrinfo#12095
vszakats wants to merge 3 commits intocurl:masterfrom
vszakats:cmake-serenity-getaddr-threadsafe

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Oct 11, 2023 •
edited
Loading

Normal detection fails. This patch opts in SerenityOS into
HAVE_GETADDRINFO_THREADSAFE with CMake.

Delete this once detection is fixed.

Ref: #12093 (comment)
Ref: #12093 (comment)
Assisted-by: Kartatz on Github
Closes #12095

@vszakats
cmake: SerenityOS has thread-safe getaddrinfo
50cf93b 
Normal detection fails. This patch opts in SerenityOS into
`HAVE_GETADDRINFO_THREADSAFE`.

Ref: curl#12093 (comment)
Ref: curl#12093 (comment)
Assisted-by: Kartatz on Github
Closes #xxxxx
@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023 •
edited
Loading

Also added it to autotools also, based on this output:

-- Check for working C compiler: /home/kartatz/senna/bin/x86_64-unknown-serenity-gcc - skipped

Ref: #12093 (comment)

and assuming the host_os value autotools is checking is supposed to match the triplet above.

I haven't tested this, please let me know if this correct or not, and if we should drop it or keep it. (It's also possible the autotools detection correctly get this already.)

/cc @Kartatz

@vszakats vszakats changed the title cmake: SerenityOS has thread-safe getaddrinfo build: SerenityOS has thread-safe getaddrinfo Oct 11, 2023
@vszakats vszakats mentioned this pull request Oct 11, 2023
Closed
@Kartatz
Copy link

Kartatz commented Oct 11, 2023 •
edited
Loading

I did some tests and it seems that the autoconf script already detects this correctly.

I added these lines to lib/hostip4.c:

#if defined(HAVE_GETADDRINFO_THREADSAFE)
	#warning "getaddrinfo is thread-safe"
#else
	#error "getaddrinfo is not thread-safe"
#endif

Then tried to cross-compile:

$ ./configure --without-ssl --enable-http --host=x86_64-unknown-serenity
$ make
Making all in lib
make[1]: Entering directory '/home/kartatz/curl/lib'
make  all-am
make[2]: Entering directory '/home/kartatz/curl/lib'
  CC       libcurl_la-hostip4.lo
hostip4.c:303:10: warning: #warning "getaddrinfo is thread-safe" [-Wcpp]
  303 |         #warning "getaddrinfo is thread-safe"
      |          ^~~~~~~
  CCLD     libcurl.la
make[2]: Leaving directory '/home/kartatz/curl/lib'
make[1]: Leaving directory '/home/kartatz/curl/lib'
Making all in src
make[1]: Entering directory '/home/kartatz/curl/src'
Making all in ../docs
make[2]: Entering directory '/home/kartatz/curl/docs'
Making all in .
make[3]: Entering directory '/home/kartatz/curl/docs'
make[3]: Nothing to be done for 'all-am'.
make[3]: Leaving directory '/home/kartatz/curl/docs'
Making all in cmdline-opts
make[3]: Entering directory '/home/kartatz/curl/docs/cmdline-opts'
make[3]: Nothing to be done for 'all'.
make[3]: Leaving directory '/home/kartatz/curl/docs/cmdline-opts'
make[2]: Leaving directory '/home/kartatz/curl/docs'
make[2]: Entering directory '/home/kartatz/curl/src'
  CCLD     curl
make[2]: Leaving directory '/home/kartatz/curl/src'
make[1]: Leaving directory '/home/kartatz/curl/src'
make[1]: Entering directory '/home/kartatz/curl'
make[1]: Nothing to be done for 'all-am'.
make[1]: Leaving directory '/home/kartatz/curl'

So it seems to be working properly.

@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023

Thanks for your tests. I'm leaving autotools as-is then.

Another interesting bit is why CMake detection fails. I must have botched copying the logic from autotools to Cmake.

@vszakats vszakats changed the title build: SerenityOS has thread-safe getaddrinfo cmake: SerenityOS has thread-safe getaddrinfo Oct 11, 2023
@vszakats vszakats mentioned this pull request Oct 11, 2023
Closed
@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023

Closing in favour of #12094.

@vszakats vszakats closed this Oct 11, 2023
@vszakats vszakats deleted the cmake-serenity-getaddr-threadsafe branch October 11, 2023 23:40
vszakats added a commit that referenced this pull request Oct 12, 2023
@vszakats
cmake: fix HAVE_H_ERRNO_ASSIGNABLE detection
104767a 
Fix `HAVE_H_ERRNO_ASSIGNABLE` to not run, only compile its test snippet,
aligning this with autotools. This fixes an error when doing
cross-builds and also actually detects this feature. It affected systems
not allowlisted into this, e.g. SerenityOS.

We used this detection result to enable `HAVE_GETADDRINFO_THREADSAFE`.

Follow-up to 04a3a37 #11979
Ref: #12095 (closed in favour of this patch)
Ref: #11964 (effort to sync cmake detections with autotools)

Reported-by: Kartatz on Github
Assisted-by: Kartatz on Github
Fixes #12093
Closes #12094
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

build cmake on-hold

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vszakats @Kartatz