Return ENOSYS for posix_spawn_file_actions_addchdir on older Android versions #985
Conversation
…versions that do not have it
|
Thoughts, @finagolfin and @jakepetroules? |
|
@swift-ci test |
|
@swift-ci please smoke test |
lib/Basic/Subprocess.cpp
Outdated
| @@ -102,6 +102,9 @@ static int posix_spawn_file_actions_addchdir_polyfill(posix_spawn_file_actions_t | |||
| // - FreeBSD 13.1 (May 2022) | |||
| // - Android 14 (October 2023) | |||
| return posix_spawn_file_actions_addchdir_np((posix_spawn_file_actions_t *)file_actions, path); | |||
| #elif defined(__ANDROID__) | |||
There was a problem hiding this comment.
Actually, would you mind moving this up below the glibc case at the top, and write like:
#elif defined(__ANDROID__) && __ANDROID_API__ < 34...to keep the "missing" cases consolidated at the top?
There was a problem hiding this comment.
Actually, would you mind moving this up below the glibc case at the top
Done!
There was a problem hiding this comment.
You missed moving it up. I'd also prefer if you combined it with the glibc and OpenBSD cases, since all return the same error.
You can also remove checking the API level is greater than 34 for the affirmative case above.
There was a problem hiding this comment.
I'd also prefer if you combined it with the glibc and OpenBSD cases, since all return the same error.
You mean like this? It seems rather tortuous…
#if (defined(__GLIBC__) && !__GLIBC_PREREQ(2, 29)) || (defined(__OpenBSD__)) || (defined(__ANDROID__) && __ANDROID_API__ < 34)
return ENOSYS;
There was a problem hiding this comment.
Yes, either way, Jake asked to move it up above. I prefer it combined like that too.
…wn_file_actions_addchdir_polyfill
| // Glibc versions prior to 2.29 don't support posix_spawn_file_actions_addchdir_np, impacting: | ||
| // - Amazon Linux 2 (EoL mid-2025) | ||
| // Currently missing as of: | ||
| // - OpenBSD 7.5 (April 2024) | ||
| // - Android <= 14 |
There was a problem hiding this comment.
While you're here, could you move the QNX 8 comment here too?. The function is actually missing on that platform. It is mistakenly labeled as supported due to an error in earlier refactoring.
There was a problem hiding this comment.
I assume you also want to add a check for defined(__QNX__) for that block? I've moved the comment and added the check in 2cfc9eb
lib/Basic/Subprocess.cpp
Outdated
| #elif defined(__OpenBSD__) | ||
| // Currently missing as of: | ||
| // - OpenBSD 7.5 (April 2024) | ||
| return ENOSYS; | ||
| #elif defined(__GLIBC__) || defined(__APPLE__) || defined(__FreeBSD__) || (defined(__ANDROID__) && __ANDROID_API__ >= 34) || defined(__musl__) |
There was a problem hiding this comment.
The ANDROID_API >= 34 bit is no longer needed.
|
@swift-ci please smoke test |
|
@swift-ci test linux |
|
@swift-ci smoke test linux |
|
@swift-ci smoke test windows |
|
Marc, please submit to 6.2 next, doesn't look like there's an auto-merger for this repo. |
This fixes the Android build error introduced in 9087bdf.
According to https://android.googlesource.com/platform/bionic/+/master/docs/status.md,
posix_spawn_file_actions_addchdir_npwas only added in API level 34. There seems to be no equivalent for earlier versions, so we just returnENOSYSfor lower API levels (such as API level 29, which the Android SDK is targeting).