Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1438853 - Review Request: lldpd - an ISC-licensed implementation of LLDP
Summary: Review Request: lldpd - an ISC-licensed implementation of LLDP
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dan Horák
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-04-04 14:46 UTC by James Hogarth
Modified: 2017-05-11 20:56 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-05-11 20:56:01 UTC
Type: ---
dan: fedora-review+


Attachments (Terms of Use)

Description James Hogarth 2017-04-04 14:46:44 UTC
Spec URL: https://fedorapeople.org/~jhogarth/lldpd/lldpd.spec
SRPM URL: https://fedorapeople.org/~jhogarth/lldpd/lldpd-0.9.7-1.fc25.src.rpm

Description:
LLDP is an industry standard protocol designed to supplant proprietary
Link-Layer protocols such as EDP or CDP. The goal of LLDP is to provide
an inter-vendor compatible mechanism to deliver Link-Layer notifications
to adjacent network devices.

Fedora Account System Username: jhogarth

NOTES: If you don't have a switch with LLDP capabilities to test the package with then installing it on a VM host and installing it in a VM guest will work for them to see the lldp packets of each other and display the details on a default KVM/boxes/virt-manager install.

Comment 2 Ralf Corsepius 2017-04-04 15:49:49 UTC
Not a formal review just some remarks:

- Building package is not verbose. 
Please append --disable-silent-rules to %configure.

- The package contains a tarball named "0.9.7.tar.gz"
I would propose to change Source0 into:
Source0:        https://github.com/%{gh_owner}/%{name}/archive/%{version}.tar.gz#/%{name}-%{version}.tar.gz

This lets downloading from github create a tarball named "lldpd-0.9.7.tar.gz"

Comment 3 Dan Horák 2017-04-05 12:41:09 UTC
formal review is here, see the notes explaining OK* and BAD statuses below:

OK	source files match upstream:
	    f3ed143ba16fb4232a237f51cddaee276af896c8  0.9.7.tar.gz
OK	package meets naming and versioning guidelines.
OK	specfile is properly named, is cleanly written and uses macros consistently.
OK	dist tag is present.
OK	license field matches the actual license.
OK	license is open source-compatible (ISC). License text included in package.
OK	latest version is being packaged.
OK*	BuildRequires are proper.
OK	compiler flags are appropriate.
OK	package builds in mock (Rawhide/x86_64).
OK	debuginfo package looks complete.
BAD	rpmlint is silent.
OK	final provides and requires look sane.
N/A	%check is present and all tests pass.
OK	shared libraries are added to the regular linker search paths with ldconfig call.
OK	owns the directories it creates.
OK	doesn't own any directories it shouldn't.
OK	no duplicates in %files.
OK	file permissions are appropriate.
OK	correct scriptlets present.
OK	code, not content.
OK	documentation is small, so no -docs subpackage is necessary.
OK	%docs are not necessary for the proper functioning of the package.
OK	headers in devel subpackage
OK	pkgconfig files in devel subpackage
OK	no libtool .la droppings.
OK	not a GUI app.

- please do the 2 changes Ralf proposed, use %{name}-%{version}.tar.gz style for the source and use a verbose build (easier to read the build log than inspect *.o files for used compiler flags)
- doxygen is present in BR, but there are no docs generated
- rpmlint complains a bit
lldpd.x86_64: W: summary-not-capitalized C lldpd is an ISC-licensed implementation of LLDP
lldpd.x86_64: W: name-repeated-in-summary C lldpd
lldpd.src: W: summary-not-capitalized C lldpd is an ISC-licensed implementation of LLDP
lldpd.src: W: name-repeated-in-summary C lldpd
lldpd-devel.x86_64: W: summary-not-capitalized C lldpd is an ISC-licensed implementation of LLDP
  -> Summary: ISC-licensed implementation of LLDP

lldpd.x86_64: W: non-standard-uid /var/lib/lldpd lldpd
lldpd.x86_64: W: non-standard-gid /var/lib/lldpd lldpd
lldpd-devel.x86_64: W: no-documentation
lldpd-devel.x86_64: W: only-non-binary-in-usr-lib
lldpd.x86_64: W: hidden-file-or-dir /usr/lib/.build-id
lldpd.x86_64: W: hidden-file-or-dir /usr/lib/.build-id
lldpd.x86_64: W: conffile-without-noreplace-flag /etc/lldpd.d/README.conf
  -> OK, can be ignored

lldpd.x86_64: E: zero-length /usr/share/doc/lldpd/ChangeLog
  -> drop the file from %doc, needs git tree and NEWS is sufficient, Contribute.md can dropped too

