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

bionic-host: rename to aosp-libs, bump to 9.0.0-r76, add aosp-utils subpackage, revbump revdeps #22906

Conversation

robertkirkman
Copy link
Contributor

@robertkirkman robertkirkman commented Jan 12, 2025

  • depends on chore(scripts/setup-ubuntu): retarget symbolic link to prepare for aosp-libs package #23054 - if this idea is approved, it will not successfully build until a new symbolic link propagates into a new termux-package-builder docker image merged.
  • twaik suggested combining and renaming the subpackages, so I have done that, and along with that I am additionally renaming the entire package, because AOSP-based libz.so appeared, AOSP-based libicuuc.so was previously added, and in general it seems like this name is more accurate and idiomatic to present and future flexible use-cases of this package.
  • bumping is required because the /system/lib/libc.so file of AOSP 8.0.0 is bugged and freezes when run inside Docker on 32-bit devices, but the same file in AOSP 9.0.0 is unaffected.
  • aosp-utils is dependencies that would be very helpful and idiomatic for deblobbify, replacing the system folder with several other components termux-docker#72.
    • mksh provides /system/bin/sh for termux-docker.
    • iputils provides /system/bin/ping for termux-docker which is a dependency of $PREFIX/bin/ping.
    • toybox provides /system/bin/su which is used as a crucial component of termux-docker to switch to the system user during its entrypoint.sh.
  • this is also progress on Some packages cannot be built 2: Electric boogaloo #21130 because it fixes the build error of pypy3 I documented there earlier (the change that fixes that error is solely adding coreutils to TERMUX_PKG_BUILD_DEPENDS).
  • Work around [Bug, CI]: 32 bit binaries fail with sigabrt #18517 by lowering the process ID limit in the GitHub Actions workflows, required for successfully building the current implementations of pypy and pypy3 for 32-bit targets (regardless of the other changes in this PR, since they appear to have passed CI by chance in previous builds).
    • I attempted a large number of things, including a rootless workaround that only worked locally, but unfortunately, no other way seems feasible to avoid this error within GitHub Actions itself, than editing the .github folder.
    • One reason that it took me a long time to realize this would be a problem was because, on my local computer, /proc/sys/kernel/pid_max was 32768, not 65536 or higher, for a very long time, probably due to some other thing on my computer unrelated to Termux, and I did not notice this. Therefore, this problem never appeared for me locally in any Docker container or Termux package I built. As a result, hopefully on the plus side, I can fairly confidently assure that setting the value to 65535 will not break any packages, because I tested building all termux packages in the past locally when I didn't know I was using a much lower value, and it never caused any problems.

@robertkirkman robertkirkman changed the title addpkg(main/{dnsmasq,iputils,mksh,toybox}-host): 8.0.0-r51 bump(main/bionic-host),addpkg(main/{boringssl,iputils,mksh,toybox}-host): 9.0.0-r76 Jan 21, 2025
@robertkirkman robertkirkman force-pushed the bionic-host-deblobbify-termux-docker branch from daeb42b to d380f68 Compare January 21, 2025 01:24
@robertkirkman robertkirkman marked this pull request as ready for review January 21, 2025 12:42
Copy link
Member

@Grimler91 Grimler91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, nice work!

Does any of the subpackages need to:

TERMUX_SUBPKG_BREAKS="bionic-host (<< 9.0.0-r76)"
TERMUX_SUBPKG_REPLACES="bionic-host (<< 9.0.0-r76)"

or are all of the subpackage files freshly added in this version?

If for example $PREFIX/opt/bionic-host/lib/libcrypto.so was part of bionic-host 8.0.0-r51 then we would get a file conflict when trying to install the subpackage without BREAKS and REPLACES.

@robertkirkman robertkirkman marked this pull request as draft January 21, 2025 13:46
@robertkirkman
Copy link
Contributor Author

robertkirkman commented Jan 21, 2025

