ei: Fix ei_resolve.c compilation for Android

[JeromeDeBretagne]

Issue detected while trying to cross-compile Erlang/OTP 25.0-rc1 :

 CC	otp_25.0-rc1_android21_arm64_SSL/lib/erl_interface/obj.mt/aarch64-unknown-linux-android/decode_boolean.o
connect/ei_resolve.c:401:3: error: implicit declaration of function 'gethostbyaddr_r' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
  gethostbyaddr_r(addr, length, type, hostp, buffer, buflen, &result,
  ^
connect/ei_resolve.c:401:3: note: did you mean 'gethostbyaddr'?

android-ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/../sysroot/usr/include/netdb.h:211:17: note: 'gethostbyaddr' declared here
struct hostent* gethostbyaddr(const void* __addr, socklen_t __length, int __type);
                ^
1 error generated.

and indeed gethostbyaddr_r was only introduced in Android 6.0 Marshmallow as documented here in the Android NDK:
https://android.googlesource.com/platform/bionic/+/master/docs/status.md

I'm a bit surprised to face this issue only now though, I've never seen this compilation error when cross-compiling Erlang/OTP 24.x with the same toolchain and I don't see which one of the few recent changes could have triggered this "regression" from the commit history : https://github.com/erlang/otp/commits/master/lib/erl_interface/src/connect/ei_resolve.c

Does anyone have a clue?

Thanks, Jérôme

[github-actions]

CT Test Results

    2 files    22 suites   5m 20s ⏱️
142 tests 137 ✔️ 5 💤 0 ❌
165 runs  160 ✔️ 5 💤 0 ❌

Results for commit de1a8e4.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[sverker]

Commits 96e8bea and 8d8ee81 are both suspect, and they are both merged to maint to be released in OTP-24.3.
I suggest you do
git rebase --onto maint master
on a fresh maint branch.
The deadline for 24.3 has passed, but that way the branch could also easily be released in a 24.3.x patch. Keep this PR based on master though.

[JeromeDeBretagne]

I suggest you do git rebase --onto maint master on a fresh maint branch.

I've just done so, and I get the same error: I guess this is what you were expecting, right?

Commits 96e8bea and 8d8ee81 are both suspect

Looking at commit 8d8ee81, it shouldn't have an impact on a cross-build targeting Android.

From commit 96e8bea, I would say that the new section starting on L8094 in lib/erl_interface/configure :

Checking if we can add -Werror=implicit to DED_WERRORFLAGS (via CFLAGS)

seems like a good candidate to explain why I'm getting this error message now. And it seems to be a totally valid error to detect!

[sverker]

I suggest you do git rebase --onto maint master on a fresh maint branch.

I've just done so, and I get the same error: I guess this is what you were expecting, right?

I was expecting a clean maint branch would get the same compile error.
And I was expecting this branch rebased on maint would fix that error. Is it not?

[JeromeDeBretagne]

Maybe I got confused by your suggested tests.

I was expecting a clean maint branch would get the same compile error.

I expect it too, I will test this again, hopefully later today.

And I was expecting this branch rebased on maint would fix that error. Is it not?

What should be the instruction to test this scenario, just to make sure I do it right?

[sverker]

What should be the instruction to test this scenario, just to make sure I do it right?

Ask that guy @JeromeDeBretagne, he pasted the compile error at the top of this PR.

That was a joke sprung out of my confusion.

We don't prioritize Android and we don't have any regression tests at all on Android.
The only testing we will do of this PR is to verify it "looks ok" and does not fail any tests on our existing platforms.

If you keep this PR as-is the fix will only be merged to master for OTP-25.0. But if you rebase the branch on maint and force push it (but keep the PR targeted for master) then the same branch can later also be merged to maint-24 and be released as some OTP-24.3.x version.

[JeromeDeBretagne]

I was expecting a clean maint branch would get the same compile error.

Indeed, I get the same error message on a clean maint branch.

And I was expecting this branch rebased on maint would fix that error. Is it not?

I can confirm this commit does fix the error on the maint branch too.

Ask that guy @JeromeDeBretagne, he pasted the compile error at the top of this PR.
That was a joke sprung out of my confusion.

😆

The only testing we will do of this PR is to verify it "looks ok" and does not fail any tests on our existing platforms.

Sounds good to me 👍

If you keep this PR as-is the fix will only be merged to master for OTP-25.0. But if you rebase the branch on maint and force push it (but keep the PR targeted for master) then the same branch can later also be merged to maint-24 and be released as some OTP-24.3.x version.

I've understood now what you were suggesting to do : rebase this branch on maint and force push done!

[sverker]

Released in OTP 24.3.2.