android-tools icon indicating copy to clipboard operation
android-tools copied to clipboard

Get other distros on board

Open ollieparanoid opened this issue 6 years ago • 19 comments

Hey @nmeum, thanks for creating this repository. I've found it from the Alpine aport in testing, and I think it makes a lot of sense to provide a sane build system for android-tools.

How about asking the maintainers of the packages in the other distributions you mentioned in the README.md, if they would like to use this source repository as well (so we have more eyeballs on it, and possibly more people maintaining it)?

PS: The link to the Void Linux package is outdated.

ollieparanoid avatar Aug 16 '18 16:08 ollieparanoid

How about asking the maintainers of the packages in the other distributions you mentioned in the README.md, if they would like to use this source repository as well (so we have more eyeballs on it, and possibly more people maintaining it)?

Yeah, actually that was the initial idea. I tried to get @anatol (the Arch Linux maintainer) on board, since the work done in this repo is based on his Arch ruby script, but sadly I didn't get a reply to the mail I send to him.

I think if we want to get other distros on board the proper way of doing so would be sending patches to them replacing their build system with this one. Unfortunately, I am very busy currently and thus didn't get around to doing this yet.

nmeum avatar Aug 17 '18 09:08 nmeum

I switched the Void Linux package to this repo https://github.com/void-linux/void-packages/commit/ca265fbf3dda37acc79c6ff2e65bca9b201c50d5, it worked quite flawlessly except some glibc architectures requiring -D_FILE_OFFSET_BITS=64.

Johnnynator avatar Oct 05 '19 23:10 Johnnynator

[…] some glibc architectures requiring -D_FILE_OFFSET_BITS=64.

Which architectures? Any idea why this is the case? If there is a specfic reason why this is needed we could just add the define to the CFLAGS and CXXFLAGS with a comment explaining why it is needed.

https://github.com/nmeum/android-tools/blob/db84fde9f61120d91c12e8ee2eb4169e16dea919/vendor/CMakeLists.txt#L36-L37

nmeum avatar Oct 07 '19 10:10 nmeum

Which architectures? Any idea why this is the case? If there is a specfic reason why this is needed we could just add the define to the CFLAGS and CXXFLAGS with a comment explaining why it is needed.

all 32bit Glibc architectures, check core/fs_mgr/liblp/utility.cpp, but adding it unconditionally to C/CXXFLAGS should not provide any issues.

static_assert(sizeof(off_t) == sizeof(int64_t), "Need 64-bit lseek");

Johnnynator avatar Oct 07 '19 10:10 Johnnynator

Added -D_FILE_OFFSET_BITS=64 to CFLAGS / CXXFLAGS by default. If you find anything else let me know. Ideally, android-tools should compile on all distros without any downstream patches.

nmeum avatar Oct 08 '19 09:10 nmeum

Submitted bug for Fedora to switch since it's super out of date. https://bugzilla.redhat.com/show_bug.cgi?id=1937578

kvuj avatar Mar 11 '21 21:03 kvuj

Anatol from Arch Linux project here. Sorry for being late to the party, 2019 was busy time for me and I completely forgotten about this activity. I just got a notification for the last message and (re-)discovered this project.

Great work, @nmeum! This build system is definitely an improvement over what we have in Arch. I'll be glad to look into adopting it for Arch. If this happen then more distros are going to be in sync and share testing/development workload.

anatol avatar Mar 12 '21 03:03 anatol

Status for Fedora: Builds well for current stable version of fedora(33) (GCC 10.2.1): https://koji.fedoraproject.org/koji/taskinfo?taskID=63781651 Fails for next(beta) version of fedora(34) (GCC 11): https://koji.fedoraproject.org/koji/taskinfo?taskID=63778383

vanaf avatar Mar 14 '21 22:03 vanaf

Arch Linux switched its package to https://github.com/nmeum/android-tools Thank you @nmeum and other folks for your work!