lldpd.src:9: W: mixed-use-of-spaces-and-tabs (spaces: line 8, tab: line 9)
lldpd.x86_64: E: missing-call-to-chdir-with-chroot /usr/sbin/lldpd
lldpd.x86_64: W: shared-lib-calls-exit /usr/lib64/liblldpctl.so.4.8.0 exit@GLIBC_2.2.5
lldpd.x86_64: W: non-ghost-in-run /run/lldpd/chroot
lldpd.x86_64: W: non-ghost-in-run /run/lldpd
lldpd-devel.x86_64: W: no-dependency-on lldpd/lldpd-libs/liblldpd
  -> need fix or comment

Comment 4 James Hogarth 2017-04-05 13:37:48 UTC
Thanks for the feedback, both of you.

  * Source0 line updated
  * Summary changed to suggestion
  * configure option updated
  * requires added to subpackage on main package
  * looking at more detail it wanted not just doxygen but the pdf part as well to produce pdf docs... that feels silly so I've pulled the BR

Koji builds:

F25: https://koji.fedoraproject.org/koji/taskinfo?taskID=18796508
F26: https://koji.fedoraproject.org/koji/taskinfo?taskID=18796713
RAW: https://koji.fedoraproject.org/koji/taskinfo?taskID=18796875
EL7: https://koji.fedoraproject.org/koji/taskinfo?taskID=18797292

Spec URL: https://fedorapeople.org/~jhogarth/lldpd/lldpd.spec
SRPM URL: https://fedorapeople.org/~jhogarth/lldpd/lldpd-0.9.7-2.fc27.src.rpm

Comment 5 Dan Horák 2017-04-05 14:10:48 UTC
Looks good now, package is APPROVED.

Comment 6 James Hogarth 2017-04-05 15:11:43 UTC
Thanks Dan ... and good news is it only required a minor maintainer patch to get it built against EPEL6 as well :)

Letting the developer know the changes I made in the patch to see if he's willing to upstream.

Comment 7 Gwyn Ciesla 2017-04-05 15:27:34 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/lldpd

Comment 8 Fedora Update System 2017-04-05 23:51:30 UTC
lldpd-0.9.7-4.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-676f5655d9

Comment 9 Fedora Update System 2017-04-05 23:51:40 UTC
lldpd-0.9.7-4.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-cedacd0aaf

Comment 10 Fedora Update System 2017-04-05 23:51:46 UTC
lldpd-0.9.7-4.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-8741526fc3

Comment 11 Fedora Update System 2017-04-06 00:42:51 UTC
lldpd-0.9.7-5.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-77fac90af3

Comment 12 Fedora Update System 2017-04-06 00:42:57 UTC
lldpd-0.9.7-5.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-8363c4e8ec

Comment 13 Fedora Update System 2017-04-06 00:43:03 UTC
lldpd-0.9.7-5.el6 has been submitted as an update to Fedora EPEL 6. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-6ab500b6b5

Comment 14 Fedora Update System 2017-04-06 00:43:09 UTC
lldpd-0.9.7-5.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-7356de7a31

Comment 15 Fedora Update System 2017-04-06 00:43:14 UTC
lldpd-0.9.7-5.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-fe76c20831

Comment 16 Fedora Update System 2017-04-06 19:19:18 UTC
lldpd-0.9.7-5.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-2017-6ab500b6b5

Comment 17 Fedora Update System 2017-04-06 19:20:50 UTC
lldpd-0.9.7-5.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-2017-fe76c20831

Comment 18 Fedora Update System 2017-04-06 19:53:45 UTC
lldpd-0.9.7-5.fc24 has been pushed to the Fedora 24 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-2017-8363c4e8ec

Comment 19 Fedora Update System 2017-04-06 20:24:25 UTC
lldpd-0.9.7-5.fc25 has been pushed to the Fedora 25 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-2017-77fac90af3

Comment 20 Fedora Update System 2017-04-06 22:22:53 UTC
lldpd-0.9.7-5.fc26 has been pushed to the Fedora 26 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-2017-7356de7a31

Comment 21 Fedora Update System 2017-04-19 17:00:14 UTC
lldpd-0.9.7-5.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2017-04-19 22:18:16 UTC
lldpd-0.9.7-5.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.

Comment 23 Fedora Update System 2017-04-19 23:20:03 UTC
lldpd-0.9.7-5.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.

Comment 24 Fedora Update System 2017-04-22 05:17:54 UTC
lldpd-0.9.7-5.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.

Comment 25 Fedora Update System 2017-04-22 05:18:35 UTC
lldpd-0.9.7-5.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.