[Bug 2239662] New: Review Request: golang-github-katalix-l2tp - L2TP library and daemons built using golang
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
Bug ID: 2239662 Summary: Review Request: golang-github-katalix-l2tp - L2TP library and daemons built using golang Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: tparkin@katalix.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://github.com/katalix/go-l2tp-fedora/blob/master/golang-github-katalix-... SRPM URL: https://github.com/katalix/go-l2tp-fedora/blob/master/golang-github-katalix-... COPR URL: https://copr.fedorainfracloud.org/coprs/tomparkin/golang-github-katalix-go-l... Description: go-l2tp is a library for building L2TP applications on Linux using the Go language. It comes bundled with some working daemons: one of these, kl2tpd, is used by default by the NetworkManager-l2tp VPN plugin (https://github.com/nm-l2tp/NetworkManager-l2tp).
I am hoping to submit the Fedora package to make it easier for users of nm-l2tp to use kl2tpd without having to download and build the Go sources themselves.
Fedora Account System Username: tomparkin
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #1 from Tom Parkin tparkin@katalix.com --- I should have added that this is my first package submission for Fedora, and I am seeking sponsorship.
My Pagure package-sponsors ticket is here: https://pagure.io/packager-sponsors/issue/592
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
Pavel Solovev daron439@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Doc Type|--- |If docs needed, set a value CC| |daron439@gmail.com
--- Comment #2 from Pavel Solovev daron439@gmail.com --- please provide direct links to spec and srpm so that they are picked by the fedora-review tool correctly (eg. https://raw.githubusercontent.com/katalix/go-l2tp-fedora/master/golang-githu...)
BuildRequires: ... aren't needed, handeled by %go_generate_buildrequires
manpages are installed in %{_mandir}/man1 %{_mandir}/man5, executable bit isn't needed
Packagers SHOULD NOT simply glob everything under a shared directory https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_lists
--- /proc/self/fd/14 2023-09-20 13:33:13.080116090 +0300 +++ golang-github-katalix-l2tp.spec 2023-09-20 13:32:37.091307757 +0300 @@ -21,14 +21,6 @@ URL: %{gourl} Source: %{gosource}
-BuildRequires: golang(github.com/go-kit/kit/log) -BuildRequires: golang(github.com/go-kit/kit/log/level) -BuildRequires: golang(github.com/mdlayher/genetlink) -BuildRequires: golang(github.com/mdlayher/netlink) -BuildRequires: golang(github.com/mdlayher/netlink/nlenc) -BuildRequires: golang(github.com/pelletier/go-toml) -BuildRequires: golang(golang.org/x/sys/unix) - %description %{common_description}
%gopkg @@ -49,13 +41,13 @@ %gopkginstall install -m 0755 -vd %{buildroot}%{_bindir} install -m 0755 -vp %{gobuilddir}/bin/* %{buildroot}%{_bindir}/ -install -m 0755 -vd %{buildroot}%{_mandir}/1 -install -m 0755 -vp %{_builddir}/go-l2tp-%{version}/doc/kl2tpd.1 %{buildroot}%{_mandir}/1 -install -m 0755 -vp %{_builddir}/go-l2tp-%{version}/doc/kpppoed.1 %{buildroot}%{_mandir}/1 -install -m 0755 -vp %{_builddir}/go-l2tp-%{version}/doc/ql2tpd.1 %{buildroot}%{_mandir}/1 -install -m 0755 -vd %{buildroot}%{_mandir}/5 -install -m 0755 -vp %{_builddir}/go-l2tp-%{version}/doc/kl2tpd.toml.5 %{buildroot}%{_mandir}/5 -install -m 0755 -vp %{_builddir}/go-l2tp-%{version}/doc/ql2tpd.toml.5 %{buildroot}%{_mandir}/5 +install -m 0755 -vd %{buildroot}%{_mandir}/man1 +install -m 0644 -vp %{_builddir}/go-l2tp-%{version}/doc/kl2tpd.1 %{buildroot}%{_mandir}/man1 +install -m 0644 -vp %{_builddir}/go-l2tp-%{version}/doc/kpppoed.1 %{buildroot}%{_mandir}/man1 +install -m 0644 -vp %{_builddir}/go-l2tp-%{version}/doc/ql2tpd.1 %{buildroot}%{_mandir}/man1 +install -m 0755 -vd %{buildroot}%{_mandir}/man5 +install -m 0644 -vp %{_builddir}/go-l2tp-%{version}/doc/kl2tpd.toml.5 %{buildroot}%{_mandir}/man5 +install -m 0644 -vp %{_builddir}/go-l2tp-%{version}/doc/ql2tpd.toml.5 %{buildroot}%{_mandir}/man5
%if %{with check} %check @@ -65,8 +57,10 @@ %files %license LICENSE %doc CHANGELOG.md README.md -%{_bindir}/* -%{_mandir}/*/* +%{_bindir}/kl2tpd +%{_bindir}/kpppoed +%{_bindir}/ql2tpd +%{_mandir}/man{1,5}/*.{1,5}*
%gopkgfiles
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #3 from Tom Parkin tparkin@katalix.com --- Thank you Pavel for your review!
I have updated my spec file and SRPM as you indicated.
Spec URL: https://raw.githubusercontent.com/katalix/go-l2tp-fedora/master/golang-githu... SRPM URL: https://raw.githubusercontent.com/katalix/go-l2tp-fedora/raw/master/golang-g...
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #4 from Pavel Solovev daron439@gmail.com --- Great! Unfortunately, it's not possible to build the package in rawhide because golang-opentelemetry-otel updates broke some stuff, so we will have to wait until it's fixed.
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #5 from Tom Parkin tparkin@katalix.com --- Ah, I see.
Do I need to do anything in the meantime or is it just a case of waiting until the golang-opentelemetry-otel issues are resolved?
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #6 from Pavel Solovev daron439@gmail.com --- Yes, you don't need to do anything, just wait. :)
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
Pavel Solovev daron439@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review+ Assignee|nobody@fedoraproject.org |daron439@gmail.com Status|NEW |POST
--- Comment #7 from Pavel Solovev daron439@gmail.com --- Golang Package Review ==============
This package was generated using go2rpm, which simplifies the review. Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
- [x] The specfile is sane. - [x] The License tag reflects the package contents and uses the correct identifiers. - [x] The package builds successfully in mock. - [x] Package is installable (checked by fedora-review). - [x] There are no relevant rpmlint errors. - [x] The package runs tests in %check. - [x] `%goipath` is set correctly. - [x] The package's binaries don't conflict with binaries already in the distribution. (Some Go projects include utility binaries with very generic names) - [x] There are no `%{_bindir}/*` wildcards in %files. (go2rpm includes these by default) - [x] The package complies with the Golang and general Packaging Guidelines.
Package approved! On import, don't forget to do the following:
- [ ] Add the package to release-monitoring.org - [ ] Give go-sig privileges (at least commit) on the package - [ ] Close the review bug by referencing its ID in the rpm changelog and the Bodhi ticket.
Thanks!
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #8 from Tom Parkin tparkin@katalix.com --- Hi Pavel,
I'm also submitting go-l2tp for Debian, and during the review process it was pointed out that the kl2tpd, ql2tpd, and kpppoed daemons need root permissions for creating the L2TP dataplane, and hence should be installed under /usr/sbin. Correspondingly the manpages should be in section 8 of the manual rather than section 1.
At this stage is it possible for me to post an updated specfile and srpm?
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
Pavel Solovev daron439@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review+ |fedora-review? Status|POST |ASSIGNED
--- Comment #9 from Pavel Solovev daron439@gmail.com --- Sure
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #10 from Tom Parkin tparkin@katalix.com --- Package updated to 0.1.6, and rebuilt using copr.
The main change is that the go-l2tp programs are now installed to /usr/sbin rather than /usr/bin. The former is more appropriate since the programs all require root permissions.
The manpages have been relocated from section 1 to section 8 correspondingly.
Spec URL: https://github.com/katalix/go-l2tp-fedora/blob/master/golang-github-katalix-... SRPM URL: https://github.com/katalix/go-l2tp-fedora/blob/master/golang-github-katalix-... COPR URL: https://copr.fedorainfracloud.org/coprs/tomparkin/golang-github-katalix-go-l...
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #11 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6543455 (failed)
Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please make sure the package builds successfully at least for Fedora Rawhide.
- If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field
--- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #12 from Tom Parkin tparkin@katalix.com --- Spec URL: https://raw.githubusercontent.com/katalix/go-l2tp-fedora/master/golang-githu... SRPM URL: https://raw.githubusercontent.com/katalix/go-l2tp-fedora/raw/master/golang-g...
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #13 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6543484 (failed)
Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please make sure the package builds successfully at least for Fedora Rawhide.
- If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field
--- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #14 from Tom Parkin tparkin@katalix.com --- Spec URL: https://raw.githubusercontent.com/katalix/go-l2tp-fedora/master/golang-githu... SRPM URL: https://github.com/katalix/go-l2tp-fedora/raw/master/golang-github-katalix-l...
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #15 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6543550 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
Pavel Solovev daron439@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ Status|ASSIGNED |POST
--- Comment #16 from Pavel Solovev daron439@gmail.com --- Looks good, reapproved! Source checksums don't match but there's no file-specific differences. Download sources with spectool -g ./golang-github-katalix-l2tp.spec
Source checksums ---------------- https://github.com/katalix/go-l2tp/archive/v0.1.6/go-l2tp-0.1.6.tar.gz : CHECKSUM(SHA256) this package : 965cf69f545417f94019a1ad3ad275b8b649908c028ca125867d21df1fbcf711 CHECKSUM(SHA256) upstream package : 2fd2c58f58ea69252272eb30c9066bed98ad70241b43b6e500fe4ebb2dee2628
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #17 from Tom Parkin tparkin@katalix.com --- Thank you Pavel.
Apologies for the checksum delta. I think that's down to process issues -- I was generating tarballs using "git archive", but I'll take the github tarball in future.
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #18 from Tom Parkin tparkin@katalix.com --- Hi Pavel,
Sorry to bother you with process questions, but I wonder if you could guide me?
I was reviewing the Fedora process documentation[1], and it's not clear to me whether I need to wait to get sponsored prior to setting up a git repo for the project using fedpkg, and getting set up for Koji builds, etc.
I wonder whether it's appropriate/possible for me to work on this while I'm waiting for a package sponsor? Or should I get a sponsor first and carry on with the rest of the process at that point?
Thanks in advance for any insights :-)
[1] https://docs.fedoraproject.org/en-US/package-maintainers/Package_Review_Proc...
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #19 from Pavel Solovev daron439@gmail.com --- Yes, you need to be in the packager group(get sponsored) to be able to request repos and upload sources. I think you may get sponsored faster if you ask about it in #fedora-devel and/or #fedora-golang IRC/Matrix.
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #20 from Tom Parkin tparkin@katalix.com --- Upstream has made a couple of updates to go-l2tp:
* 0.1.7 disables IPv6 encap tests for the transport, these are currently not working following a kernel regression * 0.1.8 addresses two issues reported by a user of NetworkManager-l2tp when attempting to make a VPN connection with a Cisco Meraki system
I've updated the specfile and srpm to contain the updated code.
Spec URL: https://raw.githubusercontent.com/katalix/go-l2tp-fedora/master/golang-githu... SRPM URL: https://github.com/katalix/go-l2tp-fedora/raw/master/golang-github-katalix-l...
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
Paul Wouters paul.wouters@aiven.io changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |paul.wouters@aiven.io
--- Comment #21 from Paul Wouters paul.wouters@aiven.io --- The spec file looks fine. Maybe also try to use the %autochangelog stuff - at least not commit as John Doe :)
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #22 from Tom Parkin tparkin@katalix.com --- (In reply to Paul Wouters from comment #21)
The spec file looks fine. Maybe also try to use the %autochangelog stuff - at least not commit as John Doe :)
Thanks Paul. I'm not sure what specifically I need to do wrt to %autochangelog.
I reviewed the RPM docs[1] and I think using %autorelease and %autochangelog in the .spec file should be sufficient. Do I need anything further?
[1]. https://docs.pagure.org/Fedora-Infra.rpmautospec/autochangelog.html
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
Pavel Solovev daron439@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review+ |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
Pavel Solovev daron439@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
--- Comment #23 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/golang-github-katalix-l2tp
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |MODIFIED
--- Comment #24 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-046ead2d21 (golang-github-katalix-l2tp-0.1.8-1.fc41) has been submitted as an update to Fedora 41. https://bodhi.fedoraproject.org/updates/FEDORA-2024-046ead2d21
https://bugzilla.redhat.com/show_bug.cgi?id=2239662
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |ERRATA Status|MODIFIED |CLOSED Last Closed| |2024-07-08 18:44:32
--- Comment #25 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-046ead2d21 (golang-github-katalix-l2tp-0.1.8-1.fc41) has been pushed to the Fedora 41 stable repository. If problem still persists, please make note of it in this bug report.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com