这是indexloc提供的服务,不要输入任何密码
Skip to content

chore(subpackages): make TERMUX_SUBPKG_DEPENDS check more readable #24146

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

Merged
merged 3 commits into from
Apr 5, 2025

Conversation

TomJo2000
Copy link
Member

This is an enhancement based on a question from discord user 444_aaa.

This first commit improves the readability of the TERMUX_SUBPKG_DEPENDS check by consolidating it into a case statement.
The semantics of requiring an empty variable in order to default to true are preserved using the : "${var:=default}" idiom.

I will be adding additional commits for general cleanup of the subpackage creation scripts to this PR shortly.

@TomJo2000 TomJo2000 requested a review from Grimler91 as a code owner April 3, 2025 16:54
@TomJo2000 TomJo2000 requested a review from finagolfin as a code owner April 3, 2025 17:50
@TomJo2000
Copy link
Member Author

I have now pushed commit 2 of this cleanup PR;
which renames the TERMUX_SUBPKG_BLACKLISTED_ARCHES control variable
used only by lldb and mesa-vulkan-icd-freedreno currently
to TERMUX_SUBPKG_BLACKLISTED_ARCHES,
bringing the terminology in line with the existing TERMUX_PKG_BLACKLISTED_ARCHES.

This should probably be committed with [no ci] since it is purely a package system change.

@TomJo2000 TomJo2000 force-pushed the subpkgs-script-triage branch from bb777b2 to c50670e Compare April 3, 2025 17:54
@TomJo2000 TomJo2000 force-pushed the subpkgs-script-triage branch 2 times, most recently from a8fd3ad to 3f840d1 Compare April 5, 2025 10:21
@TomJo2000
Copy link
Member Author

TomJo2000 commented Apr 5, 2025

The tree-wide rename of TERMUX_PKG_BLACKLISTED_ARCHES to TERMUX_PKG_EXCLUDED_ARCHES will also require an update to the wiki page for creating a new package.
https://github.com/termux/termux-packages/wiki/Creating-new-package#package-build-script-variables

@TomJo2000
Copy link
Member Author

I'd like to merge this in ~6 hours if there's no further comments.

Copy link
Contributor

@robertkirkman robertkirkman left a comment

Choose a reason for hiding this comment

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

I tested building several packages with this in several modes, and I have not seen any problems. There is no way to really test every single possible combination of modes in a short period of time, but it is probably completely working.

@TomJo2000
Copy link
Member Author

The s/BLACKLISTED/EXCLUDED is purely a name change.
So that can't cause any issues.

The rest of the changes to the subpackage scripts I have validated to preserve all observable previous semantics, but it's of course possible that I missed something.

None of these changes except the variable rename should change the public "API" of the build system.

local PKG_DEPS_SPC=" ${TERMUX_PKG_DEPENDS//,/} "
if [ -z "$TERMUX_SUBPKG_DEPEND_ON_PARENT" ] && [ "${PKG_DEPS_SPC/ $SUB_PKG_NAME /}" = "$PKG_DEPS_SPC" ] || [ "$TERMUX_SUBPKG_DEPEND_ON_PARENT" = "true" ]; then
# Does pacman supports versioned dependencies?
#TERMUX_SUBPKG_DEPENDS+=", $TERMUX_PKG_NAME (= $TERMUX_PKG_FULLVERSION)"
Copy link
Contributor

Choose a reason for hiding this comment

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

well, I saw that this part appears to change whether the pacman TERMUX_SUBPKG_DEPEND_ON_PARENT=true subpackage has a versioned dependency on the pacman parent package or a non-versioned dependency, but maybe you checked to be sure this is completely working. It looks fine to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to check with @Maxython just to be sure.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hold on, you're right I copied the case here over from the debian subpackage script when I should have included that note from the if statement.

Copy link
Member Author

@TomJo2000 TomJo2000 Apr 5, 2025

Choose a reason for hiding this comment

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

To answer the question in the comment by the way, yes pacman supports versioned dependencies.
https://wiki.archlinux.org/title/PKGBUILD#Dependencies

Example from my PC.
image

But I think that's best left as a matter for another PR, this one isn't meant to change existing behavior.

I'll adjust the comment to make this a TODO.

@TomJo2000 TomJo2000 force-pushed the subpkgs-script-triage branch from 3f840d1 to 5b6e7c2 Compare April 5, 2025 19:30
@TomJo2000 TomJo2000 force-pushed the subpkgs-script-triage branch from 5b6e7c2 to 79d1659 Compare April 5, 2025 19:33
@TomJo2000 TomJo2000 merged commit 3203a79 into termux:master Apr 5, 2025
@TomJo2000 TomJo2000 deleted the subpkgs-script-triage branch April 5, 2025 19:40
@TomJo2000
Copy link
Member Author

Wiki has been updated, I have also documented TERMUX_SUBPKG_EXCLUDED_ARCHES, which was previously missing.
https://github.com/termux/termux-packages/wiki/Creating-new-package#package-build-script-variables

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.

3 participants