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

build-bootstraps: bump to 0.2.0 #10296

Closed
wants to merge 3 commits into from

Conversation

truboxl
Copy link
Contributor

@truboxl truboxl commented Apr 23, 2022

Supersedes #9382

No longer includes no-Neon build option (will reintroduce later)
Add optional build-extra-first option
Fix building bootstraps by only including packages for that specific arch and "all" variant

truboxl added 3 commits April 24, 2022 00:14
* add support for building additional packages first
* fix extract_debs to only extract compatible packages for targeted
  architecture
@truboxl truboxl requested a review from Grimler91 as a code owner April 23, 2022 16:23
@truboxl
Copy link
Contributor Author

truboxl commented Apr 23, 2022

CC @agnostic-apollo for review

@agnostic-apollo
Copy link
Member

agnostic-apollo commented Apr 25, 2022

Thanks for the important fix in 7c5578c. I think before this, the bootstrap archives generated must not be valid since only the first alphabetically sorted arch would get included for all debs as per for loop *.deb expansion and $TERMUX_ARCHITECTURES order, unless a force build was done. This is my fault, I should have tested multiple archs. However, I think a better way would be only loop on correct arch to prevent needless echos.

while IFS= read -r -d '' deb; do
    # code here
done < <(find . -type f \( -name "*_$package_arch.deb" -o -name "*_all.deb" \) -print0 | sort -nz)

But thinking more on it, there are still two issues. Firstly, if the output directory already had debs that weren't supposed to be part of the default packages, they would also get included. Secondly, if the multiple versions of the same package existed, only one would get extracted, which may not even be the required one.

One way would be to check if deb package exists in $PACKAGES array and its version matches the version defined by $TERMUX_PKG_VERSION and $TERMUX_PKG_REVISION in build.sh for the package by using scripts/build/termux_extract_dep_info.sh. But $PACKAGES array won't have dependencies. I think best way is to just always nuke $TERMUX_BUILT_DEBS_DIRECTORY output directory at start of for loop for each architecture instead of just for force builds.

Otherwise pull request looks good. Thanks again!

cc: @simbadMarino

@simbadMarino
Copy link

Luckly I've always used the forced way @agnostic-apollo hehe. Thanks @truboxl for the update! It might save a few MB in the future and of course ease the build process =).

@agnostic-apollo
Copy link
Member

Luckly I've always used the forced way @agnostic-apollo hehe.

And here I wanted to break even your boostraps after breaking sharedUserId APKs. Really lucky you ;)

@truboxl Let me know if you want to add the fixes yourself or want me to do it.

@truboxl
Copy link
Contributor Author

truboxl commented Apr 29, 2022

I am fine with @agnostic-apollo changes since that look simpler.

Though the "pull all the debs inside the output folder and build bootstrap out of it" can be a useful feature for experimenting.

If possible, I like to have some caching mechanism. Nuking all and then rebuild LLVM package took way too long (many packages depends or build depends on this). Maybe we can just use back llvm.deb after nuking? Probably work on this later.

And yeah, there's a long way to go before reaching size equivalent to generate-bootstraps.sh with the non-existant dependency management in build-bootstraps.sh...

@stale
Copy link

stale bot commented Jun 13, 2022

This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix Issue won't be fixed label Jun 13, 2022
@agnostic-apollo agnostic-apollo removed the wontfix Issue won't be fixed label Jun 13, 2022
@stale
Copy link

stale bot commented Jul 28, 2022

This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix Issue won't be fixed label Jul 28, 2022
@agnostic-apollo agnostic-apollo removed the wontfix Issue won't be fixed label Jul 29, 2022
@stale
Copy link

stale bot commented Sep 12, 2022

This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@twaik
Copy link
Member

twaik commented Oct 19, 2024

Is this PR still relevant? It was opened more than 2 years ago and build system was modified. The branch has conflicts out of date.

@agnostic-apollo
Copy link
Member

No, entire bootstrap scripts have been rewritten, but not complete due to repo changes being the blocker.

@twaik
Copy link
Member

twaik commented Oct 19, 2024

So should we keep this PR opened?

@agnostic-apollo
Copy link
Member

Can be closed since these changes won't directly be used.

@truboxl truboxl deleted the build-bootstraps.sh-0.2.0 branch October 19, 2024 07:02
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.

6 participants