Works fine on my device.

Since this PR makes newly added build step disabled by default it should be harmless.
I will merge it in 12 hours if there are no objections.

1. Using local and sub shell together hides the result value of the command and set -e abort won't happen. All local variables must be declared beforehand.

• https://github.com/koalaman/shellcheck/wiki/SC2155#problematic-code-in-the-case-of-local

$ func() { set -e; local res="$(false)"; echo here1; }
(func;)
func1() { set -e; local res; res="$(false)"; echo here2; }
(func1;)
here1

2. Using -printf '%f|' will result in last file to also have | appended, which will result in a dangling | in the regex, which is invalid.
3. Package names are not safe to be parsed as regex, for example, libc++, libsigc++-3.0, etc have regex special characters like . and +, which can have unexpected consequences. Additionally, a double ++ would be invalid too.
4. Instead of using .* in .*/($PKG_REGEX)$", cd first and use ^\./.

local packages packages_regex

packages="$(find ${TERMUX_PACKAGES_DIRECTORIES} -mindepth 1 -maxdepth 1 -type d -printf '%f\n')"
[[ -z "$packages" ]] && return 0

# Exclude current package.
packages="$(printf "%s" "$packages" | grep -Fxv "$TERMUX_PKG_NAME")"
[[ -z "$packages" ]] && return 0

packages_regex="$(printf "%s" "$packages" | sed -zE 's/[][\.|$(){}?+*^]/\\&/g' | sed -E 's/(.*)/(\1)/g' | sed -zE -e 's/[\n]+/|/g' -e 's/(.*)/(\1)/g')"

(cd "$TERMUX_TOPDIR" && find . -mindepth 1 -maxdepth 1 -type d -regextype posix-extended -regex "^\./$packages_regex$" -exec rm -rf "{}" +)

1. Using local and sub shell together hides the result value of the command and set -e abort won't happen. All local variables must be declared beforehand.

In this case nothing bad will happen, cleanup will not be performed. Seems to be harmless.

2. Using -printf '%f|' will result in last file to also have | appended, which will result in a dangling | in the regex, which is invalid.

Correct, and that is the reason for ^$ dummy token to be added. Empty filenames are not possible in our case so it will have no matches.
Also | is not a part of any packagename in termux-packages so IMHO it should not be escaped and can be safely used here.

3. Package names are not safe to be parsed as regex, for example, libc++, libsigc++-3.0, etc have regex special characters like . and +, which can have unexpected consequences. Additionally, a double ++ would be invalid too.

Correct. Did not think about it.

4. Instead of using .* in .*/($PKG_REGEX)$", cd first and use ^\./.

find appends a string literal passed to commandline as a part of response so it should be safe to use a response without cd, isn't it?

Please init TERMUX_CLEANUP_BUILT_PACKAGES_ON_LOW_DISK_SPACE variable with false value in scripts/build/termux_step_setup_variables.sh instead

to allow reuse outside scripts/build/termux_step_cleanup_packages.sh

Truboxl, I asked about that but, twaik explained that there is some reason it is better to stay this way here

It would be extremely helpful and speed up review process of changing the build scripts by having CI for on device build. Not that we are allowing on device built packages to be uploaded to server yet (with some exceptions like pypy).

It would be extremely helpful and speed up review process of changing the build scripts by having CI for on device build. Not that we are allowing on device built packages to be uploaded to server yet (with some exceptions like pypy).

Probably we should create termux-docker-based workflow for checking build-system capabilities for all cases (host, docker and termux-docker (aka on-device)).

Seems to be fine, I'll merge this in 12 hours if no one objects.

It would be extremely helpful and speed up review process of changing the build scripts by having CI for on device build. Not that we are allowing on device built packages to be uploaded to server yet (with some exceptions like pypy).

I discussed some information about that here, i believe licy183 might try to make that, but I will try to make it also.

I created a $TERMUX_ON_DEVICE_BUILD=true-compatible build.sh for pypy as a test and it works, but I will wait to upload it until after there is a workflow capable of testing it in CI.

There are a lot of packages that currently have various errors when built with $TERMUX_ON_DEVICE_BUILD=true, so if such a workflow is enabled, it would fail to build a lot of packages, but maybe it would be good if it helps to fix those errors.

Probably we should create termux-docker-based workflow for checking build-system capabilities for all cases (host, docker and termux-docker (aka on-device)).

There are some packages that rely on audio or graphics libraries, which I believe termux-docker cannot currently build or test effectively with exactly accurate behavior to what a full AVD would, because of not having, for one example, OpenSL ES libraries or stubs yet, so any workflow doing this would need to be limited primarily to purely "headless" CLI packages first, but it would still be a good start.

In this case nothing bad will happen, cleanup will not be performed. Seems to be harmless.

In your old code, you would be getting an empty variable for packages when command fails, and then using it to generate a regex and then using it to remove directories. If something bad doesn't happen, it wouldn't be because its harmless, but because you were lucky.

You still haven't separated the local declaration from assignment with commands. Fix it.

Correct, and that is the reason for ^$ dummy token to be added. Empty filenames are not possible in our case so it will have no matches.

You would still not be creating a valid or at least logical regex. That dummy token will be added to ".*/($PKG_REGEX)$" and you will get something like ".*/(foo|bar|^$)$", where there are double end anchors $.

find appends a string literal passed to commandline as a part of response so it should be safe to use a response without cd, isn't it?

Yes, it should be fine to do that as well, at the cost of slight cost if there are lot of directories.

You still haven't separated the local declaration from assignment with commands. Fix it.

You mean declare all local variables on separate line at the start of the function?

You would still not be creating a valid or at least logical regex. That dummy token will be added to ".*/($PKG_REGEX)$" and you will get something like ".*/(foo|bar|^$)$", where there are double end anchors $.

The whole point was to create a token that will not match in any case :) .

Yes, separate line.

https://github.com/koalaman/shellcheck/wiki/SC2155#problematic-code-in-the-case-of-local

Oh, ok. TIL.

Done.

So now it should be fine, right?

Yes, thanks