https://bugzilla.redhat.com/show_bug.cgi?id=1263941
Bug ID: 1263941 Summary: Review Request: tayga - Simple out-of-kernel stateless NAT64 daemon Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: ingvar@linpro.no QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: http://users.linpro.no/ingvar/tayga/tayga.spec SRPM URL: http://users.linpro.no/ingvar/tayga/tayga-0.9.2-1.fc22.src.rpm Description: Simple out-of-kernel stateless NAT64 daemon Fedora Account System Username: ingvar
TAYGA is an out-of-kernel stateless NAT64 implementation for Linux that uses the TUN driver to exchange IPv4 and IPv6 packets with the kernel. It is intended to provide production-quality NAT64 service for networks where dedicated NAT64 hardware would be overkill.
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
Ingvar Hagelund ingvar@linpro.no changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |william@firstyear.id.au
--- Comment #1 from Ingvar Hagelund ingvar@linpro.no --- *** Bug 1028206 has been marked as a duplicate of this bug. ***
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #2 from Ingvar Hagelund ingvar@linpro.no --- Additional info, or why NAT64?
With the support of a DNS64 server, tayga allows the translation of ipv4 addresses into an ipv6 prefix. This allows a network to be ipv6 only, while still maintaining contact with ipv4 networks.
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |ppisar@redhat.com
--- Comment #3 from Petr Pisar ppisar@redhat.com --- I suspect the spec file is missing various BuildRequires like coreutils, sed, make, gcc for executed commands.
It should build-require systemd for the %{_unitdir} macro. The %post and other scriptlets should run-require chkconfig or systemd respectively. See https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd.
The COPYING file should be packaged by %license macro instead of the %doc macro on Fedora.
I worry that that the LDFLAGS="%{optflags} -pie" ignores default LDFLAGS defined by rpmbuild.
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #4 from Ingvar Hagelund ingvar@linpro.no --- (In reply to Petr Pisar from comment #3)
Thank you, Petr
Updated src.rpm and specfile here:
http://users.linpro.no/ingvar/tayga/tayga-0.9.2-2.fc22.src.rpm http://users.linpro.no/ingvar/tayga/tayga.spec
I suspect the spec file is missing various BuildRequires like coreutils, sed, make, gcc for executed commands.
I don't think this is necessary. There is an implicit minimum build system on all fedora build systems that includes coreutils, binutils, gcc, make, etc. The packages do build fine in mock and koji without adding these explicitly.
It should build-require systemd for the %{_unitdir} macro. The %post and other scriptlets should run-require chkconfig or systemd respectively. See https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd.
Fixed
The COPYING file should be packaged by %license macro instead of the %doc macro on Fedora.
Fixed
I worry that that the LDFLAGS="%{optflags} -pie" ignores default LDFLAGS defined by rpmbuild.
Fixed
Ingvar
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #5 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- ingvar's scratch build of tayga-0.9.2-2.fc22.src.rpm for epel7 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11284416
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #6 from Jason Tibbitts tibbs@math.uh.edu --- Just so you know, the packaging guidelines include no implicit minimum build system and while I doubt it will actually happen, it's quite possible for gcc to not be there. Nobody is enforcing this currently, but if buildroot changes break your packages due to incompletely specified dependencies then you do get to keep all of the pieces.
Plus having the actual dependencies properly specified does make it easier for distro bootstrapping, though that is a minor thing for a leaf package such as this.
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #7 from Ingvar Hagelund ingvar@linpro.no --- Okay, I've added explicit buildreqs on gcc, make, and coreutils. Updated source package and spec, though not bumped release.
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #8 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- ingvar's scratch build of tayga-0.9.2-2.fc22.src.rpm for dist-6E-epel failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11284998
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #9 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- ingvar's scratch build of tayga-0.9.2-2.fc22.src.rpm for dist-6E-epel failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11285099
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #10 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- ingvar's scratch build of tayga-0.9.2-2.fc22.src.rpm for dist-6E-epel failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11285162
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #11 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- ingvar's scratch build of tayga-0.9.2-2.fc22.src.rpm for dist-6E-epel completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11285185
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
Lubomir Rintel lkundrak@v3.sk changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |lkundrak@v3.sk Assignee|nobody@fedoraproject.org |lkundrak@v3.sk Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #12 from Lubomir Rintel lkundrak@v3.sk --- * Named correctly * Versioned correctly * Packaging the latest version * License good for fedora * rpmlint mostly happy (see below) * Builds fine in mock * Dependencies sane * Fileslist sane * SPEC file clean and legible
0.) License tag not correct
License: GPLv2
It seems to be "GPLv2+" (there's an or later version clause).
1.) Please add comments that would describe the upstreaming status of the patches
Patch0: tayga-0.9.2_redhat_initscripts_and_systemd.patch Patch1: tayga-0.9.2_cflags_override.patch
Have you send them upstream? Is there a mailing list or bug tracker reference?
2.) Why do you turn off PIE on ppc64?
# PIE hardening seems to fail on ppc64
If this is really the case, please add a more descriptive comment (error output).
3.) You're missing the %defattr tag
It's not needed for current RPM, but seems like you're targetting old RHEL versions as well?
4.) You're installing a service with name of a templated service:
ln -s %{_unitdir}/%{name}@.service %{buildroot}%{_unitdir}/%{name}@example.service
This makes no sense. systemd would probably just ignore that. 'systemctl enable tayga@example' would make the link in the proper place; just drop that line.
5.) rpmlint complains:
tayga.x86_64: E: incorrect-fsf-address /usr/share/licenses/tayga/COPYING The Free Software Foundation address in this file seems to be outdated or misspelled. Ask upstream to update the address, or if this is a license file, possibly the entire file with a new copy available from the FSF.
This is something the upstream would need to fix; you may want to let them know. Obviously not a review blocker.
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #13 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- ingvar's scratch build of tayga-0.9.2-3.el7.src.rpm for epel7 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12546574
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #14 from Ingvar Hagelund ingvar@linpro.no --- Hello, Lubomir. Thanks for taking the review :-)
Updated src.rpm: https://ingvar.fedorapeople.org/tayga/tayga-0.9.2-3.fc23.src.rpm Updated specfile: https://ingvar.fedorapeople.org/tayga/tayga.spec
(In reply to Lubomir Rintel from comment #12)
0.) License tag not correct
License: GPLv2
It seems to be "GPLv2+" (there's an or later version clause).
Fixed. I have also asked upstream to add this to README or some other more visible file. Atm, it's only visible in the manpage.
1.) Please add comments that would describe the upstreaming status of the patches
Patch0: tayga-0.9.2_redhat_initscripts_and_systemd.patch Patch1: tayga-0.9.2_cflags_override.patch
Done
Have you send them upstream? Is there a mailing list or bug tracker reference?
Yes, they are sent upstream. I have not found any issue tracker nor mailing list for tayga. I have asked upstream for this as well, but not received any reply.
2.) Why do you turn off PIE on ppc64?
# PIE hardening seems to fail on ppc64
If this is really the case, please add a more descriptive comment (error output).
After a bit of debugging, I found this not to be a problem with PIE, but a difference on Red Hat's ppc64 and x86_64 koji builders. The ppc64 one does not take "-z now" in LDFLAGS, though the x86_64 one does. Changing to "-znow" seems to work, so I have removed the workaround.
3.) You're missing the %defattr tag
It's not needed for current RPM, but seems like you're targetting old RHEL versions as well?
Fixed.
4.) You're installing a service with name of a templated service:
ln -s %{_unitdir}/%{name}@.service %{buildroot}%{_unitdir}/%{name}@example.service
This makes no sense. systemd would probably just ignore that.
No, it treats it as a service.
'systemctl enable tayga@example' would make the link in the proper place; just drop that line.
Okay, dropped, but it would not be trivial for the non-seasoned systemd user to know how to enable the service. Should I add a README.RedHat with a note explaining this?
Also, Will the %systemd_preun %{name}@.service remove both the unit file and any generated symlinks?
5.) rpmlint complains:
tayga.x86_64: E: incorrect-fsf-address /usr/share/licenses/tayga/COPYING The Free Software Foundation address in this file seems to be outdated or misspelled. Ask upstream to update the address, or if this is a license file, possibly the entire file with a new copy available from the FSF.
This is something the upstream would need to fix; you may want to let them know. Obviously not a review blocker.
I did notice upstream about this as well.
Ingvar
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #15 from Upstream Release Monitoring upstream-release-monitoring@fedoraproject.org --- ingvar's scratch build of tayga-0.9.2-3.el5.src.rpm for dist-5E-epel completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12547542
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #16 from Ingvar Hagelund ingvar@linpro.no --- I've updated the spec with a README.redhat that describes how to set up multi instances of tayga, for both sysv and systemd, and updated the init scripts similarly. The package also now builds on epel5.
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
Lubomir Rintel lkundrak@v3.sk changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #17 from Lubomir Rintel lkundrak@v3.sk --- (In reply to Ingvar Hagelund from comment #14)
Hello, Lubomir. Thanks for taking the review :-)
Updated src.rpm: https://ingvar.fedorapeople.org/tayga/tayga-0.9.2-3.fc23.src.rpm Updated specfile: https://ingvar.fedorapeople.org/tayga/tayga.spec
(In reply to Lubomir Rintel from comment #12)
0.) License tag not correct
License: GPLv2
It seems to be "GPLv2+" (there's an or later version clause).
Fixed. I have also asked upstream to add this to README or some other more visible file. Atm, it's only visible in the manpage.
Well, it's visible in the comments in the source code, which is what actually matters.
1.) Please add comments that would describe the upstreaming status of the patches
Patch0: tayga-0.9.2_redhat_initscripts_and_systemd.patch Patch1: tayga-0.9.2_cflags_override.patch
Done
Have you send them upstream? Is there a mailing list or bug tracker reference?
Yes, they are sent upstream. I have not found any issue tracker nor mailing list for tayga. I have asked upstream for this as well, but not received any reply.
Thanks.
2.) Why do you turn off PIE on ppc64?
# PIE hardening seems to fail on ppc64
If this is really the case, please add a more descriptive comment (error output).
After a bit of debugging, I found this not to be a problem with PIE, but a difference on Red Hat's ppc64 and x86_64 koji builders. The ppc64 one does not take "-z now" in LDFLAGS, though the x86_64 one does. Changing to "-znow" seems to work, so I have removed the workaround.
Good job on this one, thanks.
3.) You're missing the %defattr tag
It's not needed for current RPM, but seems like you're targetting old RHEL versions as well?
Fixed.
4.) You're installing a service with name of a templated service:
ln -s %{_unitdir}/%{name}@.service %{buildroot}%{_unitdir}/%{name}@example.service
This makes no sense. systemd would probably just ignore that.
No, it treats it as a service.
'systemctl enable tayga@example' would make the link in the proper place; just drop that line.
Okay, dropped, but it would not be trivial for the non-seasoned systemd user to know how to enable the service.
Yes, but how does this help a non-seasoned user?
Ideally upstream would should distribute the service files along with the documentation on how to use them.
Should I add a README.RedHat with a note explaining this?
Yes, I think that's okay.
Also, Will the %systemd_preun %{name}@.service remove both the unit file and any generated symlinks?
Of course not. The packages shouldn't touch /etc. If the user installs file in /etc, he's responsible for dealing with it. That's the same regardless of whether the service is templated or not.
5.) rpmlint complains:
tayga.x86_64: E: incorrect-fsf-address /usr/share/licenses/tayga/COPYING The Free Software Foundation address in this file seems to be outdated or misspelled. Ask upstream to update the address, or if this is a license file, possibly the entire file with a new copy available from the FSF.
This is something the upstream would need to fix; you may want to let them know. Obviously not a review blocker.
I did notice upstream about this as well.
Thank you.
The package looks good to me now.
APPROVED
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #18 from Ingvar Hagelund ingvar@linpro.no --- (In reply to Lubomir Rintel from comment #17)
(In reply to Ingvar Hagelund from comment #14) Ideally upstream would should distribute the service files along with the documentation on how to use them.
Should I add a README.RedHat with a note explaining this?
Yes, I think that's okay.
I included this the redhat initscript patch. I sent a note upstream on this as well.
The package looks good to me now. APPROVED
Thank you very much for the review. I have requested new packages in the pkdb:
user: ingvar request package: tayga on branch master user: ingvar request package: tayga on branch f23 user: ingvar request package: tayga on branch f22 user: ingvar request package: tayga on branch epel7 user: ingvar request package: tayga on branch el6 user: ingvar request package: tayga on branch el5
Ingvar
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #19 from Jon Ciesla limburgher@gmail.com --- Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/tayga
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #20 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.el5 has been submitted as an update to Fedora EPEL 5. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-fa11e5ba72
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #20 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.el5 has been submitted as an update to Fedora EPEL 5. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-fa11e5ba72
--- Comment #21 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-6782b896fa
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #22 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-75f83adc15
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #22 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-75f83adc15
--- Comment #23 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.el6 has been submitted as an update to Fedora EPEL 6. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-75f06b53c0
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #24 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2016-cf4194ca93
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
Ingvar Hagelund ingvar@linpro.no changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |NEXTRELEASE Last Closed| |2016-01-15 10:22:03
--- Comment #25 from Ingvar Hagelund ingvar@linpro.no --- Package built and requested for testing on all branches. Closing this as NEXTRELEASE.
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|CLOSED |ON_QA Resolution|NEXTRELEASE |--- Keywords| |Reopened
--- Comment #26 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.el5 has been pushed to the Fedora EPEL 5 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-fa11e5ba72
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #27 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-6782b896fa
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #28 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-cf4194ca93
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #29 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.el6 has been pushed to the Fedora EPEL 6 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-75f06b53c0
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #30 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-75f83adc15
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #31 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #32 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #33 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #34 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1263941
--- Comment #35 from Fedora Update System updates@fedoraproject.org --- tayga-0.9.2-3.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.
package-review@lists.fedoraproject.org