CI finished building so I assume the solution works fine.
I see INFO: cleaning up some disk space for building "python-onnxruntime". in the log so we know we ran out of space and build system triggered cleanup successfully.

I will merge it in ~10 hours or after it gets 2+ approves.

This behavior should be enabled for ci with a env variable, not for local builds. Users may be building bootstrap with lot of packages, which may need to be rebuilt again, and deleting built files is a no go.

Zzz breaks on device build

df: Unknown option 'B' (see "df --help")
/data/data/com.termux/files/home/termux-package/scripts/build/termux_step_cleanup_packages.sh: line 8: [: : integer expression expected

This behavior should be enabled for ci with a env variable, not for local builds. Users may be building bootstrap with lot of packages, which may need to be rebuilt again, and deleting built files is a no go.

i can't think of a scenario in which building a bootstrap locally with this change would cause packages to be rebuilt multiple times, is that what you meant or do you mean something a little different?

If you mean that someone might be editing the code of packages and somehow repeatedly recompiling them intentionally without storing the changes as .patch files, then I see what you mean, I am just not sure whether many people do that.

df: Unknown option 'B'

In code I wrote in the past, there is a block for detecting available space on device that has been very heavily tested on many of my devices and others' devices, so it should not be affected by the same problem on any device. I'm sorry I should have mentioned it earlier but I forgot about it.

BLOCKS_FREE=$(awk -F ' ' '{print $4}' <(df ~ | tail -1))
if (( 5242880 > BLOCKS_FREE )); then
    echo "less than 5 GB available"
fi

if you end up keeping the termux_step_cleanup_packages() enabled in on device builds, this example might be adaptable to implement that.

We already use

df: Unknown option 'B'

In code I wrote in the past, there is a block for detecting available space on device that has been very heavily tested on many of my devices and others' devices, so it should not be affected by the same problem on any device. I'm sorry I should have mentioned it earlier but I forgot about it.

BLOCKS_FREE=$(awk -F ' ' '{print $4}' <(df ~ | tail -1))
if (( 5242880 > BLOCKS_FREE )); then
    echo "less than 5 GB available"
fi

if you end up keeping the termux_step_cleanup_packages() enabled in on device builds, this example might be adaptable to implement that.

Could we replace that magic number with a named constant, or a equation and comment, or both.
e.g.
CLEANUP_THRESHOLD="$(( 5 * 1024 ** 3 ))"

CLEANUP_THRESHOLD="$(( 5 * 1024 ** 3 ))"

df outputs size in 1024 byte blocks. So `$(( 5 * 1024 ** 2 )).

forgot again

#23530

i can't think of a scenario in which building a bootstrap locally with this change would cause packages to be rebuilt multiple times

If you delete output directory with debs, then packages need to be rebuilt, and if you delete build progress, builds will start from scratch, which can take hours. Additionally, free disk space can vary on mobile device/docker, etc, and cleanup may not be required anyways for small packages. Any deletion should not be automatic, even force builds that cleanup packages require -f to be passed. Note that if you use a flag, the nested calls for dependencies will need to be forwarded the flag.

I do not see an option to pass environment variable from CI to build-package.sh in both docker and non-docker codepath. Can you provide an example?

Yes, I've seen it. But in this case ./build-package.sh does not receive it as environment variable, it is passed as a part of commandline.

If you delete output directory with debs, then packages need to be rebuilt

i might be wrong, but i did not think this change deletes the output directory with debs.

Any deletion should not be automatic

I agree with this concept, but I was not sure how important it is in this situation since twaik moved forward with a version that always runs regardless of whether it is in CI or not. If you think that this concept is very important to make sure that the data of people who are building packages with unknown custom configurations locally is not lost, then I agree with your opinion.

i might be wrong, but i did not think this change deletes the output directory with debs.

It does not.

If you think that this concept is very important to make sure that the data of people who are building packages with unknown custom configurations locally is not lost, then I agree with your opinion.

I agree too, but currently I am not really sure how to make it cleanup packages only in CI.

i might be wrong, but i did not think this change deletes the output directory with debs.

No it wont, but if someone deletes the deb/output directory as there are older or unneeded debs, etc, and then wants to rebuild them again, in that case if built progress was lost, then it will require rebuilding everything from scratch.

Yes, this cleanup must not be automatic. It needs to be specifically enabled for CI or if a user wants to.

I agree too, but currently I am not really sure how to make it cleanup packages only in CI.

It would be necessary to edit build-package.sh to have an additional argument for enabling the cleaning up packages step, and have the step only run when that argument is specified, then invoke build-package.sh with that argument in all locations where build-package.sh is called from CI, and also pass the argument to nested invocations of build-package.sh like agnostic-apollo mentioned.

Yes, this cleanup must not be automatic. It needs to be specifically enabled for CI or if a user wants to.

So how exactly should it be enabled? With additional argument or environment variable or something similar?

Yes, I've seen it. But in this case ./build-package.sh does not receive it as environment variable, it is passed as a part of commandline.

Pass an additional flag just like -I, I don't see an issue, it will need to be forwarded for build-package.sh call for dependencies. Env variables are also being passed for ndk.

Pass a flag like --cleanup-built-packages-on-low-storage.

@agnostic-apollo I pushed this change to #23530 as a part of fixing on-device building.

Thanks, should be fine. Continue build help entry will need to be removed from my pull though.

Thanks, should be fine

It would be an absolute honor if you hit that 'Approve' button for me! (I still not decided if it should be a joke or I am serious, but anyway it will be a pleasure to me).

lolz, there are other changes I haven't reviewed, I am away from laptop right now. You can consider the part of the flag as approved.