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

transmission version: check with SHA & allow git push to trigger build workflow #2843

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from

Conversation

ilike2burnthing
Copy link
Contributor

Breaking change

None

Proposed change

@ksurl & @pkishino

Looking at the commits for this on the dev branch, there appear to be two problems:

  1. repeated changes between no version number and the current 4.0.5
  2. the Actions bot commits do not trigger a new build

A healthy job fetches ~25000B, whereas an unhealthy job is ~100 times less than that, so I'm guessing some sort of error page (there's no output to confirm).

Adding the additional check, that LATEST_VERSION isn't null or empty, should prevent these pointless bot commits.


While unlikely, Transmission could publish a new build (e.g. 4.0.6), we pull and build with it, they delete it (say because of a security hotfix), and they publish a new 4.0.6 build. The workflow would not see this as a new build, and we'd be left using the vulnerable build. Switching to the commit SHA avoids this issue.

Alternatively, we could use one of the following:

Published Date

curl -L https://api.github.com/repos/transmission/transmission/releases/latest | awk '/published_at/ {print $2}' | sed -e 's/[",]//g'

Release ID

curl -L https://api.github.com/repos/transmission/transmission/releases/latest | tac | tac | awk '/id/ {print $2; exit}' | sed -e 's/[",]//g'

You can see the results of the workflow here:


Currently, the git push here does not trigger the Transmission Builds workflow. This requires the use of a personal access token with repo rights, and which would need to be set up as a repository secret with the name PAT, before committing this PR.

I don't know if that would also be need for docker-transmission-builds.yml, in order to trigger the Image Builds workflow, I can't test that out.


Questions, comments, whatever; let me know.

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (which adds functionality to this container)
  • Breaking change (fix/feature causing existing functionality to break)

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to PR: relates to #2691
  • Link to documentation updated (if done separately): https://...

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.

If user exposed functionality or configuration variables are added/changed:

  • Documentation added/updated

@ksurl
Copy link
Contributor

ksurl commented May 21, 2024

Good point about overwritten version releases, but anyone doing that is a terrible practice for precisely this reason. It is typical to release fixes for pulled versions as a new version even if the old one is no longer available. Has transmission done this in the past?

@ilike2burnthing
Copy link
Contributor Author

Yea, I've had to upload hotfixes before, and even if I'd deleted the originals, they would still have been distinct (v3.3.14, v3.3.14-hotfix, v3.3.14-hotfix2) - https://github.com/FlareSolverr/FlareSolverr/releases

While I don't think Transmission ever have or would do it, it's best to avoid the risk if we can.

kinda off-topic

I've a vague recollection of a Jackett build half failing for us, and we just re-ran the job. It had only been public for 2mins, but it was the same version number. Not a great example of high standards, but CCleaner pulled a build that was bugged to all hell, and a few days later re-released it, same version number.

Also, bit of an aside, but WinRAR has terrible versioning when it comes their beta builds. You can currently download v7.0.0.0, v7.0.1.0, and v7.1.0.0.
v7.0.1.0 was the beta, and then you 'updated' to v7.0.0.0 🙄

Long story short, sometimes even sensible people do stupid things.

But otherwise, does that look ok to you? Do you think docker-transmission-builds.yml would need to use the personal access token as well?

@ksurl
Copy link
Contributor

ksurl commented May 23, 2024

the code looks good to me for extracting the sha. I'm not sure if token is required or not. can always add it on next release of transmission if the commit fails

@ilike2burnthing
Copy link
Contributor Author

@pkishino are you able to add the secret and merge this? Thanks

@ilike2burnthing
Copy link
Contributor Author

ilike2burnthing commented Jun 6, 2024

Whoops, also requires #2849, as on schedule can only be run on master branch.

I'm also going to roll back the three upstream txt files to 4.0.5, so we can test if this actually works when merged, without having to manually trigger anything.

@pkishino
Copy link
Collaborator

pkishino commented Jun 7, 2024

I’ll take a look at this later, but as I’m not the owner of this repo I don’t think I can create a pat with the great necessary rights..

@ilike2burnthing
Copy link
Contributor Author

Ah, good point. My guess would be that if you can write to a repo, then your PAT should be able to be used to do so as well, but I haven't tried it, 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.

None yet

3 participants