Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: beakerlib - shell-level integration testing library
https://bugzilla.redhat.com/show_bug.cgi?id=561470
Summary: Review Request: beakerlib - shell-level integration testing library Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: pmuller@redhat.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora Target Release: ---
Spec URL: http://www.afri.cz/files/beakerlib.spec SRPM URL: http://www.afri.cz/files/beakerlib-1.0-1.src.rpm Description: The BeakerLib project means to provide a library of various helpers, which could be used when writing operating system level integration tests.
Note: This is my first package in Fedora, so I'm probably gonna need a sponsor.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
Petr Muller pmuller@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jlaska@redhat.com, | |psplicha@redhat.com Blocks| |177841(FE-NEEDSPONSOR)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #1 from Petr Muller pmuller@redhat.com 2010-02-03 13:04:44 EST --- More info: https://fedorahosted.org/beakerlib/ (we are also "upstream")
rpmlint:
$ rpmlint /home/afri/qa/beakerlib/rpm-build/noarch/beakerlib-1.0-1.noarch.rpm beakerlib.noarch: W: self-obsoletion beaker-lib obsoletes beaker-lib beakerlib.noarch: W: self-obsoletion rhtslib obsoletes rhtslib
These are the "old" names for the same stuff. Both names are in both "Provides" and "Obsoletes" field, which I believed was the right way.
beakerlib.noarch: W: only-non-binary-in-usr-lib We believe that because all of the scripts are meant to be just sourced and used as a library, /usr/lib is the right place for them.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #2 from James Laska jlaska@redhat.com 2010-02-05 13:29:21 EST --- I can do the review, but I'm not able to sponsor you.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
James Laska jlaska@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |jlaska@redhat.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #3 from Jason Tibbitts tibbs@math.uh.edu 2010-02-05 14:23:06 EST --- The review must be done by the sponsor, at least according to Fedora policy.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
James Laska jlaska@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|jlaska@redhat.com |nobody@fedoraproject.org
--- Comment #4 from James Laska jlaska@redhat.com 2010-02-05 14:33:56 EST --- Jason, thanks ... it wasn't obvious to me from https://fedoraproject.org/wiki/Package_Review_Process that a sponsor could only do the review. I knew I wouldn't be able to complete the review, but I'd like to provide my review feedback anyway.
According to https://fedoraproject.org/wiki/Packaging:ReviewGuidelines
= General =
Can the .spec summary be made more specific? Presently, it says "An operating system integration testing harness". Is that the correct summary for beakerlib?
= MUST =
* MUST: rpmlint must be run on every package. The output should be posted in the review.
FAIL - I've attached a small spec patch to address several issues, but you will need to adjust further to address the issues identified below.
# rpmlint beakerlib-1.0-1.src.rpm beakerlib.src:11: W: unversioned-explicit-obsoletes rhtslib beakerlib.src:11: W: unversioned-explicit-obsoletes beaker-lib beakerlib.src:12: W: unversioned-explicit-provides rhtslib beakerlib.src:12: W: unversioned-explicit-provides beaker-lib
Unsure ... may need to get a second opinion on this.
beakerlib.src:30: E: files-attr-not-set
Corrected in attached patch
beakerlib.src:31: E: hardcoded-library-path in /usr/lib/beakerlib/*.sh
I don't think bash source files should live in /usr/lib/beakerlib. According to the FHS (http://www.pathname.com/fhs/pub/fhs-2.3.html#USRLIBLIBRARIESFORPROGRAMMINGAN...) -
"/usr/lib includes object files, libraries, and internal binaries that are not intended to be executed directly by users or shell scripts. [22]
Applications may use a single subdirectory under /usr/lib. If an application uses a subdirectory, all architecture-dependent data exclusively used by the application must be placed within that subdirectory."
beakerlib does not contain architecture dependent files and does not include object files, libraries or internal binaries.
I'd recommend moving bash content into a system-wide location such as /etc/profile.d/
beakerlib.src:31: E: files-attr-not-set beakerlib.src:32: E: files-attr-not-set beakerlib.src:33: E: hardcoded-library-path in /usr/lib/beakerlib/virtualX.sh
Same as above
beakerlib.src:33: E: files-attr-not-set beakerlib.src:34: E: hardcoded-library-path in /usr/lib/beakerlib/python/rlMemAvg.py* beakerlib.src:34: E: files-attr-not-set beakerlib.src:35: E: hardcoded-library-path in /usr/lib/beakerlib/python/rlMemPeak.py* beakerlib.src:35: E: files-attr-not-set beakerlib.src:36: E: hardcoded-library-path in /usr/lib/beakerlib/python/journalling.py* beakerlib.src:36: E: files-attr-not-set beakerlib.src:37: E: hardcoded-library-path in /usr/lib/beakerlib/python/journal-compare.py* beakerlib.src:37: E: files-attr-not-set
Python content should be moved into %{python_sitelib}. See https://fedoraproject.org/wiki/Packaging:Python
beakerlib.src:38: E: hardcoded-library-path in /usr/lib/beakerlib/perl/deja-summarize
Same, can this content be moved into the system perl directory?
beakerlib.src:38: E: files-attr-not-set beakerlib.src:39: E: files-attr-not-set beakerlib.src: W: no-cleaning-of-buildroot %install
You have such a call in the .spec, but it is wrapped around a conditional check to ensure BUILD_ROOT != '/'. I've updated the .spec in in the attached patch to address this
beakerlib.src: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 3)
Mixed use of tabs and spaces, I've converted to using spaces in the attached patch.
1 packages and 0 specfiles checked; 17 errors, 6 warnings.
# rpmlint beakerlib-1.0-1.noarch.rpm beakerlib.noarch: W: self-obsoletion beaker-lib obsoletes beaker-lib beakerlib.noarch: W: self-obsoletion rhtslib obsoletes rhtslib beakerlib.noarch: W: only-non-binary-in-usr-lib
I believe this will be addressed by moving bash, python and perl content into standard system-wide locations.
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
# rpmlint beakerlib.spec beakerlib.spec:11: W: unversioned-explicit-obsoletes rhtslib beakerlib.spec:11: W: unversioned-explicit-obsoletes beaker-lib beakerlib.spec:12: W: unversioned-explicit-provides rhtslib beakerlib.spec:12: W: unversioned-explicit-provides beaker-lib beakerlib.spec:33: E: hardcoded-library-path in /usr/lib/beakerlib/*.sh beakerlib.spec:35: E: hardcoded-library-path in /usr/lib/beakerlib/virtualX.sh beakerlib.spec:36: E: hardcoded-library-path in /usr/lib/beakerlib/python/rlMemAvg.py* beakerlib.spec:37: E: hardcoded-library-path in /usr/lib/beakerlib/python/rlMemPeak.py* beakerlib.spec:38: E: hardcoded-library-path in /usr/lib/beakerlib/python/journalling.py* beakerlib.spec:39: E: hardcoded-library-path in /usr/lib/beakerlib/python/journal-compare.py* beakerlib.spec:40: E: hardcoded-library-path in /usr/lib/beakerlib/perl/deja-summarize
Addressed earlier
beakerlib.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 3)
Addressed earlier
0 packages and 1 specfiles checked; 7 errors, 5 warnings.
* MUST: The package must be named according to the [[Packaging/NamingGuidelines| Package Naming Guidelines]] .
PASS
* MUST: The spec file name must match the base package <code>%{name}</code>, in the format <code>%{name}.spec</code> unless your package has an exemption. .
PASS
* MUST: The package must meet the [[Packaging/Guidelines| Packaging Guidelines]] .
PASS
* MUST: The package must be licensed with a Fedora approved license and meet the [[Packaging/LicensingGuidelines| Licensing Guidelines]] .
PASS
* MUST: The License field in the package spec file must match the actual license.
FAIL - * I can't tell by looking at the code what the license is. You may wish to include a LICENSE file. * The Makefile lists '# License: GPL v2 or later', but the package is listed as GPLv2. if this is the case, you may wish to change the .spec file License: GPLv2+ * src/staf-rhts/BEAKERLIB.pm shows "Eclipse Public License (EPL) V1.0" which is not compatible with the GPLv2 (see http://fedoraproject.org/wiki/Licensing#SoftwareLicenses).
* MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in <code>%doc</code>.
No LICENSE file included, so not an issue.
* MUST: The spec file must be written in American English.
PASS
* MUST: The spec file for the package MUST be legible.
PASS
* MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the [[Packaging/SourceURL| Source URL Guidelines]] for how to deal with this.
FAIL - Please correct according to https://fedoraproject.org/wiki/Packaging/SourceURL
* MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
WARN - while the package builds successfully, it won't build properly once the %files are adjusted to suitable system-wide locations. The build process will need to be adjusted.
* MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in <code>ExcludeArch</code>. Each architecture listed in <code>ExcludeArch</code> MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding <code>ExcludeArch</code> line.
* MUST: All build dependencies must be listed in <code>BuildRequires</code>, except for any that are listed in the [[Packaging/Guidelines#Exceptions_2|exceptions section of the Packaging Guidelines]] ; inclusion of those as <code>BuildRequires</code> is optional. Apply common sense.
* MUST: The spec file MUST handle locales properly. This is done by using the <code>%find_lang</code> macro. Using <code>%{_datadir}/locale/*</code> is strictly forbidden.
N/A
* MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in <code>%post</code> and <code>%postun</code>.
N/A
* MUST: Packages must NOT bundle copies of system libraries.<ref name="no_bundle">[[Packaging:Guidelines#Duplication_of_system_libraries|Packaging Guidelines: Duplication of System Libraries]]</ref>
PASS
* MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker.
N/A
* MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory.
FAIL - please see attached patch
* MUST: A Fedora package must not list a file more than once in the spec file's %files listings.
FAIL - please see attached patch
* MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every <code>%files</code> section must include a <code>%defattr(...)</code> line.
FAIL - please see attached patch
* MUST: Each package must have a %clean section, which contains <code>rm -rf %{buildroot}</code> ([[Packaging/Guidelines#UsingBuildRootOptFlags|or $RPM_BUILD_ROOT]]).
WARN - noted earlier in rpmlint output
* MUST: Each package must consistently use macros.
PASS
* MUST: The package must contain code, or permissable content.
PASS
* MUST: Large documentation files must go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity). <ref name="docs">[[Packaging/Guidelines#PackageDocumentation|Packaging Guidelines: Package Documentation]]</ref>
N/A
* MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. <ref name="docs">[[Packaging/Guidelines#PackageDocumentation|Packaging Guidelines: Package Documentation]]</ref>
N/A
* MUST: Header files must be in a -devel package. <ref name="devel">[[Packaging/Guidelines#DevelPackages|Packaging Guidelines: Devel Packages]]</ref>
N/A
* MUST: Static libraries must be in a -static package. <ref name="static">[[Packaging/Guidelines#StaticLibraries|Packaging Guidelines: Packaging Static Libraries]]</ref>
N/A
* MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). <ref name="pkgconfig">[[Packaging/Guidelines#PkgconfigFiles|Packaging Guidelines: Pkgconfig Files]]</ref>
N/A
* MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. <ref name="devel">[[Packaging/Guidelines#DevelPackages|Packaging Guidelines: Devel Packages]]</ref>
N/A
* MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: <code>Requires: %{name} = %{version}-%{release} </code> <ref name="requirebase">[[Packaging/Guidelines#RequiringBasePackage|Packaging Guidelines: Requiring Base Package]]</ref>
N/A
* MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built.<ref name="static">[[Packaging/Guidelines#StaticLibraries|Packaging Guidelines: Packaging Static Libraries]]</ref>
N/A
* MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation.
N/A
* MUST: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the <code>filesystem</code> or <code>man</code> package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time.
N/A
* MUST: At the beginning of <code>%install</code>, each package MUST run <code>rm -rf %{buildroot}</code> ([[Packaging/Guidelines#UsingBuildRootOptFlags|or $RPM_BUILD_ROOT]]).
FAIL - please see attached patch
* MUST: All filenames in rpm packages must be valid UTF-8.
PASS
= SHOULD =
* SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
Pmuller is upstream in this case, I've recommended including a LICENSE file
* SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
N/A
* SHOULD: The reviewer should test that the package builds in mock.
PASS - tested using a koji scratch build. But additional spec changes will be needed http://koji.fedoraproject.org/koji/taskinfo?taskID=1965521
* SHOULD: The package should compile and build into binary rpms on all supported architectures.
PASS
* SHOULD: The reviewer should test that the package functions as described. A package should not segfault instead of running, for example.
WARN - will wait for packaging to settle down first
* SHOULD: If scriptlets are used, those scriptlets must be sane. This is vague, and left up to the reviewers judgement to determine sanity.
N/A
* SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. <ref name="requirebase">[[Packaging/Guidelines#RequiringBasePackage|Packaging Guidelines: Requiring Base Package]]</ref>
N/A
* SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg. A reasonable exception is that the main pkg itself is a devel tool not installed in a user runtime, e.g. gcc or gdb. <ref name="pkgconfig">[[Packaging/Guidelines#PkgconfigFiles|Packaging Guidelines: Pkgconfig Files]]</ref>
N/A
* SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.
N/A
* SHOULD: your package should contain man pages for binaries/scripts. If it doesn't, work with upstream to add them where they make sense.<ref name="manpages">[[Packaging/Guidelines#Man_pages|Packaging Guidelines: Man Pages]]</ref>
PASS (kudos!)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #5 from James Laska jlaska@redhat.com 2010-02-05 14:35:40 EST --- (In reply to comment #4)
Jason, thanks ... it wasn't obvious to me from https://fedoraproject.org/wiki/Package_Review_Process that a sponsor could only do the review.
I see now "The Reviewer can be any Fedora account holder, who is a member of the packager group. There is one exception: If it is the first package of a Contributor, the Reviewer must be a Sponsor.".
Hopefully my review feedback is helpful.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #6 from James Laska jlaska@redhat.com 2010-02-05 14:37:47 EST --- Created an attachment (id=389172) --> (https://bugzilla.redhat.com/attachment.cgi?id=389172) spec file patch
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #7 from Petr Å plÃchal psplicha@redhat.com 2010-02-08 04:44:55 EST ---
Can the .spec summary be made more specific? Presently, it says "An operating system integration testing harness". Is that the correct summary for beakerlib?
What about using the summary from the wiki? Should be quite up-to-date and more specific:
Summary:
A shell-level integration testing library
Description:
BeakerLib is a shell-level integration testing library, providing convenience functions which simplify writing, running and analysis of integration and blackbox tests.
The essential features are:
* Journal - uniform logging mechanism (saved in XML format) * Phases - clear separation of setup / test / cleanup * Asserts - common checks (exit codes, check file existence & content) * Common operations - managing services, backup & restore
- MUST: The License field in the package spec file must match the actual
license.
FAIL -
- I can't tell by looking at the code what the license is. You may wish to
include a LICENSE file.
I'll add the LICENSE file as part of the documentation update patch.
- The Makefile lists '# License: GPL v2 or later', but the package is listed
as GPLv2. if this is the case, you may wish to change the .spec file License: GPLv2+
- src/staf-rhts/BEAKERLIB.pm shows "Eclipse Public License (EPL) V1.0" which
is not compatible with the GPLv2 (see http://fedoraproject.org/wiki/Licensing#SoftwareLicenses).
The staf-rhts directory should be removed altogether I guess. This is Beaker-Staff integration stuff which has not been used AFAIK.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #8 from Michael Schwendt mschwendt@gmail.com 2010-02-08 07:55:40 EST ---
BuildArch: noarch %{_libdir}/%{name}/python/rlMemAvg.py*
Bzzz! Noarch plus %_libdir won't fly, since the value of %_libdir would change with the build host.
Source0: %{name}-%{version}.tar.gz
This is no valid URL where to download the tarball. If the tarball cannot be downloaded anywhere, please add a comment on how to construct it from a scm repository.
That redirects to:
https://fedorahosted.org/web/410
No such project.
The requested project does not exist on Fedora Hosted.
Obsoletes: rhtslib beaker-lib Provides: rhtslib beaker-lib
Have you tried "rpmlint -i ..." on the rpms?
It's common practise to specify a max. version in Obsoletes tags in order to not occupy a namespace completely. And to specify an explicit EVR in Provides. Example:
Obsoletes: rhtslib < 1.0 Provides: rhtslib = %{version}-%{release}
If not doing that, this pair of Obsoletes/Provides becomes much more questionable. Look! On Fedora 12:
$ repoquery --whatprovides rhtslib $ repoquery --whatprovides beaker-lib $
Nothing provides these so far. Even if it may be "old names": IMO, it's being frowned upon to provide alternative virtual package names just for fun. My suggestion: keep the Obsoletes with proper max. version, but don't add the Provides. It's silly to run with multiple alternative/competing packages names, whether virtual or not.
%makeinstall DESTDIR=$RPM_BUILD_ROOT
Please prefer "make DESTDIR=$RPM_BUILD_ROOT install" over %makeinstall unless the normal "make install ..." doesn't work. The macro redefines many values, which bears a risk.
/usr/lib/beakerlib/python/journal-compare.py*
Which packages owns /usr/lib/beakerlib/python/?
/usr/lib/beakerlib/perl/deja-summarize
Which packages owns /usr/lib/beakerlib/perl/?
http://fedoraproject.org/wiki/Packaging:UnownedDirectories
%{_mandir}/man1/beakerlib*1.gz
Since man pages typically are compressed transparently by rpmbuild, prefer a wildcard over a file extension such as .gz:
%{_mandir}/man1/beakerlib*1*
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #9 from Petr Muller pmuller@redhat.com 2010-02-09 11:27:06 EST --- Thanks you for all the feedback! I'll fix everything and let know...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #10 from Petr Muller pmuller@redhat.com 2010-02-12 11:12:43 EST --- Spec URL: http://www.afri.cz/files/beakerlib.spec SRPM URL: http://www.afri.cz/files/beakerlib-1.0-2.src.rpm
(In reply to comment #8)
Bzzz! Noarch plus %_libdir won't fly, since the value of %_libdir would change with the build host.
No libdir now. All scripts meant for sourcing are now in /usr/share/beakerlib/, all files meant for executing are in /usr/bin and prefixed 'beakerlib-'.
Source0: %{name}-%{version}.tar.gz
This is no valid URL where to download the tarball. If the tarball cannot be downloaded anywhere, please add a comment on how to construct it from a scm repository.
I placed the tarball at the website and provided a link to it. It's a link to Trac attachment.
That redirects to: https://fedorahosted.org/web/410 No such project. The requested project does not exist on Fedora Hosted.
Sorry, the right URL is http://git.fedorahosted.org/git/beakerlib.git. Anyways, Source now point directly to the tarball attachment, not osurce repo.
Obsoletes: rhtslib beaker-lib Provides: rhtslib beaker-lib
Nothing provides these so far. Even if it may be "old names": IMO, it's being frowned upon to provide alternative virtual package names just for fun. My suggestion: keep the Obsoletes with proper max. version, but don't add the Provides. It's silly to run with multiple alternative/competing packages names, whether virtual or not.
We've decided to remove these entirely, as the names are not present in Fedora.
%makeinstall DESTDIR=$RPM_BUILD_ROOT
Please prefer "make DESTDIR=$RPM_BUILD_ROOT install" over %makeinstall unless the normal "make install ..." doesn't work. The macro redefines many values, which bears a risk.
Fixed
/usr/lib/beakerlib/python/journal-compare.py*
Which packages owns /usr/lib/beakerlib/python/?
/usr/lib/beakerlib/perl/deja-summarize
Which packages owns /usr/lib/beakerlib/perl/?
Not applicable now. Package now needs /usr/share/beakerlib and /usr/share/doc/beakerlib, both are owned by it.
%{_mandir}/man1/beakerlib*1.gz
Since man pages typically are compressed transparently by rpmbuild, prefer a wildcard over a file extension such as .gz: %{_mandir}/man1/beakerlib*1*
Fixed
rpmlint is now silent on the package
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #11 from Petr Muller pmuller@redhat.com 2010-02-12 11:27:03 EST --- (In reply to comment #4)
Can the .spec summary be made more specific? Presently, it says "An operating system integration testing harness". Is that the correct summary for beakerlib?
I've put there what Petr Splichal proposed in comment 5
- MUST: rpmlint must be run on every package. The output should be posted in
the review.
FAIL - I've attached a small spec patch to address several issues, but you will need to adjust further to address the issues identified below.
rpmlint is now silent on the package
FAIL -
- I can't tell by looking at the code what the license is. You may wish to
include a LICENSE file.
- The Makefile lists '# License: GPL v2 or later', but the package is listed
as GPLv2. if this is the case, you may wish to change the .spec file License: GPLv2+
- src/staf-rhts/BEAKERLIB.pm shows "Eclipse Public License (EPL) V1.0" which
is not compatible with the GPLv2 (see http://fedoraproject.org/wiki/Licensing#SoftwareLicenses).
staf-rhts was removed. The correct license is GPLv2 only and all files should have standardized license infirmation.
- MUST: If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s) for the package must be included in <code>%doc</code>.
No LICENSE file included, so not an issue.
LICENSE now included and tagged by %doc
FAIL - Please correct according to https://fedoraproject.org/wiki/Packaging/SourceURL
Fixed, see previous comments
- MUST: The package MUST successfully compile and build into binary rpms on at
least one primary architecture.
WARN - while the package builds successfully, it won't build properly once the %files are adjusted to suitable system-wide locations. The build process will need to be adjusted.
Paths were fixed, see previous comments.
- MUST: A package must own all directories that it creates. If it does not
create a directory that it uses, then it should require a package which does create that directory.
FAIL - please see attached patch
Fixed, see previous comment
- MUST: A Fedora package must not list a file more than once in the spec file's
%files listings.
FAIL - please see attached patch
Fixed
- MUST: Permissions on files must be set properly. Executables should be set
with executable permissions, for example. Every <code>%files</code> section must include a <code>%defattr(...)</code> line.
FAIL - please see attached patch
I believe all files have right permissions.
- MUST: Each package must have a %clean section, which contains <code>rm -rf
%{buildroot}</code> ([[Packaging/Guidelines#UsingBuildRootOptFlags|or $RPM_BUILD_ROOT]]).
WARN - noted earlier in rpmlint output
Fixed.
- MUST: At the beginning of <code>%install</code>, each package MUST run
<code>rm -rf %{buildroot}</code> ([[Packaging/Guidelines#UsingBuildRootOptFlags|or $RPM_BUILD_ROOT]]).
FAIL - please see attached patch
Fixed.
- SHOULD: If the source package does not include license text(s) as a separate
file from upstream, the packager SHOULD query upstream to include it.
Pmuller is upstream in this case, I've recommended including a LICENSE file
Included.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #12 from James Laska jlaska@redhat.com 2010-02-22 19:11:56 EST ---
Spec URL: http://www.afri.cz/files/beakerlib.spec SRPM URL: http://www.afri.cz/files/beakerlib-1.0-2.src.rpm
The src.rpm URL does not work, I've instead used http://afri.fedorapeople.org/beakerlib/beakerlib-1.0-2.fc12.src.rpm
[ FAIL ] MUST: rpmlint must be run on every package beakerlib.src: W: invalid-url Source0: beakerlib-1.0.tar.gz beakerlib.spec: W: invalid-url Source0: beakerlib-1.0.tar.gz
I believe you mixed the definitions of URL and Source0. The following patch resolves the problem accordingly.
--- /tmp/jlaska-rpm/SPECS/beakerlib.spec 2010-02-12 11:00:20.000000000 -0500 +++ beakerlib.spec 2010-02-22 19:07:48.534865389 -0500 @@ -8 +8 @@ -Source0: %{name}-%{version}.tar.gz +Source0: https://fedorahosted.org/%%7Bname%7D/attachment/wiki/tarballs/%%7Bname%7D-%%... @@ -10 +10 @@ -URL: https://fedorahosted.org/beakerlib/attachment/wiki/tarballs/beakerlib-1.0.ta... +URL: https://fedorahosted.org/%%7Bname%7D
[ OK ] MUST: The package must be named according to the Package Naming Guidelines [ OK ] MUST: The spec file name must match the base package %{name} [...] [ OK ] MUST: The package must meet the Packaging Guidelines [ OK ] MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines [ FAIL ] MUST: The License field in the package spec file must match the actual license
beakerlib.spec noted GPLv2, however the following files included in beakerlib show 'GPLv2 or later': git_rules.mk:1:# License: GPL v2 or later py_rules.mk:1:# License: GPL v2 or later rpmspec_rules.mk:1:# License: GPL v2 or later upload_rules.mk:1:# License: GPL v2 or later scm_rules.mk:1:# License: GPL v2 or later
I think beakerlib.spec should note "GPLv2+" or the *.mk files should be adjusted
[ OK ] MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc [ OK ] MUST: The spec file must be written in American English. [ WARN ] MUST: The spec file for the package MUST be legible.
It's common practice to include two newlines prior to each %section in the spec file. I'd recommend the same here for readability. Please see the tool rpmdev-newspec (provided by rpmdevtools) or the spec template included in vim now.
[ FAIL ] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this.
The Source0 line must be a URL to the location of the source upstream. Something like (note, the URL will depend on the location you post this, but I'd recommend anchored somewhere from your fedorahosted project space) ...
Source0: http://fedorahosted.org/releases/b/e/beakerlib/beakerlib-%%7Bversion%7D.tar....
See http://fedoraproject.org/wiki/Packaging/SourceURL for more info
[ OK ] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture
koji scratch build - http://koji.fedoraproject.org/koji/taskinfo?taskID=2007048
[ N/A ] MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line [ OK ] MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense. [ N/A ] MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden [ N/A ] MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. [ N/A ] MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker. [ OK ] MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. [ OK ] MUST: A package must not contain any duplicate files in the %files listing. [ OK ] MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line. [ OK ] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [ OK ] MUST: Each package must consistently use macros. [ OK ] MUST: The package must contain code, or permissable content. [ N/A ] MUST: Large documentation files must go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity). [ OK ] MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. [ N/A ] MUST: Header files must be in a -devel package. [ N/A ] MUST: Static libraries must be in a -static package. [ N/A ] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). [ N/A ] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. [ N/A ] MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} [ N/A ] MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built. [ N/A ] MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation. [ OK ] MUST: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time. [ OK ] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [ OK ] MUST: All filenames in rpm packages must be valid UTF-8.
[ N/A ] SHOULD: If the source package does not include license text(s) as a separate file from upstream, should query upstream to include it. [ N/A ] SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. [ OK ] SHOULD: The reviewer should test that the package builds in mock.
koji scratch build - http://koji.fedoraproject.org/koji/taskinfo?taskID=2007048
[ OK ] SHOULD: The package should compile and build into binary rpms on all supported architectures.
See above
[ OK ] SHOULD: The reviewer should test that the package functions as described. A package should not segfault instead of running, for example.
Being used in the AutoQA project https://fedorahosted.org/pipermail/autoqa-devel/2010-February/000217.html
[ N/A ] SHOULD: If scriptlets are used, those scriptlets must be sane. This is vague, and left up to the reviewers judgement to determine sanity. [ N/A ] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. [ N/A ] SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg. A reasonable exception is that the main pkg itself is a devel tool not installed in a user runtime, e.g. gcc or gdb. [ OK ] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself. [ OK ] SHOULD: your package should contain man pages for binaries/scripts. If it doesn't, work with upstream to add them where they make sense.[34]
/usr/share/man/man1/beakerlib-analyze.1.gz /usr/share/man/man1/beakerlib-beakerlib.1.gz /usr/share/man/man1/beakerlib-infrastructure.1.gz /usr/share/man/man1/beakerlib-journal.1.gz /usr/share/man/man1/beakerlib-logging.1.gz /usr/share/man/man1/beakerlib-performance.1.gz /usr/share/man/man1/beakerlib-rpms.1.gz /usr/share/man/man1/beakerlib-testing.1.gz /usr/share/man/man1/beakerlib-virtualX.1.gz /usr/share/man/man1/beakerlib.1.gz
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #13 from Petr Muller pmuller@redhat.com 2010-03-22 13:11:38 EDT --- (In reply to comment #12)
Spec URL: http://www.afri.cz/files/beakerlib.spec SRPM URL: http://www.afri.cz/files/beakerlib-1.0-2.src.rpm
The src.rpm URL does not work, I've instead used http://afri.fedorapeople.org/beakerlib/beakerlib-1.0-2.fc12.src.rpm
Sorry for that, I b0rked the filename. New ones:
http://www.afri.cz/files/beakerlib-1.1-0.fc12.src.rpm http://www.afri.cz/files/beakerlib.spec
[ FAIL ] MUST: rpmlint must be run on every package beakerlib.src: W: invalid-url Source0: beakerlib-1.0.tar.gz beakerlib.spec: W: invalid-url Source0: beakerlib-1.0.tar.gz
I believe you mixed the definitions of URL and Source0. The following patch resolves the problem accordingly.
Thanks for the patch - it should be fixed now. Source0: https://fedorahosted.org/released/%%7Bname%7D/%%7Bname%7D-%%7Bversion%7D.tar... URL: https://fedorahosted.org/%%7Bname%7D
$ rpmlint beakerlib-1.1-0.fc12.src.rpm beakerlib-1.1-0.fc12.noarch.rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings.
[ FAIL ] MUST: The License field in the package spec file must match the actual license
beakerlib.spec noted GPLv2, however the following files included in beakerlib show 'GPLv2 or later': git_rules.mk:1:# License: GPL v2 or later py_rules.mk:1:# License: GPL v2 or later rpmspec_rules.mk:1:# License: GPL v2 or later upload_rules.mk:1:# License: GPL v2 or later scm_rules.mk:1:# License: GPL v2 or later I think beakerlib.spec should note "GPLv2+" or the *.mk files should be adjusted
Fixed to GPLv2, one file was removed.
[ WARN ] MUST: The spec file for the package MUST be legible.
It's common practice to include two newlines prior to each %section in the spec file. I'd recommend the same here for readability. Please see the tool rpmdev-newspec (provided by rpmdevtools) or the spec template included in vim now.
Thanks, I didn't know that. I've added empty lines between sections.
[ FAIL ] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this.
The Source0 line must be a URL to the location of the source upstream. Something like (note, the URL will depend on the location you post this, but I'd recommend anchored somewhere from your fedorahosted project space) ...
I've moved the tarball to the fedorahosted release infrastructure, the tarballs will be here now: https://fedorahosted.org/released/beakerlib/
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #14 from James Laska jlaska@redhat.com 2010-03-24 13:59:20 EDT --- Thanks for the updates Petr. Those changes look good.
While re-reviewing the licensing, I noticed that the file 'src/test/shunit2' lists LGPL in the header. There's the issue of lining up the licensing information of shunit2.
Before handling lining up with the license in the header of 'src/test/shunit2', I think we need to clarify whether this constitutes bundling a library. Is this code upstream somewhere? Perhaps one of these projects?
* shunit - http://shunit.sourceforge.net/ * shunit2 - http://code.google.com/p/shunit2/
Can you explain how shunit2 is used? The 'src/test/README' file doesn't seem to have correct links to the project. Either way, I suspect we might not be allowed to include another project inside beakerlib. I have a suspicion shunit2 would need to be packaged separately in Fedora.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
Tom "spot" Callaway tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |tcallawa@redhat.com
--- Comment #15 from Tom "spot" Callaway tcallawa@redhat.com 2010-03-24 14:07:40 EDT --- Yeah, you really do want to package shunit2 independently.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #16 from Petr Muller pmuller@redhat.com 2010-04-08 09:59:35 EDT --- For now, I have removed the offending file entirely - we will replace it with a simple minilibrary which we will write. Or perhaps package shunit2, I don't know now, but I don't want this to be blocker of beakerlib in Fedor a.
New links: http://www.afri.cz/files/beakerlib-1.2-0.fc12.src.rpm http://www.afri.cz/files/beakerlib.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
James Laska jlaska@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(tcallawa@redhat.c | |om)
--- Comment #17 from James Laska jlaska@redhat.com 2010-04-13 12:45:06 EDT --- Looks good Petr, thanks for the updates. I can confirm that shunit2 is no longer packaged with beakerlib, and from what I can tell, all license information appears kosher.
# rpmlint beakerlib-1.2-0.fc13.noarch.rpm beakerlib-1.2-0.fc12.src.rpm beakerlib.spec 2 packages and 1 specfiles checked; 0 errors, 0 warnings.
tcallawa: Any other packaging concerns you can think of?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
Tom "spot" Callaway tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(tcallawa@redhat.c | |om) |
--- Comment #18 from Tom "spot" Callaway tcallawa@redhat.com 2010-04-13 12:59:58 EDT --- Nope. :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
Terje Røsten terjeros@phys.ntnu.no changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |terjeros@phys.ntnu.no
--- Comment #19 from Terje Røsten terjeros@phys.ntnu.no 2010-04-13 17:34:14 EDT --- Some pedantic stuff:
o why start release with 0 and not 1? o remove trailing ; here: %clean rm -rf $RPM_BUILD_ROOT; o add a comment why %build is empty o use INSTALL='install -p' in make install to preserve timestamps o it's very common to have documentation in %{_docdir}/%{name}-%{version}, any reason not the follow that?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |kevin@tummy.com
--- Comment #20 from Kevin Fenzi kevin@tummy.com 2010-04-19 14:22:11 EDT --- Petr: Can you address the questions in comment #19?
I can look at reviewing this formally and sponsoring you after that...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #21 from Petr Muller pmuller@redhat.com 2010-04-29 10:14:55 EDT --- Thanks for your points!
(In reply to comment #19)
Some pedantic stuff:
o why start release with 0 and not 1?
Well, I tend to start everything from zero :) I will keep that in mind for the next time
o remove trailing ; here: %clean rm -rf $RPM_BUILD_ROOT;
done
o add a comment why %build is empty
added
o use INSTALL='install -p' in make install to preserve timestamps
fix'd
o it's very common to have documentation in %{_docdir}/%{name}-%{version}, any reason not the follow that?
I've changed Makefiles and specfile to follow that.
(In reply to comment #20)
Petr: Can you address the questions in comment #19?
I can look at reviewing this formally and sponsoring you after that...
Sorry for the delay, I was travelling and I avoided computer stuff for a while :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #22 from Petr Muller pmuller@redhat.com 2010-04-29 10:15:19 EDT --- New links: http://www.afri.cz/files/beakerlib-1.2-1.fc12.src.rpm http://www.afri.cz/files/beakerlib.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |kevin@tummy.com Flag| |fedora-review?
--- Comment #23 from Kevin Fenzi kevin@tummy.com 2010-05-02 18:34:55 EDT --- Look for a full review in a bit.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #24 from Kevin Fenzi kevin@tummy.com 2010-05-02 18:46:52 EDT ---
OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License (GPLv2) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. See below - Sources match upstream md5sum:
5e5ee854add958ce30746a15b9d7e713 beakerlib-1.2.tar.gz ae18ea068c48e82196ff6cd381e663d9 beakerlib-1.2.tar.gz.orig
OK - BuildRequires correct See below - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package has rm -rf RPM_BUILD_ROOT at top of %install
OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. OK - Package obey's FHS standard (except for 2 exceptions) See below - No rpmlint output. See below - final provides and requires are sane.
SHOULD Items:
OK - Should build in mock. OK - Should build on all supported archs OK - Should have dist tag OK - Should package latest version OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin
Issues:
1. Source doesn't match from upstream: 5e5ee854add958ce30746a15b9d7e713 beakerlib-1.2.tar.gz ae18ea068c48e82196ff6cd381e663d9 beakerlib-1.2.tar.gz.orig
2. rpmlint says:
Probibly all these should be chmod 644 or have a #!/bin/sh added if they should be called stand alone:
beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/analyze.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/beakerlib.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/dictionary.vim beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/infrastructure.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/journal.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/logging.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/performance.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/rpms.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/testing.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/virtualX.sh
This can be ignored, but it would be nice to have man pages for these:
beakerlib.noarch: W: no-manual-page-for-binary beakerlib-deja-summarize beakerlib.noarch: W: no-manual-page-for-binary beakerlib-journalcmp beakerlib.noarch: W: no-manual-page-for-binary beakerlib-journalling beakerlib.noarch: W: no-manual-page-for-binary beakerlib-rlMemAvg beakerlib.noarch: W: no-manual-page-for-binary beakerlib-rlMemPeak
All these should be mode 644:
beakerlib.noarch: W: spurious-executable-perm /usr/share/doc/beakerlib-1.2/LICENSE beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-analyze.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-beakerlib.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-infrastructure.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-journal.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-logging.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-performance.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-rpms.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-testing.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-virtualX.1.gz
3. Shouldn't this have a 'Requires: python2' ?
4. Should you add the examples as %doc files?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #25 from Petr Muller pmuller@redhat.com 2010-05-12 07:51:44 EDT --- Kevin,
first of all, thanks for reviewing!
New links: http://www.afri.cz/files/beakerlib-1.3-1.fc12.src.rpm http://www.afri.cz/files/beakerlib.spec
(In reply to comment #24)
Issues:
- Source doesn't match from upstream:
5e5ee854add958ce30746a15b9d7e713 beakerlib-1.2.tar.gz ae18ea068c48e82196ff6cd381e663d9 beakerlib-1.2.tar.gz.orig3
It should match now. The fact that we are also upstream (and we track the specfile in repo, and it then gets in the tarball) makes this a bit complicated to maintain, because the tarball changes everytime I change even the specfile.
- rpmlint says:
Probibly all these should be chmod 644 or have a #!/bin/sh added if they should be called stand alone:
beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/analyze.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/beakerlib.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/dictionary.vim beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/infrastructure.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/journal.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/logging.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/performance.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/rpms.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/testing.sh beakerlib.noarch: E: script-without-shebang /usr/share/beakerlib/virtualX.sh
Not intended for standalone calling, I've fixed the perms.
This can be ignored, but it would be nice to have man pages for these:
beakerlib.noarch: W: no-manual-page-for-binary beakerlib-deja-summarize beakerlib.noarch: W: no-manual-page-for-binary beakerlib-journalcmp beakerlib.noarch: W: no-manual-page-for-binary beakerlib-journalling beakerlib.noarch: W: no-manual-page-for-binary beakerlib-rlMemAvg beakerlib.noarch: W: no-manual-page-for-binary beakerlib-rlMemPeak
Good suggestion. We'll try to write something for the ones which are supposed to be executed directly, not from inside Beakerlib
All these should be mode 644:
beakerlib.noarch: W: spurious-executable-perm /usr/share/doc/beakerlib-1.2/LICENSE beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-analyze.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-beakerlib.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-infrastructure.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-journal.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-logging.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-performance.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-rpms.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-testing.1.gz beakerlib.noarch: W: spurious-executable-perm /usr/share/man/man1/beakerlib-virtualX.1.gz
Perms fixed
- Shouldn't this have a 'Requires: python2' ?
Seems so. Added.
- Should you add the examples as %doc files?
Good idea, thanks! Added.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841(FE-NEEDSPONSOR) | Flag|fedora-review? |fedora-review+
--- Comment #26 from Kevin Fenzi kevin@tummy.com 2010-05-13 00:35:45 EDT --- 1. Looks good.
13df10e1b94c8a1abee91e9ef111dd29 beakerlib-1.3.tar.gz 13df10e1b94c8a1abee91e9ef111dd29 beakerlib-1.3.tar.gz.orig
2. Looks good.
rpmlint now says:
beakerlib.noarch: W: no-manual-page-for-binary beakerlib-rlMemAvg beakerlib.noarch: W: no-manual-page-for-binary beakerlib-rlMemPeak beakerlib.noarch: W: no-manual-page-for-binary beakerlib-journalling beakerlib.noarch: W: no-manual-page-for-binary beakerlib-journalcmp beakerlib.noarch: W: no-manual-page-for-binary beakerlib-deja-summarize
3. Looks good.
4. Looks good.
I don't see any further blockers here, so this package was APPROVED.
I will go ahead and sponsor you. What is your Fedora Account system account? Once you are sponsored, you can continue the process at: https://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_an...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #27 from Petr Muller pmuller@redhat.com 2010-05-14 09:59:33 EDT --- Kevin,
Thanks for sponsoring. My FAS account name is 'afri'.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #28 from Kevin Fenzi kevin@tummy.com 2010-05-19 12:08:24 EDT --- Sorry for the delay, I was off traveling. ;(
I have sponsored you now. Please continue from https://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_an...
If you have any questions at all, please feel free to email me or catch me on irc (my nick is 'nirik').
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
Petr Muller pmuller@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #29 from Petr Muller pmuller@redhat.com 2010-05-25 11:34:55 EDT --- New Package CVS Request ======================= Package Name: beakerlib Short Description: A shell-level integration testing library Owners: afri Branches: F-13 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
Dennis Gilmore dennis@ausil.us changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #30 from Dennis Gilmore dennis@ausil.us 2010-05-25 16:57:43 EDT --- CVS Done
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #31 from Fedora Update System updates@fedoraproject.org 2010-05-31 08:28:03 EDT --- beakerlib-1.3-1.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/beakerlib-1.3-1.fc13
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
--- Comment #32 from Fedora Update System updates@fedoraproject.org 2010-06-01 14:16:03 EDT --- beakerlib-1.3-1.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=561470
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Fixed In Version| |beakerlib-1.3-1.fc13 Resolution| |ERRATA
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla