-
Notifications
You must be signed in to change notification settings - Fork 226
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
Autobuild: Create Debian repository on release #3013
Conversation
run: | | ||
set -eu | ||
|
||
[[ "${GPG_PRIVATE_KEY:-}" ]] || { |
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.
Forks which don't have a GPG key set, will have a failed build.
I'm not sure if a silent failure would be better. Skipping the repo build should be possible.
There are multiple options:
- Remove the whole job (easy)
- Introduce a variable enabling/disabling the repo creation
- If GPG_PRIVATE_KEY is not present, skip the job. We still need to spin up the container, I'm afraid. GitHub dosn't support checking for present secrets in the .yaml file directly as far as I understand
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.
Will needs.create_release.outputs.publish_to_release == 'true'
be true? Is that what you mean by "failed build"?
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.
Yes. If they publish a release it will be true and the job will fail.
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.
Ok. Maybe my new approach works.
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.
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.
That run demonstrates that skipping the job works if the GPG_PRIVATE_KEY is not set. I can't see a run that demonstrates the action is working properly when the private key IS set.
I'm assuming that the main jamulus repo has a private key, but the ann0see repo does not. But to test this before merging, I guess either ann0see's repo needs a (temporary?) private key, or perhaps we need to merge these changes into a branch on the jamulus repo, so that it can use the jamulus repo secrets?
I don't profess to be an expert (or even moderately knowledgeable) in Debian packaging or repos. Most of my packaging experience is with RPMs on RH-based systems.
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.
See https://github.com/ann0see/jamulus/releases/tag/r3_9_1devrepo
That's a working (?) repo
mkdir -p gpghome | ||
chmod 700 gpghome | ||
echo "${GPG_PRIVATE_KEY}" | gpg --homedir gpghome --import - | ||
# Unfortunately download-artifact action doesn't support wild card downloads. Thus downloading all artifacts |
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.
While this approach is not perfect, I think it's easier to download all artifacts instead of one step per deb file. In future wildcards for downloading only .deb files might be supported.
Alternative: use gh command in CLI
path: releasedl/ | ||
- name: Create debian repository | ||
run: | | ||
set -eu |
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.
Could be in a separate .sh file. Hoffie once said he's in favor of inlining.
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.
It's easier to review in context although it makes the autobuild.yml
grow as it becomes more feature-rich. Maybe we need to have a think about it.
|
||
- name: Upload Packages file to release | ||
id: deb-upload-packagesfile | ||
uses: actions/upload-release-asset@v1 |
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.
release-asset is outdated. Changing to the other action might need some work.
GITHUB_REPOSITORY="jamulussoftware/jamulus" | ||
|
||
echo "Setting up Jamulus repo at ${REPO_FILE}..." | ||
echo "deb https://github.com/${GITHUB_REPOSITORY}/releases/latest/download/ ./" > ${REPO_FILE} |
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.
This uses a trick: latest points to the last release tagged as "stable" by GitHub. Thus, the link remains unchanged, but a redirect guarantees the correct source.
@@ -0,0 +1,32 @@ | |||
#!/bin/bash |
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.
Where should we host this file? Is this repo good or should it be on the website?
I believe this is now ready for testing and review |
244b1e0
to
691001e
Compare
@pljones Do you know anyone who knows a bit more about Debian packaging or someone who can review this approach? |
Sorry, no - for Linux, I've pretty much been a consumer for packaging technologies. I've no development experience there, nor know anyone who has. (At work we use RPMs and don't even use standard deployment for those, so totally unhelpful here!) |
Ok. mirabilos was the Debian maintainer, but he stepped back so I'm not sure if he'd want to review this. ignotus has some knowledge of bash scripts, but I don't think he knows deb packaging. @trebmuh might know a bit as he develops a distribution. |
I've seen this review request, and will review it in the next day or two. |
Thank you! |
echo "Setting up Jamulus repo at ${REPO_FILE}..." | ||
echo "deb https://github.com/${GITHUB_REPOSITORY}/releases/latest/download/ ./" > ${REPO_FILE} | ||
echo "Installing Jamulus GPG key at ${KEY_FILE}..." | ||
curl --fail --show-error -sLo "${KEY_FILE}" https://github.com/${GITHUB_REPOSITORY}/releases/latest/download/key.asc |
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.
Just trying this script locally, and this step fails with a 404 for key.asc
.
Is this script intended for use by a Github workflow, or by an end user? I can't see it referred to anywhere else in the tree.
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.
Is this script intended for use by a Github workflow, or by an end user? I can't see it referred to anywhere else in the tree.
Purely by our git build to create a repo for apt to auto update from.
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.
You'd need to replace the variables. The release will generate a runnable script.
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've currently checked out ann0see/debrepo
, but I can't see anywhere that calls setup_repo.sh
. I would have expected it to be referred to somewhere within autobuild.yml
? Or within another script somewhere in the tree?
You can tell I'm a bit rusty!
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.
setup_repo.sh is a script which the user should run on his machine to install the repository. The logic to create the repository is in autobuild.yml. I believe the content in autobuild.yaml correctly replaces the content of the setup_repo.sh script and uploads the modified version to the release.
Edit: I think I've removed the replacing logic. You'll need to change the GITHUB_REPOSITORY variable to ann0see/jamulus
Also my repo has a GPG key set. See e.g https://github.com/ann0see/jamulus/actions/runs/4493966363 |
Just checking: is there anything outstanding on this? Rebase? Does it push beta and pre releases to the repo? (I forgot to check.) |
The standard URL will point to the latest stable release. So even though the files for the Debian repo will get uploaded to beta releases users would need to set the respective direct link to beta/nightly releases. |
@softins Are you OK with a merge? Documentation will be added. |
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.
Ok. Will do a rebase and then merge it. We can always state that this is still experimental |
Co-authored-by: Christian Hoffmann <[email protected]>
Dropping needs documentation as I've opened jamulussoftware/jamuluswebsite#937 on the website repository. |
Short description of changes
I consider this approach easier as it is more automated and there's no need for a separate repo.
CHANGELOG: Linux: Debian users can now use the official PPA to get automatic updates. See the Linux install page on jamulus.io for more information.
Context: Fixes an issue?
Related to: #2122
Does this change need documentation? What needs to be documented and how?
Yes. We need to supply the link to the repository, probably using the redirect to the latest release by GitHub and a link to the GPG key on jamulus.io
Status of this Pull Request
Ready for review. Before this can be merged, we need to add a GPG key as secret.
What is missing until this pull request can be merged?
Add GPG key as secret.
Checklist