Bug 2436366

Copr build:
https://copr.fedorainfracloud.org/coprs/build/10093780
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2436366-forgejo-runner/fedora-rawhide-x86_64/10093780-forgejo-runner/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Taking this review.

Initial spec review:

> Source0:        https://code.forgejo.org/forgejo/runner/archive/v%{version}.tar.gz

This should be restructured so that the tarball is properly named. Something like "%{url}/archive/v%{version}.tar.gz#/%{name}-%{version}.tar.gz"

You can see an example in Quickshell: https://src.fedoraproject.org/rpms/quickshell/blob/rawhide/f/quickshell.spec

> Requires:       git

Does it require *all* of git or just git-core and a few other things? Also, even "git" would not be enough to ensure things like git-lfs are pulled in, right?

> %setup -n runner -q -a1
> %autopatch -p1

Can't this be simplified to "%autosetup -C -a1 -p1"?

> %doc examples README.md RELEASE-NOTES.md release-notes/757.md

Is it always going to 757.md? Should you just include the release-notes folder entirely instead?

> %attr(755, root, root) %{_bindir}/forgejo-runner
> %config(noreplace) %attr(644, root, root) %{_sysconfdir}/forgejo-runner/config.yml

"root, root" is already default, you can use "-, -" here instead.

> %config(noreplace) %attr(755, runner, runner) %{_sharedstatedir}/runner/

I don't love that the user+group and homedir are just called "runner". Can we call this "forgejo-runner" instead?

(In reply to Neal Gompa from comment #3)
> Initial spec review:
> 
> > Source0:        https://code.forgejo.org/forgejo/runner/archive/v%{version}.tar.gz
> 
> This should be restructured so that the tarball is properly named. Something
> like "%{url}/archive/v%{version}.tar.gz#/%{name}-%{version}.tar.gz"
> 
> You can see an example in Quickshell:
> https://src.fedoraproject.org/rpms/quickshell/blob/rawhide/f/quickshell.spec
> 

Sure, cleaned up

> > Requires:       git
> 
> Does it require *all* of git or just git-core and a few other things? Also,
> even "git" would not be enough to ensure things like git-lfs are pulled in,
> right?
> 

The docs about requiring git [0], but checking how it's being used [1], core and lfs are enough to cover the usage.

> > %setup -n runner -q -a1
> > %autopatch -p1
> 
> Can't this be simplified to "%autosetup -C -a1 -p1"?
> 

Sure, cleaned up

> > %doc examples README.md RELEASE-NOTES.md release-notes/757.md
> 
> Is it always going to 757.md? Should you just include the release-notes
> folder entirely instead?
> 

Yeah, makes sense, changed

> > %attr(755, root, root) %{_bindir}/forgejo-runner
> > %config(noreplace) %attr(644, root, root) %{_sysconfdir}/forgejo-runner/config.yml
> 
> "root, root" is already default, you can use "-, -" here instead.
> 

Cleaned up

> > %config(noreplace) %attr(755, runner, runner) %{_sharedstatedir}/runner/
> 
> I don't love that the user+group and homedir are just called "runner". Can
> we call this "forgejo-runner" instead?

TBH, I kept it as "runner" since it is how upstream documented it's usage, but I agree that it makes more sense to use "forgejo-runner" instead.
Changed.

Also updated the package to 12.7.0

[0] https://forgejo.org/docs/next/admin/actions/runner-installation/#system-requirements
[1] https://code.forgejo.org/forgejo/runner/src/commit/94ddcf7de6e7bf7a8e193bd9c3a82d864250371b/act/common/git/git.go#L433-L494

Spec URL: https://codeberg.org/dherrerace/forgejo-runner/src/branch/main/forgejo-runner.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/dherrera/forgejo-runner/srpm-builds/10153920/forgejo-runner-12.7.0-1.src.rpm

Created attachment 2130238 [details]
The .spec file difference from Copr build 10093780 to 10154266

Copr build:
https://copr.fedorainfracloud.org/coprs/build/10154266
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2436366-forgejo-runner/fedora-rawhide-x86_64/10154266-forgejo-runner/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Package was generated with go2rpm, simplifying the review.

✅ package contains only permissible content
✅ package builds and installs without errors on rawhide
✅ test suite is run and all unit tests pass
✅ latest version packaged
✅ license matches upstream specification and is acceptable for Fedora
✅ license file is included with %license in %files
✅ package complies with Go Packaging Guidelines

Package APPROVED.

===

Recommended post-import tasks:

- set up package on release-monitoring.org:
https://release-monitoring.org/project/378107/

- add @go-sig with "commit" access as package co-maintainer

- track package in koschei for all built branches
  (should happen automatically once go-sig is co-maintainer)

The Pagure repository was created at https://src.fedoraproject.org/rpms/forgejo-runner

FEDORA-2026-55e95c085e (forgejo-runner-12.7.0-1.fc45) has been submitted as an update to Fedora 45.
https://bodhi.fedoraproject.org/updates/FEDORA-2026-55e95c085e

FEDORA-2026-55e95c085e (forgejo-runner-12.7.0-1.fc45) has been pushed to the Fedora 45 stable repository.
If problem still persists, please make note of it in this bug report.