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

Conversation

@TomJo2000
Copy link
Member

@TomJo2000 TomJo2000 commented Apr 16, 2025

Based on discussion in the Discord/Matix room by user n-ce

This PR changes TERMUX_PKG_RECOMMENDS entries to TERMUX_PKG_SUGGESTS where the optional dependencies in question should not be installed by default when installing that package.

Notes:

  • geckodriver has TERMUX_PKG_RECOMMENDS="firefox" set meaning it will try to install it when installing geckodriver.
    firefox is in the x11 repo, which may not be installed since the geckodriver package is in the main repository channel.
    There's 3 possible solutions I see to this issue
    • 1.) Keep it as is (adding a comment in the build script pointing out the potential issue)
    • 2.) Changing the RECOMMENDS to a SUGGESTS so it doesn't default to trying to install firefox as a dependency
    • 3.) Move geckodriver to the X11 repo channel, this wouldn't be ideal either since it's only X11 dependency is non-absolute.
      Commit omitted to not fail CI run.
  • glow
    I've done more of a general rewrite for this build script since it was missing its man page and shell completions.
    git is only used by glow for file discovery purposes inside Git repos, thus doesn't warrant being pulled in by it.
    If a user is using glow in a git repo, it's a fair bet they'll also have installed Git.
  • qbittorrent/transmission
    jackett is a rather large, optional, dependency for the BitTorrent clients and shouldn't be pulled in automatically.
  • w3m
    libsixel is rather non-essential to the usage of w3m
    especially with Add graphics in terminal support: - Sixel and iTerm2 protocols termux-app#2973 not yet merged.
  • openssh
    while there is a sshd and ssh-agent service for openssh,
    it automatically pulling in termux-services (and by extension runit)
    may be unexpected or undesired if the user does not intend to make use of the provided services.
    The existence and method of enabling the services is already laid out in openssh's postinst script.
    And it is probably better to make opting into using termux-services a conscious decision on the user's part.

@TomJo2000 TomJo2000 requested a review from Grimler91 as a code owner April 16, 2025 10:40
@TomJo2000 TomJo2000 force-pushed the cleanup-recommends branch from 0cdebd5 to 5eaf827 Compare April 16, 2025 10:41
Copy link
Member

@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 was wondering about this too, thank you for fixing it.

@TomJo2000
Copy link
Member Author

I also clarified the descriptions of the two variables in:
https://github.com/termux/termux-packages/wiki/Creating-new-package#package-build-script-variables
(And documented their SUBPKG equivalents)

Copy link
Member

@twaik twaik left a comment

Choose a reason for hiding this comment

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

LGTM.

@TomJo2000
Copy link
Member Author

Actually this'll be a good test for the artifact caching, let me rebase the branch quick to make sure that's up to date.

@TomJo2000 TomJo2000 force-pushed the cleanup-recommends branch from 5eaf827 to 3fb9da0 Compare April 18, 2025 09:54
@twaik
Copy link
Member

twaik commented Apr 18, 2025

I did not implement cached artifact reusing. I only added detecting if there is workflow run which contains suitable artifact. I want to monitor it for a while and when it will be ready I will implement artifact downloading and injecting as well.

@TomJo2000
Copy link
Member Author

I did not implement cached artifact reusing. I only added detecting if there is workflow run which contains suitable artifact. I want to monitor it for a while and when it will be ready I will implement artifact downloading and injecting as well.

So this will still build every package in this PR after I merge it?
I thought we already merged support for using a PR's last successful CI run as a cache for the packages updated by it.

@TomJo2000 TomJo2000 merged commit 573743e into termux:master Apr 18, 2025
9 checks passed
@TomJo2000 TomJo2000 deleted the cleanup-recommends branch April 18, 2025 10:16
@twaik
Copy link
Member

twaik commented Apr 18, 2025

So this will still build every package in this PR after I merge it?

Yep.

I thought we already merged support for using a PR's last successful CI run as a cache for the packages updated by it.

Before we merge it I want to monitor suitable artifact detecting for a while. I do not want to enable this feature until I make sure it is 100% safe.
I already caught few minor problems of the script with both push and pull_request events so I am not sure we will enable it this week.

But injecting last successful artifact content should be easy to implement. It is simply downloading and unpacking zip and tar archives to some temporary folder, injecting debs into regular or docker buildsystem cache (and output/ directory as well) and populating .built-packages.txt. Or at least I hope so.

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