Upgrade to platform-tools-34.0.5#132
Conversation
* adb/0024-Add-explicit-constructor-for-fdevent_event.patch https://android.googlesource.com/platform/packages/modules/adb.git/+/c7bb7813905b5f879dc4c30cac1ee2a64852b8da * base/0004-Move-Theme-Entry-type-definition-up-before-it-is-use.patch https://android.googlesource.com/platform/frameworks/base/+/e8e793a340c87d650a73a29d7915f12e1dc6146b * core/0010-Correct-version-in-which-gettid-was-introduced-to-gl.patch https://android.googlesource.com/platform/system/core.git/+/253445ce3ad507f41c61ebf0f829f75ee2c37509 https://android.googlesource.com/platform/system/core.git/+/8b0160868eefc2997155906aa361470fb9a6c9ab * libziparchive/0002-fix-cxx-narrowing-error.patch https://android.googlesource.com/platform/system/libziparchive/+/035f8d302af88ae2d866c84f5a1528e098e3d1a0
|
Macos checks fail because it looks like this functionality need to be disabled at macos platform. |
|
Alpine GCC fails with Alpine uses musl library. Looking at the musl sourcecode here https://git.musl-libc.org/cgit/musl/tree/include/sys/types.h I see that off64_t requires |
I am not sure if that will cause any further issue. Should it be enabled for all architectures and all libc? |
There should not be. It is a compile define that enables a few extra functionalities. In fact we already enabled the 64bit file functions here b7ab657 but musl uses a define with different name for some reason. I think it will be safe to add the required define in addition to what we already have in vendor/CMakeLists.txt |
This fixes the following error in Alpine linux. In file included from /andy/vendor/libbase/abi_compatibility.cpp:20: /andy/vendor/libbase/include/android-base/file.h:105:71: error: 'off64_t' has not been declared 105 | bool ReadFullyAtOffset(borrowed_fd fd, void* data, size_t byte_count, off64_t offset); | ^~~~~~~
This is disabled due to following compiler error.
core/libcutils/include/private/fs_config.h:26:10: fatal error: 'linux/capability.h' file not found
include <linux/capability.h>
^~~~~~~~~~~~~~~~~~~~
|
Now only the clang errors remain. |
|
The build problems apparently only occur with more recent clang versions. I can't contribute directly to the solution at the moment, but if this is the stopper for the release, I would suggest temporarily disabling clang builds. The package managers of the distris can switch to gcc for the time being. |
It is not required to disable anything in this repository. The CI would be red only. |
|
Disabling clang is a big restriction. I would like to understand better what is going on with the compiler and fix/workaround the problem if possible. I tried to reproduce the problem with godbolt but |
|
One might try to revert this upstream commit and see if it makes clang happy https://android.googlesource.com/platform/packages/modules/adb/+/45720fdaea751c63081541b6c18ab512becd32a4%5E%21/ |
Nope! clang becomes angry again after reverting that commit |
|
I also looked at the flags used for compilation here https://android.googlesource.com/platform/packages/modules/adb/+/refs/heads/main/Android.bp The only unusual item I spotted is use of |
The vendor/cmakelists.txt sets default C++ 20 standard. I have tried with C++ 23 (gnu++2b) in that cmake file but got same compiler error in CI. clang 16 does not have |
|
Here is a repro case https://godbolt.org/z/WGdq9jfsd If flag I tried to set So one solution is to use the same stdlib as AOSP in case if clang+*nux is used. We do not need this flag for clang+macos. |
|
And the whole reprocase is reduced to #include <string.h>
#include <algorithm>
#include <memory>
#include <type_traits>
#include <utility>
#include <vector>
struct Block {
std::unique_ptr<char[]> data_;
};
static_assert(std::is_standard_layout<Block>());The default PS it could be related to this GCC bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104419 |
|
Did you test with clang 17 or greater? I was wondering if ArchLinux could provide clang 17 for testing. clang 17 supports gnu++2c option. |
|
It fails the same way with clang 17 and The problem is really is in different implementation of |
|
Let's get this PR done. There is only one step to be completed here. That is the incompatibility between |
|
I am not sure how to solve the error. Please feel free to add necessary changes. |
|
Okay so I switched to clang's diff --git a/vendor/CMakeLists.txt b/vendor/CMakeLists.txt
index bcabe7e..09da9db 100644
--- a/vendor/CMakeLists.txt
+++ b/vendor/CMakeLists.txt
@@ -16,6 +16,12 @@ if(APPLE)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -D_DARWIN_C_SOURCE -D__DARWIN_C_LEVEL=__DARWIN_C_FULL")
endif()
+if((CMAKE_CXX_COMPILER_ID MATCHES "Clang"))
+ # GNU libstd is not fully compatible with Clang, see https://github.com/nmeum/android-tools/pull/132#issuecomment-1949759598
+ add_compile_options($<$<COMPILE_LANG_AND_ID:CXX,Clang>:-stdlib=libc++>)
+ add_link_options($<$<LINK_LANGUAGE:CXX>:-lc++>)
+endif()
+
# Android seems to use various attributes supported by clang but not by
# GCC which causes it to emit lots of warnings. Since these attributes
# don't seem to effect runtime behaviour simply disable the warnings.It fixes the compilation error above. BUT, it adds this linker error googling around I found this SO message https://stackoverflow.com/a/50836934 i.e. exactly what we try to do here. We try to link our tools against If my hypothesis is true then we really have 2 options here:
I am in favor of # 1. |
|
Now hm... |
|
As well as this @Biswa96 is it something expected? |
|
We can ignore the macos 13 issue. The error comes from installing python 3.12. The same issue happened with python 3.11 but was fixed later actions/runner-images#6459 |
It is OK to ignore it briefly. But there need to be a fix for this runner; otherwise |
|
Looks good to me. I will be happy to see this PR merged. |
Probably, there are working on this actions/runner-images@250c514 |
|
Time for 34.0.5 release? |
3 participants
Fixes #131