are all of the subpackage files freshly added in this version?

  • Yes, all of the subpackage files do not exist in the previous version
  • libcrypto.so is added to a subpackage because it is not used by anything that is in the main package, it is only a dependency of some of the subpackages, so it seemed appropriate to do that
  • On the other hand, there are some other freshly added files that appeared that I did not add to subpackages because they seemed to become dependencies of one of the components that was explicitly included within the original bionic-host package for some other purpose, crash_dump64. Here is one of the dependency chains I noticed between the newer crash_dump64 and some of the newly added files, that did not seem to happen in the 8.0.0 version:
    • /system/bin/crash_dump64 is dynamically linked to /system/lib64/libunwindstack.so
    • /system/lib64/libunwindstack.so is dynamically linked to /system/lib64/libdexfile.so
    • /system/lib64/libdexfile.so is dynamically linked to /system/lib64/libz.so

If the crash_dump64 created by make crash_dump is still wanted or needed and isn't meant to be in a subpackage, then these additional libraries probably belong with it in the main package, but if the original user of crash_dump64 thinks it's a good idea for it to be a subpackage instead, then maybe it and those other libraries should be in subpackages as well.

@robertkirkman robertkirkman force-pushed the bionic-host-deblobbify-termux-docker branch from d380f68 to 29bc357 Compare January 21, 2025 15:18
@robertkirkman robertkirkman marked this pull request as ready for review January 21, 2025 16:02
@twaik
Copy link
Member

twaik commented Jan 27, 2025

Is there any specific reason for creating lots of subpackages? In the case if you want to split it only for separating bionic itself and other stuff we can rename this package to termux-docker-system-img or something similar.

@robertkirkman robertkirkman marked this pull request as draft January 27, 2025 16:07
@robertkirkman
Copy link
Contributor Author

robertkirkman commented Jan 27, 2025

Is there any specific reason for creating lots of subpackages? In the case if you want to split it only for separating bionic itself and other stuff we can rename this package to termux-docker-system-img or something similar.

yes it is split for organizational purposes and clarity, since the pypy and pypy3 packages do not depend on the entire "base system" , they only depend on the contents of the original bionic-host package, and for a couple of other reasons.

For example, in an earlier version, I had boringssl explicitly removed, and instead had dnsmasq here, but now I decided to switch to the dnsmasq from root/dnsmasq , for two reasons:

    1. because it fixes a bug that is present in the current termux-docker
    1. because, unlike the other components, the dnsmasq implementation from AOSP is not really used "in the wild" (average real-world Android devices) for the same purpose that termux-docker uses it for.
    • I was wondering for a little while, "many Android devices have /system/bin/dnsmasq, but ps -ef in ADB shell never seems to show my devices using it. What conditions does an average Android device need to be under, for it to start using its /system/bin/dnsmasq?"
    • After a little while, I found the answer. Touching the "Mobile Hotspot" Button in SystemUI, causes this process to appear in ADB shell ps -ef:
      • root 27388 1148 0 09:39:12 ? 00:00:00 dnsmasq --keep-in-foreground --no-resolv --no-poll --dhcp-authoritative --dhcp-option-force=43,ANDROID_METERED --pid-file --listen-mark 0xf0063 --user root --except-interface=wlan0 --dhcp-range=192.168.42.2,192.168.42.254,255.255.255.0,1h --dhcp-range=192.168.43.2,192.168.43.254,255.255.255.0,1h --dhcp-range=192.168.44.2,192.168.44.254,255.255.255.0,1h --dhcp-range=192.168.45.2,192.168.45.254,255.255.255.0,1h --dhcp-range=192.168.46.2,192.168.46.254,255.255.255.0,1h --dhcp-range=192.168.47.2,192.168.47.254,255.255.255.0,1h --dhcp-range=192.168.48.2,192.168.48.254,255.255.255.0,1h --dhcp-range=192.168.49.2,192.168.49.254,255.255.255.0,1h --dhcp-range=192.168.50.2,192.168.50.254,255.255.255.0,1h --dhcp-range=192.168.51.2,192.168.51.254,255.255.255.0,1h --dhcp-range=192.168.60.2,192.168.60.254,255.255.255.0,1h
    • That process is not like the way dnsmasq is invoked within termux-docker, and, turning off the "Mobile Hotspot" causes that process to stop.
    • For that reason, it seems like the preexisting termux-packages dnsmasq is more appropriate for termux-docker, and the AOSP dnsmasq is not needed, since that one is for Mobile Hotspots, which is not an intended use case of termux-docker. Instead, termux-docker uses dnsmasq at all times and with fewer arguments, to resolve hostnames for all software.

