Bug 165985
| Summary: | Review Request: Aeryn - A C++ testing framework | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Paul F. Johnson <paul> | ||||
| Component: | Package Review | Assignee: | John Mahowald <jpmahowald> | ||||
| Status: | CLOSED NOTABUG | QA Contact: | David Lawrence <dkl> | ||||
| Severity: | medium | Docs Contact: | |||||
| Priority: | medium | ||||||
| Version: | rawhide | CC: | fedora-package-review | ||||
| Target Milestone: | --- | ||||||
| Target Release: | --- | ||||||
| Hardware: | All | ||||||
| OS: | Linux | ||||||
| URL: | http://www.paulgrenyer.dyndns.org/aeryn/ | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | Environment: | ||||||
| Last Closed: | 2006-10-12 16:18:01 UTC | Type: | --- | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Embargoed: | |||||||
| Bug Depends On: | |||||||
| Bug Blocks: | 201449 | ||||||
| Attachments: |
|
||||||
|
Description
Paul F. Johnson
2005-08-15 14:40:01 UTC
specfile/SRPM URL missing. http://www.all-the-johnsons.co.uk/aeryn/aeryn2.spec for the spec, the source archive is there also. I'm having a problem with the install though (it's the first time I've done this!) The spec file has now been updated and the RPM builds cleanly. Can you please review it for inclusion? http://www.all-the-johnsons.co.uk/aeryn/aeryn2.spec The RPMs are also there http://www.all-the-johnsons.co.uk/aeryn2/aeryn2-2-1.1.i386.rpm http://www.all-the-johnsons.co.uk/aeryn2/aeryn2-debuginfo-2-1.1.i386.rpm src.rpm also available from http://www.all-the-johnsons.co.uk/aeryn2/aeryn2-2-1.1.src.rpm Okay, let's start somewhere. A first rather brief look: Package builds as "aeryn2-2-1.1.i386.rpm" although the upstream version is "2.1.1", so why not name the package "aeryn" with version "2.1.1" and release "1"? > $ rpmls aeryn2 | grep ^d > drwxr-xr-x /usr/include/aeryn/aeryn > drwxr-xr-x /usr/include/aeryn/aeryn/details This reveals that the directory /usr/include/aeryn/ is not included in the package. A look at the complete package listing reveals that /usr/lib/aeryn/ is not included either. Further, /usr/bin/TestClient is a rather generic name. It doesn't look like a useful binary that should be included, as it only runs a test suite which most likely ought to be ran during rpmbuild in %check or so instead. It looks like it returns a proper exit code for "failed/successful" state. > $ rpm -qd aeryn2 > $ No documentation included at all? [...] Now to the spec file (I usually start a review at the spec level, but moved it here): > Summary: Aeryn2 is a C++ unit test package Better IMO => Summary: C++ unit testing framework > Name: aeryn2 > Version: 2 > Release: 1.1%{?dist} As mentioned above, I believe this should be "Name: aeryn", "Version: 2.1.1" and "Release: 1%{?dist}". > Source0: http://www.all-the-johnsons.co.uk/aeryn/aeryn2.tar.bz2 This was a surprise, since the Source0 URL should point to the upstream projects download location. It seems you repackaged upstream author's ZIP archive and included the PDF documentation file in the tarball. Why not just take his ZIP as "Source0" and put the PDF into "Source1"? Sort of: %prep %setup -T -c -n %{name} unzip %{SOURCE0} and everything you did to build your tar.bz2 should be done after extracting the zip. > $ rpmlint aeryn2-2-1.1.src.rpm > E: aeryn2 description-line-too-long Keep %description lines below 80 characters. > %build > make %{?_smp_mflags} Code compiles without $RPM_OPT_FLAGS, e.g. > g++ -W -Wall -Werror -pedantic -Wcast-qual -Wcast-align -Wwrite-strings > -Winline -finline-limit=1048576 -g3 -c -o noncopyable.o > ../src/noncopyable.cpp -I../include -DNO_OUTPUT_OPERATOR_DETECTION It may be necessary to patch ./make/common.mk and adjust CXXFLAGS there. > %clean > [ "x${RPM_BUILD_ROOT}" != "x/" ] && rm -rf "${RPM_BUILD_ROOT}" Just "rm -rf $RPM_BUILD_ROOT" is enough and more readable. You do define a default buildroot. Rare users, who rpmbuild as superuser and redefine the buildroot play with fire anyway. > %files > %defattr(-, root, root) > %{_bindir}/* > %{_includedir}/aeryn/* > %{_libdir}/aeryn/* Instead, these two %{_includedir}/aeryn/ %{_libdir}/aeryn/ would also include the two directories and not just their contents. The package is actually called Aeryn2 with a version number of 2.1.1 hense the name! Documentation is a couple of PDFs - I can convert them to something sane, so that's not an issue and I'll change the TestClient executable to be performed as %check. I'll also make the changes so that it's not /usr/include/aeryn/aeryn but /usr/include/aeryn. No problems with the others. Thanks for the pointers and I'll get a newer version up at some point today. Other things are pressing though :-( > The package is actually called Aeryn2 with a version number
> of 2.1.1 hense the name!
Okay, then let me prove the opposite:
$ ls aeryn2-2-1.1.src.rpm
aeryn2-2-1.1.src.rpm
$ grep 'Ver\|Nam\|Rel' aeryn2.spec
Name: aeryn2
Version: 2
Release: 1.1%{?dist}
Created attachment 122353 [details] spec patch Makes most of the recommended changes in comment 5. %changelog * Fri Dec 16 2005 John Mahowald <jpmahowald> - 2.1.1-2 - Rename to aeryn - Add devel package - Copy files to build root - ldconfig - Use upstream sources Any progress on this? If not, will close in a week as per http://fedoraproject.org/wiki/Extras/Policy/StalledReviews Closing. Reopen to resume work on this. |