anatol avatar Mar 15 '21 00:03 anatol

Fails for next(beta) version of fedora(34) (GCC 11): https://koji.fedoraproject.org/koji/taskinfo?taskID=63778383

Theory: usage of std::numeric_limits requires #include <limits> which is missing in https://android.googlesource.com/platform/system/libziparchive/+/refs/tags/platform-tools-30.0.5/zip_archive_stream_entry.cc

minlexx avatar Mar 15 '21 00:03 minlexx

Arch Linux switched its package to https://github.com/nmeum/android-tools

Whoo! :tada: I also wouldn't mind giving you push access to the repository, if you want it. If more distros end up using this we might as well move it to a dedicated GitHub organization. CI for testing the build on different distros would probably also be useful then.

Theory: usage of std::numeric_limits requires #include <limits>

I think so too, will check later today :)

nmeum avatar Mar 15 '21 08:03 nmeum

If more distros end up using this we might as well move it to a dedicated GitHub organization.

Many (maybe not Debian) used Arch as "upstream", I would expect them to follow Arch with the Switch.

So far the list should be Alpine, Arch, Exherbo, Void and soon Fedora.

Johnnynator avatar Mar 15 '21 12:03 Johnnynator

@vanaf I added a patch fixing that particular build failure (fc2f2d05edb9fbbd6917f8df1b81657a1d450c6b), it was indeed a missing include. Unfortunately, boringssl (even the current git head from https://github.com/google/boringssl) does not build with gcc 11 currently (likely due to added/improved warnings). I suppose you will also run into this issue with other software using boringssl in Fedora 34. Probably best to report it to boringssl upstream or just compile with -Wno-error for now.

nmeum avatar Mar 15 '21 16:03 nmeum

Chromium builds well with boringssl on gcc-11

[headless_shell:2108/33279] gcc -MMD -MF obj/third_party/boringssl/boringssl/tasn_enc.o.d -DUSE_AURA=1 -DUSE_NSS_CERTS=1 -DUSE_OZONE=1 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DBORINGSSL_ALLOW_CXX_RUNTIME -DBORINGSSL_IMPLEMENTATION -DBORINGSSL_NO_STATIC_INITIALIZER -DOPENSSL_SMALL -D_XOPEN_SOURCE=700 -I../.. -Igen -I../../third_party/boringssl/src/include -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -funwind-tables -fPIC -pipe -pthread -m64 -march=x86-64 -msse3 -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -fno-omit-frame-pointer -g0 -fvisibility=hidden -Wno-unused-local-typedefs -Wno-maybe-uninitialized -Wno-deprecated-declarations -Wno-comments -Wno-packed-not-aligned -Wno-missing-field-initializers -Wno-unused-parameter -O2 -fno-ident -fdata-sections -ffunction-sections -std=gnu11 -std=c99 -c ../../third_party/boringssl/src/crypto/asn1/tasn_enc.c -o obj/third_party/boringssl/boringssl/tasn_enc.o

vanaf avatar Mar 15 '21 18:03 vanaf

Lets discuss this further in #16

nmeum avatar Mar 15 '21 18:03 nmeum

Whoo! I also wouldn't mind giving you push access to the repository, if you want it.

I'll be great to get one. I am not a cmake person, but i'll be glad to help with C/compilation related issues.

anatol avatar Mar 15 '21 20:03 anatol

I'll be great to get one. I am not a cmake person, but i'll be glad to help with C/compilation related issues.

Done, just make sure you use git config pull.rebase true. Thanks! :)

nmeum avatar Mar 16 '21 17:03 nmeum

Hi, I'd love to make a pull request for Gentoo's repository, but for that this project needs to be licensed. Could you license it please? Thanks.

xbjfk avatar Mar 21 '21 03:03 xbjfk

Could you license it please? Thanks.

That should be fixed with the most recent release (31.0.0), see #21.

nmeum avatar Mar 28 '21 15:03 nmeum