I added in the boringssl package because, originally, I found a way to work without it, by excluding the /system/bin/ping6 program and by patching toybox more heavily, however, when I considered the options, I realized the amount of patching and the overall number of lines of code in the package is reduced if I just leave in the boringssl package and allow it to be used as a dependency, and if someone has an IPv6 network they might find some use from the ping6 utility that I don't, so that's what I did.

The subpackage organization made it easy for me to keep track of what I was doing during those changes.

If you would like to remove the subpackages and make everything combined into a single package, and rename the package, I would suggest this name:

  • termux-aosp

Reflecting the idea that this is AOSP, but compiled and patched in a unique way for the needs of some Termux projects.

If you would like to combine most of the subpackages into a single subpackage, but keep the main package separate, I would suggest they could use these names:

  • termux-aosp-libs: containing the files that are currently in bionic-host and boringssl-host
  • termux-aosp-utils: containing the files that are currently in iputils-host, mksh-host and toybox-host

If you would like to rename the package but keep all the subpackages separate still, I would suggest they could use these names:

  • termux-aosp-bionic
  • termux-aosp-boringssl
  • termux-aosp-iputils
  • termux-aosp-mksh
  • termux-aosp-toybox

I think names like these (any of the above) generally make it easier to understand at a glance exactly what this package is, than the current names, especially since:

  • originally, this package was used only for building pypy and pypy3
  • but then, based mostly on your suggestion in your issue, it has become obvious that after the changes in this PR, this package is also potentially capable of deblobbifying termux-docker
  • in the future, unknown events could happen that could potentially create additional use-cases for this package that might not necessarily be directly related to termux-docker, pypy, or pypy3.

Those are the reasons why I think it would be best to choose a name and organization for the package carefully and accurately, that is flexible to match any present or future potential use cases.

I think if the name will be changed, the input of multiple people might be helpful in order to choose the best name. EDIT: after considering names some more, I have decided to try "aosp-libs", dropping the "termux-" prefix, because all existing packages named "termux"-something are associated with repositories that are mostly formed from code that was written from scratch explicitly for Termux, while this code is mostly copied and pasted from AOSP with only minimal changes for Termux.

If you want to go with the name I suggested, I would suggest using the installation folder name "/data/data/com.termux/files/usr/opt/aosp/" instead of the folder name "bionic-host" or "termux-aosp" , that's just my opinion, but maybe others have better ideas.

Just so others know I understand the following part, I am aware of the other instances of the string "bionic-host" that are used for pypy and pypy3, so if the decision is made to change the package name and installation folder name, I am prepared to submit changes for all the appropriate files, and revision-bump pypy and pypy3 to match.

image

robertkirkman added a commit to robertkirkman/termux-packages that referenced this pull request Jan 28, 2025
@robertkirkman robertkirkman changed the title bump(main/bionic-host),addpkg(main/{boringssl,iputils,mksh,toybox}-host): 9.0.0-r76 bionic-host: rename to aosp-libs, bump to 9.0.0-r76, add aosp-utils subpackage, revbump revdeps Jan 28, 2025
@robertkirkman robertkirkman force-pushed the bionic-host-deblobbify-termux-docker branch 3 times, most recently from 645904e to 307368f Compare January 29, 2025 05:51
Grimler91 pushed a commit to robertkirkman/termux-packages that referenced this pull request Jan 29, 2025
@robertkirkman robertkirkman force-pushed the bionic-host-deblobbify-termux-docker branch 2 times, most recently from 02b2178 to eba45ac Compare February 3, 2025 12:04
@robertkirkman robertkirkman marked this pull request as ready for review February 3, 2025 13:56
@robertkirkman
Copy link
Contributor Author

Might be adjusted to follow changes made by licy183 to the pypy package, if necessary, in the correct merging order.

