Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update platform-tools to 33.0.1 #65

Closed
wants to merge 10 commits into from
Closed

Conversation

jershell
Copy link
Contributor

No description provided.

@jershell jershell mentioned this pull request May 30, 2022
@jershell
Copy link
Contributor Author

Okay. I have a successfull build on ubuntu 22.04 & clang 14.0

cmake -DCMAKE_C_COMPILER=/usr/bin/clang -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_BUILD_TYPE=Debug ..

@jershell
Copy link
Contributor Author

jershell commented Jun 6, 2022

i updated usb patch for fix some build errors

@jershell
Copy link
Contributor Author

jershell commented Jun 6, 2022

Okay =, i fixed it.

@jershell
Copy link
Contributor Author

jershell commented Jun 6, 2022

So, its look like a good result.
I send a new patch update for musl.
Let`s see what we get.

@jershell
Copy link
Contributor Author

jershell commented Jun 7, 2022

Okay, it was last patch. I hope.
What interesting if I have installation of gcc-12 and libgcc-12-dev on my ubuntu 22.04 system I have broken clang build. After removing libgcc-12-dev I can do it by clang again. WTF?
May be it is okay, i don't know, but clang used gcc std.
Its from clang build log
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/vector.tcc:77:29: error: constexpr if condition is not a constant expression /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/type_traits:725:38: error: incomplete type 'android::Theme::Entry' used in type trait expression /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/allocator.h:182:50: error: invalid application of 'sizeof' to an incomplete type 'android::Theme::Entry'
Some simular i read here https://stackoverflow.com/questions/68226260/old-clang-not-compiling-after-installing-llvm-12
After i decided to remove libgcc-12-dev i got successful build with clang-14.0

@nmeum
Copy link
Owner

nmeum commented Jun 8, 2022

What interesting if I have installation of gcc-12 and libgcc-12-dev on my ubuntu 22.04 system I have broken clang build. After removing libgcc-12-dev I can do it by clang again. WTF?

I also attempted to fix some issues with GCC 12 with plattform-tools-31.0.3, see #61 (comment). Unfortunately, I haven't had the time to further debug/fix those yet. Not sure if they are related to your issues though. Otherwise, thanks for your work on upgrading to platform-tools 33.0.1 would be cool if you could make it pass on the CI :)

@munix9
Copy link
Contributor

munix9 commented Jun 8, 2022

hm, strange. The build on OBS based on this PR works for openSUSE Leap 15.3, 15.4 and Tumbleweed without problems.

The environment is a bit different due to the OBS defaults, but basically it should work with the Github Actions without errors.

I'll have to take a closer look when I have more time, but maybe someone else has an idea.
I suspect a certain compiler option or something similar.

https://build.opensuse.org/package/show/home:munix9/android-tools
https://build.opensuse.org/package/live_build_log/home:munix9/android-tools/15.3/x86_64
https://build.opensuse.org/package/live_build_log/home:munix9/android-tools/15.4/x86_64
https://build.opensuse.org/package/live_build_log/home:munix9/android-tools/openSUSE_Tumbleweed/i586
https://build.opensuse.org/package/live_build_log/home:munix9/android-tools/openSUSE_Tumbleweed/x86_64

@munix9
Copy link
Contributor

munix9 commented Jun 9, 2022

In the meantime found out the following:

in the file build.yml in line 123 remove the option -DCMAKE_BUILD_TYPE=Release (diff).

With this at least the build with gcc for tumbleweed and archlinux works.

I would deactivate the builds for tumbleweed-clang and archlinux-clang for now, since errors still occur here.

https://github.com/munix9/android-tools/blob/master/.github/workflows/build.yml
https://github.com/munix9/android-tools/actions

@jershell
Copy link
Contributor Author

jershell commented Jun 9, 2022

For build with clang necessary uninstall gcc 12 and use gcc11 or gcc10

@munix9
Copy link
Contributor

munix9 commented Jun 9, 2022

For build with clang necessary uninstall gcc 12 and use gcc11 or gcc10

No, this does not work and quite a few dependencies are broken on the package level,
both on Tumbleweed with zypper and on ArchLinux with pacman.
clang and go, for example, require gcc[12] and are uninstalled accordingly as well.
Trying to forcefully break the package dependencies leads to an inconsistent system.
This makes no sense in my opinion to test builds with clang + gcc12 at the moment.
I would disable the two affected builds for the time being, as already suggested.

@nmeum
Copy link
Owner

nmeum commented Jul 14, 2022

I fixed the master branch build with GCC 12.X.

Can you rebase your changes against that? Maybe it helps with the build error.

@jershell
Copy link
Contributor Author

I fixed the master branch build with GCC 12.X.

Can you rebase your changes against that? Maybe it helps with the build error.

yes. I did it. I have build errors

  1. build (archlinux:base, clang, CC=clang CXX=clang++)
  2. build (opensuse/tumbleweed, clang, CC=clang CXX=clang++)
In file included from /home/jershell/20t/android-tools/vendor/base/libs/androidfw/AssetManager2.cpp:19:
In file included from /home/jershell/20t/android-tools/vendor/base/libs/androidfw/include/androidfw/AssetManager2.h:20:
In file included from /home/jershell/20t/android-tools/vendor/libbase/include/android-base/macros.h:22:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/utility:69:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_pair.h:60:
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/type_traits:964:7: error: static_assert failed due to requirement 'std::__is_complete_or_unbounded(std::__type_identity<android::Theme::Entry>{})' "template argument must be a complete class or an unbounded array"
      static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
      ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/alloc_traits.h:532:11: note: in instantiation of template class 'std::is_nothrow_destructible<android::Theme::Entry>' requested here
        noexcept(is_nothrow_destructible<_Up>::value)
                 ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_uninitialized.h:1060:54: note: in instantiation of exception specification for 'destroy<android::Theme::Entry>' requested here
             && noexcept(std::allocator_traits<_Allocator>::destroy(
                                                            ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_uninitialized.h:1080:28: note: in instantiation of exception specification for '__relocate_object_a<android::Theme::Entry, android::Theme::Entry, std::allocator<android::Theme::Entry>>' requested here
    noexcept(noexcept(std::__relocate_object_a(std::addressof(*__result),
                           ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_uninitialized.h:1129:23: note: in instantiation of exception specification for '__relocate_a_1<android::Theme::Entry *, android::Theme::Entry *, std::allocator<android::Theme::Entry>>' requested here
    noexcept(noexcept(__relocate_a_1(std::__niter_base(__first),
                      ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_uninitialized.h:1127:5: note: in instantiation of exception specification for '__relocate_a<android::Theme::Entry *, android::Theme::Entry *, std::allocator<android::Theme::Entry>>' requested here
    __relocate_a(_InputIterator __first, _InputIterator __last,
    ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_vector.h:503:14: note: in instantiation of function template specialization 'std::__relocate_a<android::Theme::Entry *, android::Theme::Entry *, std::allocator<android::Theme::Entry>>' requested here
        return std::__relocate_a(__first, __last, __result, __alloc);
                    ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/vector.tcc:80:8: note: in instantiation of member function 'std::vector<android::Theme::Entry>::_S_relocate' requested here
              _S_relocate(this->_M_impl._M_start, this->_M_impl._M_finish,
              ^
/home/jershell/20t/android-tools/vendor/base/libs/androidfw/AssetManager2.cpp:1350:19: note: in instantiation of member function 'std::vector<android::Theme::Entry>::reserve' requested here
  theme->entries_.reserve(kInitialReserveSize);
                  ^

@JamiKettunen JamiKettunen mentioned this pull request Jul 21, 2022
@nmeum
Copy link
Owner

nmeum commented Aug 6, 2022

Interesting that it fails with clang since that it was AOSP actually supports. Don't have the time to debug this further myself right now but will hopefully find some time to revisit this eventually if nobody else get around to it in the meantime.

@anatol
Copy link
Collaborator

anatol commented Aug 9, 2022

@jershell could you please rebase the PR on top of the recent changes?

@jershell
Copy link
Contributor Author

done

@motorto
Copy link

motorto commented Aug 12, 2022

@jershell maybe a git rebase master instead of a git merge, to remove that unneeded commit

@motorto
Copy link

motorto commented Aug 12, 2022

@jershell maybe a git rebase master instead of a git merge, to remove that unneeded commit

Thanks for your hard working looking forward to this update

@jershell
Copy link
Contributor Author

@jershell maybe a git rebase master instead of a git merge, to remove that unneeded commit

Thanks for your hard working looking forward to this update

@jershell maybe a git rebase master instead of a git merge, to remove that unneeded commit

when we will finished the pr, we can "squash" commits for clear history

@anatol
Copy link
Collaborator

anatol commented Sep 22, 2022

Hi @jershell I rebased your patches on top of the master that contains GCC fixes. It builds fine with Arch Linux 12.2.0 GCC. I pushed the rebase to this PR.

I also bumped the upstream release to 33.0.3.

clang build fails for me with

/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.0/../../../../include/c++/12.2.0/type_traits:910:7: error: static_assert failed due to requirement 'std::__is_complete_or_unbounded(std::__type_identity<android::Theme::Entry>{})' "template argument must be a complete class or an unbounded array"
      static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
      ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I keep looking to see if I can fix it.

@anatol
Copy link
Collaborator

anatol commented Sep 22, 2022

My last patch has fixed the build for clang. The tests are green.

I briefly tested adb/fastboot and things look good so far.

The PR is ready for review and merging to master.

@nmeum
Copy link
Owner

nmeum commented Sep 23, 2022

Thanks @jershell and @anatol. I squashed some commits and merged this into the master branch.

I also created a 33.0.3 release tarball: https://github.com/nmeum/android-tools/releases/tag/33.0.3

Let me know if you encounter any issues.

Thanks again!

@nmeum nmeum closed this Sep 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants