-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Cleanup recommends #24351
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
Cleanup recommends #24351
Conversation
0cdebd5 to
5eaf827
Compare
robertkirkman
left a comment
There was a problem hiding this 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.
|
I also clarified the descriptions of the two variables in: |
twaik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
|
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. |
- chore: clean up build script - fix: `RECOMMENDS` -> `SUGGESTS`
5eaf827 to
3fb9da0
Compare
|
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? |
Yep.
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. 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 |
Based on discussion in the Discord/Matix room by user
n-ceThis PR changes
TERMUX_PKG_RECOMMENDSentries toTERMUX_PKG_SUGGESTSwhere the optional dependencies in question should not be installed by default when installing that package.Notes:
geckodriverhasTERMUX_PKG_RECOMMENDS="firefox"set meaning it will try to install it when installinggeckodriver.firefoxis in thex11repo, which may not be installed since thegeckodriverpackage is in the main repository channel.There's 3 possible solutions I see to this issue
RECOMMENDSto aSUGGESTSso it doesn't default to trying to installfirefoxas a dependencygeckodriverto 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.
glowI've done more of a general rewrite for this build script since it was missing its man page and shell completions.
gitis only used byglowfor file discovery purposes inside Git repos, thus doesn't warrant being pulled in by it.If a user is using
glowin a git repo, it's a fair bet they'll also have installed Git.qbittorrent/transmissionjackettis a rather large, optional, dependency for the BitTorrent clients and shouldn't be pulled in automatically.w3mlibsixelis rather non-essential to the usage ofw3mespecially with Add graphics in terminal support: - Sixel and iTerm2 protocols termux-app#2973 not yet merged.
opensshwhile there is a
sshdandssh-agentservice foropenssh,it automatically pulling in
termux-services(and by extensionrunit)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-servicesa conscious decision on the user's part.