@robertkirkman robertkirkman marked this pull request as draft February 25, 2025 18:35
@robertkirkman robertkirkman force-pushed the bionic-host-deblobbify-termux-docker branch from eba45ac to 9574e87 Compare February 26, 2025 09:14
@robertkirkman robertkirkman marked this pull request as ready for review February 26, 2025 11:04
@robertkirkman robertkirkman force-pushed the bionic-host-deblobbify-termux-docker branch from 9574e87 to a1ca926 Compare February 27, 2025 04:39
@robertkirkman robertkirkman marked this pull request as draft February 27, 2025 18:56
@robertkirkman robertkirkman force-pushed the bionic-host-deblobbify-termux-docker branch from a1ca926 to ee137ce Compare February 27, 2025 21:37
@robertkirkman
Copy link
Contributor Author

I have a question about the licensing. While working on this package, I have been simply assuming that the current TERMUX_PKG_LICENSE "BSD 3-clause" and included LICENSE.txt are sufficient regardless of the precise contents of the package, because this package has always contained a liblzma.so, (which is not really the same kind of liblzma.so that comes from main/liblzma, that one is a heavily diverged fork that is merged into xz-utils, while this one is an AOSP-based one that is built from code more similar to the original code released by the creator of 7-zip), which is marked with a "public domain" license here, which I thought is not really the same thing as "BSD 3-clause".

But, now after my changes, the package will include binaries built from code in the following sections of AOSP, that it previously did not:

external/zlib (Zlib license)
external/boringssl (the old OpenSSL license)
external/mksh (MirOS license)
external/toybox (BSD 0-clause license)
external/iputils (BSD 4-clause license)

Is it OK to leave the overall package license unchanged, or should I try to add every exact individual license in a list to TERMUX_PKG_LICENSE like some packages have?

TERMUX_PKG_LICENSE="GPL-2.0, GPL-2.0-or-later, GPL-3.0, GPL-3.0-or-later, LGPL-2.1, LGPL-2.1-or-later, LGPL-3.0, LGPL-3.0-or-later, Mozilla-1.1, Openfont-1.1"

@robertkirkman robertkirkman marked this pull request as ready for review February 28, 2025 01:28
@robertkirkman robertkirkman force-pushed the bionic-host-deblobbify-termux-docker branch 4 times, most recently from b75f03e to b84f3be Compare March 1, 2025 04:04
@robertkirkman
Copy link
Contributor Author

To be on the side of caution I decided to just do it, but BSD 0-Clause is not an available option so does not pass the linter, so I just left it off from the TERMUX_PKG_LICENSE list, but still added it in TERMUX_PKG_LICENSE_FILE at external/toybox/NOTICE.

@robertkirkman robertkirkman force-pushed the bionic-host-deblobbify-termux-docker branch from b84f3be to efadf4f Compare March 1, 2025 05:01
@twaik
Copy link
Member

twaik commented Mar 3, 2025

LGTM.

@robertkirkman
Copy link
Contributor Author

Ok, I will merge this in 24 hours if no problems are found in that time.

The reason that the contents of the termux-docker PR do not yet match the changed name of the package is because I am locally preparing the next version of that to be ready to download all packages from the mirror (and not use deb files stored in git), so it is convenient for me to wait for this PR before pushing more changes into that PR.

Copy link
Member

@licy183 licy183 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@twaik
Copy link
Member

twaik commented Mar 4, 2025

Seems like it is time to create PR to termux-docker for using this package as a base.

@robertkirkman
Copy link
Contributor Author

robertkirkman commented Mar 4, 2025

Yes I have code of it I am ready to test locally, but first I have to wait ~2 hours for this PR to be compiled by the merge-step CI and upload its packages to packages-cf.termux.dev, before my code will be able to download the packages from the cloud.

I have several adjustments and refactors related to that, compared to the version currently in the termux-docker PR (the version currently in the PR only downloads dnsmasq from packages-cf.termux.dev and not the rest of the packages),

I just won't know whether that part is 100% working until the packages are available to download in the package mirror, then I will be able to test it.

EDIT: it worked, now i uploaded the code to the termux-docker PR

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.

4 participants