Skip to content

cmake: fix HAVE_H_ERRNO_ASSIGNABLE detection#12094

Closed
vszakats wants to merge 3 commits intocurl:masterfrom
vszakats:cmake-fix-HAVE_H_ERRNO_ASSIGNABLE-cross
Closed

cmake: fix HAVE_H_ERRNO_ASSIGNABLE detection#12094
vszakats wants to merge 3 commits intocurl:masterfrom
vszakats:cmake-fix-HAVE_H_ERRNO_ASSIGNABLE-cross

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Oct 11, 2023

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

Skip `check_c_source_runs()` call when cross-building.

Regression from 04a3a37 curl#11979

Reported-by: Kartatz on Github
Fixes curl#12093
Closes #xxxxx
@vszakats
Copy link
Member Author

vszakats commented Oct 11, 2023

After re-reading the autotools code and made more tests, I'm attempting
another fix.

It looks like this test code isn't meant to be run; compiling it is enough. This
tested OK on Darwin with the HAVE_H_ERRNO_ASSIGNABLE test compiling
successfully. This also works in cross-builds, just like autotools.

If this works correctly with SerenityOS, we might drop the allowlisting.
(or might as well keep it for build performance reasons, if this test result
is stable across OS versions.)

[ What confused me is that the test returns a meaningful result when run, but
it looks like this isn't actually used or needed. ]

/cc @Kartatz

@Kartatz
Copy link

Kartatz commented Oct 11, 2023

If this works correctly with SerenityOS, we might drop the allowlisting.

This fixes the issue for me.

Both HAVE_H_ERRNO_ASSIGNABLE and HAVE_GETADDRINFO_THREADSAFE tests are being detected correctly.

@vszakats vszakats removed the on-hold label Oct 11, 2023
@vszakats vszakats changed the title cmake: exclude a detection from cross-builds cmake: fix HAVE_GETADDRINFO_THREADSAFE detection Oct 11, 2023
@vszakats vszakats changed the title cmake: fix HAVE_GETADDRINFO_THREADSAFE detection cmake: fix HAVE_H_ERRNO_ASSIGNABLE detection Oct 11, 2023
@vszakats
Copy link
Member Author

Perfect! This made the fix simpler, enabled it for cross-builds and made the allow-list unnecessary.

This is ready to merge.

@vszakats vszakats closed this in 104767a Oct 12, 2023
@vszakats vszakats deleted the cmake-fix-HAVE_H_ERRNO_ASSIGNABLE-cross branch October 12, 2023 06:47
kennethmyhra added a commit to kennethmyhra/serenity that referenced this pull request Oct 16, 2023
According to the issue curl/curl#12093 the
curl build doesn't properly detect that Serenity has getaddrinfo() and
outputs the following:
---
Performing Test HAVE_H_ERRNO_ASSIGNABLE
CMake Error: try_run() invoked in cross-compiling mode, please set the
following cache variables appropriately:
HAVE_H_ERRNO_ASSIGNABLE_EXITCODE (advanced)
---

Setting the CMake cache variable HAVE_GETADDRINFO_THREADSAFE=1 solves
the mentioned error.

Also see: curl/curl#12094
gmta pushed a commit to SerenityOS/serenity that referenced this pull request Oct 16, 2023
According to the issue curl/curl#12093 the
curl build doesn't properly detect that Serenity has getaddrinfo() and
outputs the following:
---
Performing Test HAVE_H_ERRNO_ASSIGNABLE
CMake Error: try_run() invoked in cross-compiling mode, please set the
following cache variables appropriately:
HAVE_H_ERRNO_ASSIGNABLE_EXITCODE (advanced)
---

Setting the CMake cache variable HAVE_GETADDRINFO_THREADSAFE=1 solves
the mentioned error.

Also see: curl/curl#12094
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Cross-compiling to SerenityOS fails

2 participants