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

api: add add_external_repo and rm_external_repo #1902

Merged
merged 7 commits into from
Jan 1, 2024

Conversation

ryanfortner
Copy link
Collaborator

@ryanfortner ryanfortner commented May 28, 2022

As suggested in #1889, this is a function that adds external apt repositories when provided:

  • an external repo path (such as https://ryanfortner.github.io/box64-debs/debian ./)
  • a repo name (such as box64)
  • a url to the repo's gpg pubkey (such as https://ryanfortner.github.io/box64-debs/KEY.gpg)

Example of the usage of these functions:

add_external_repo "https://ryanfortner.github.io/box64-debs/debian ./" "box64" "https://ryanfortner.github.io/box64-debs/KEY.gpg" || exit 1

rm_external_repo "box64" || exit 1 # this will remove the repo if nothing is used in it
rm_external_repo "box64" "force" || exit 1 # this will remove the repo even if software is installed from it

This is my first try at creating a function for pi-apps and I would appreciate any feedback I receive!

@theofficialgman
Copy link
Collaborator

We already have a remove repo if unused function. You should extend that to remove the key if used as well.

pi-apps/api

Line 214 in d20a633

remove_repofile_if_unused() { #Given a sources.list.d file, delete it if nothing from that repository is currently installed. Deletion skipped if $2 is 'test'

api Outdated Show resolved Hide resolved
api Outdated Show resolved Hide resolved
api Outdated Show resolved Hide resolved
api Outdated Show resolved Hide resolved
api Outdated Show resolved Hide resolved
api Outdated Show resolved Hide resolved
@ryanfortner
Copy link
Collaborator Author

I just made a few changes. If everything looks good, I can go ahead and change the scripts for each of the apps that need the functions in this branch, so we can get it all done at once.

api Outdated Show resolved Hide resolved
api Outdated Show resolved Hide resolved
api Outdated Show resolved Hide resolved
@ryanfortner
Copy link
Collaborator Author

Alright @theofficialgman I changed the rm_external_repo function to be more like the remore_repofile_if_unused function.

Just in case the repo would need to be removed no matter what, I added a force option where the repo will be removed regardless of what packages are installed from it. (see the top comment)

@theofficialgman
Copy link
Collaborator

theofficialgman commented Jun 3, 2022

this might be overkill, but spaces are not allowed in apt repo .lists filename or keyname. this function doesn't do anything to prevent the script writer from making that mistake or document it.

at minimum some documents should be made to specify that reponame (the second input) should never contain spaces

@ryanfortner
Copy link
Collaborator Author

I added a check for spaces and updated documentation.

api Outdated Show resolved Hide resolved
api Outdated Show resolved Hide resolved
api Outdated Show resolved Hide resolved
@theofficialgman theofficialgman marked this pull request as draft June 11, 2022 20:18
@theofficialgman
Copy link
Collaborator

theofficialgman commented Jun 11, 2022

marking this as draft since I have been doing some testing and found that some repos need the signed-by to specify the key location, and some others don't need dearmor (depending on if the key is hardened or not)... this is going to be complicated to do universally because of this and it might be best to have a set of guidelines and something done on a script by script basis

extensive documentation here: https://wiki.debian.org/DebianRepository/UseThirdParty

if you read the documentation, you will find that repo/keyname combination is also standardized, so we need to use this as well instead of making out own format

@theofficialgman
Copy link
Collaborator

Deb822 format (.sources) has been supported for many years now (and all modern distros are switching to it) and it is probably best that if we were to add functions for adding and removing repos that we also do this with the Deb822 format rather than the older .list format.

https://manpages.debian.org/bookworm/apt/sources.list.5.en.html#THE_DEB_AND_DEB-SRC_TYPES:_GENERAL_FORMAT

One nice thing about deb822 is it supported external public key files as well as embedding the public key directly within the .sources file.

@theofficialgman theofficialgman force-pushed the ryanfortner-key-add branch 2 times, most recently from 9aa632b to e523164 Compare December 28, 2023 01:28
@theofficialgman
Copy link
Collaborator

theofficialgman commented Dec 28, 2023

I have revived this PR.
However the test case in the initial comment #1902 (comment) will not work since the order of options changed to better follow intuition

@theofficialgman

This comment was marked as outdated.

@theofficialgman theofficialgman force-pushed the ryanfortner-key-add branch 2 times, most recently from 8b08d49 to b290182 Compare December 28, 2023 02:30
@theofficialgman theofficialgman marked this pull request as ready for review December 28, 2023 02:32
@theofficialgman
Copy link
Collaborator

I have switched all applications that can be switched to use these generic functions.

The other applications (vivaldi and brave) cannot use these generic functions as they use their own custom non-standard key and repofile names and update them themselves as part of other debs.

@theofficialgman theofficialgman force-pushed the ryanfortner-key-add branch 2 times, most recently from 786afc6 to fa0c563 Compare January 1, 2024 04:11
@theofficialgman
Copy link
Collaborator

theofficialgman commented Jan 1, 2024

A new test case (based on deb822 edits):

add_external_repo "adoptium" "https://adoptium.jfrog.io/artifactory/api/security/keypair/default-gpg-key/public" "https://adoptium.jfrog.io/artifactory/deb" "bionic" "main" || exit 1

rm_external_repo "adoptium" || exit 1 # this will remove the repo if nothing is used in it
rm_external_repo "adoptium" "force" || exit 1 # this will remove the repo even if software is installed from it

the original test case now becomes:

add_external_repo "box64" "https://ryanfortner.github.io/box64-debs/KEY.gpg" "https://ryanfortner.github.io/box64-debs/debian" "./" || exit 1

rm_external_repo "box64" || exit 1 # this will remove the repo if nothing is used in it
rm_external_repo "box64" "force" || exit 1 # this will remove the repo even if software is installed from it

…_repo`

also extend `remove_repofile_if_unused` to be able to read .sources files in addition to .list files
@theofficialgman
Copy link
Collaborator

@ryanfortner thanks for your original contribution! I will merge the current iteration of this PR. Sorry it took so long to get back around to this.

Between when this PR was opened and now pi-apps moved documentation from this repo to our website so I will rewrite that information there soon.

@theofficialgman theofficialgman merged commit d7ea689 into master Jan 1, 2024
25 checks passed
@theofficialgman theofficialgman deleted the ryanfortner-key-add branch January 1, 2024 21:28
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.

5 participants