Bump Android minimum supported API to 26 (Android O) [crashpad/crashpad : main]
Andrew Grieve (Gerrit)
Andrew Grieve added 1 comment![]( "Open in Gerrit")
Looks like there are no android trybots, but I did at least build all targets locally.
Related details
- Mark Mentovai
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
Mark Mentovai added 8 comments![]( "Open in Gerrit")
#if __ANDROID_MIN_SDK_VERSION__ < 26Do you need to `#include` something for this?
#error "Crashpad requires Android API 26 or above."You don’t need the "quotes".
#if __ANDROID_MIN_SDK_VERSION__ < 26
#error "Crashpad requires Android API 26 or above."
#endifActually, I don’t think we need this at all.
We don’t do this for other OS platforms, we didn’t do it for Android before this, and this seems like a weird place to put it. Chances are decent that if someone tries building with an older version, they won’t even see this, because enough other things will break that this message will either be buried or the build won’t even have progressed to this file so it won’t even appear at all.
"android/sys/user.h",I think we only have this because the NDK’s `<sys/user.h>` was broken and needed something else to be `#include`d first. Is that still the case? If so, can we ask the NDK to be fixed?
#if !defined(ELF32_ST_VISIBILITY)
#define ELF32_ST_VISIBILITY(other) ((other) & 0x3)
#endif
#if !defined(ELF64_ST_VISIBILITY)
#define ELF64_ST_VISIBILITY(other) ELF32_ST_VISIBILITY(other)
#endifNDK still doesn’t have these? Can we ask for them to be added?
#include <android/api-level.h>No longer used.
32-bit platforms. See Chrome’s
[`build/config/android/config.gni`](https://chromium.googlesource.com/chromium/src/+/main/build/config/android/config.gni)
which sets `android32_ndk_api_level` and `android64_ndk_api_level`.I think that this part of the comment, which tells people where to find a good value for the current API level, is still helpful.
android_api_level = 21No need to specify manually anymore?
Related details
- Andrew Grieve
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
Mark Mentovai added 3 comments![]( "Open in Gerrit")
Note: Chrome's minSdkVersion is Q right now, so bumping to O seemsGive the number that corresponds to Q also.
Also include a note that you’re updating mini_chromium.
Bug: chromium:425948259, chromium:40875565chromium: tags are no longer needed, because the bug namespace has been merged.
Related details
- Andrew Grieve
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Grieve (Gerrit)
Andrew Grieve voted and added 11 comments![]( "Open in Gerrit")
Votes added by Andrew Grieve
| Commit-Queue | +1 |
11 comments
Note: Chrome's minSdkVersion is Q right now, so bumping to O seemsGive the number that corresponds to Q also.
Done
Also include a note that you’re updating mini_chromium.
Done
chromium: tags are no longer needed, because the bug namespace has been merged.
Done
Do you need to `#include` something for this?
No, the compiler set it.
You don’t need the "quotes".
Removed.
#if __ANDROID_MIN_SDK_VERSION__ < 26
#error "Crashpad requires Android API 26 or above."
#endifActually, I don’t think we need this at all.
We don’t do this for other OS platforms, we didn’t do it for Android before this, and this seems like a weird place to put it. Chances are decent that if someone tries building with an older version, they won’t even see this, because enough other things will break that this message will either be buried or the build won’t even have progressed to this file so it won’t even appear at all.
Removed.
"android/sys/user.h",I think we only have this because the NDK’s `<sys/user.h>` was broken and needed something else to be `#include`d first. Is that still the case? If so, can we ask the NDK to be fixed?
Seems to work without it. Deleted!
#if !defined(ELF32_ST_VISIBILITY)
#define ELF32_ST_VISIBILITY(other) ((other) & 0x3)
#endif
#if !defined(ELF64_ST_VISIBILITY)
#define ELF64_ST_VISIBILITY(other) ELF32_ST_VISIBILITY(other)
#endifNDK still doesn’t have these? Can we ask for them to be added?
Filed a bug and added a comment with a link to it.
#include <android/api-level.h>Andrew GrieveNo longer used.
Done
32-bit platforms. See Chrome’s
[`build/config/android/config.gni`](https://chromium.googlesource.com/chromium/src/+/main/build/config/android/config.gni)
which sets `android32_ndk_api_level` and `android64_ndk_api_level`.I think that this part of the comment, which tells people where to find a good value for the current API level, is still helpful.
Done
android_api_level = 21No need to specify manually anymore?
- Mark Mentovai
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Grieve (Gerrit)
Message from Andrew Grieve![]( "Open in Gerrit")
- Andrew Grieve
- Mark Mentovai
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Grieve (Gerrit)
Andrew Grieve voted and added 1 comment![]( "Open in Gerrit")
Votes added by Andrew Grieve
| Commit-Queue | +1 |
1 comment
Looks like mini_chromium was rolled separately, so removed from CL.
Related details
- Mark Mentovai
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
Mark Mentovai voted Code-Review+1![]( "Open in Gerrit")
- Andrew Grieve
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Grieve (Gerrit)
Andrew Grieve voted Commit-Queue+2![]( "Open in Gerrit")
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
crashpad-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
crashpa...@luci-project-accounts.iam.gserviceaccount.com submitted the change![]( "Open in Gerrit")
Change information
Bump Android minimum supported API to 26 (Android O)
Chrome's minSdkVersion is Q (API 29) right now, so bumping to O seems
reasonable.
- M client/crashpad_client_linux_test.cc
- M compat/BUILD.gn
- D compat/android/dlfcn_internal.cc
- D compat/android/dlfcn_internal.h
- M compat/android/elf.h
- D compat/android/linux/elf.h
- D compat/android/linux/prctl.h
- D compat/android/linux/ptrace.h
- D compat/android/sched.h
- D compat/android/sys/epoll.cc
- D compat/android/sys/epoll.h
- D compat/android/sys/mman.h
- D compat/android/sys/mman_mmap.cc
- D compat/android/sys/syscall.h
- D compat/android/sys/user.h
- M doc/developing.md
- M handler/linux/exception_handler_server_test.cc
- M snapshot/linux/process_reader_linux_test.cc
- M test/scoped_set_thread_name_posix.cc
- M util/linux/exception_handler_client.cc
- M util/linux/initial_signal_dispositions.cc
- M util/linux/thread_info.h
Code-Review: +1 by Mark Mentovai
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |