Bug 561470 - Review Request: beakerlib - shell-level integration testing library
Summary: Review Request: beakerlib - shell-level integration testing library
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-02-03 18:00 UTC by Petr Muller
Modified: 2016-09-20 02:06 UTC (History)
8 users (show)

Fixed In Version: beakerlib-1.3-1.fc13
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-06-01 18:16:12 UTC
kevin: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
spec file patch (2.42 KB, patch)
2010-02-05 19:37 UTC, James Laska
no flags Details | Diff

Description Petr Muller 2010-02-03 18:00:31 UTC
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.

Comment 1 Petr Muller 2010-02-03 18:04:44 UTC
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.

Comment 2 James Laska 2010-02-05 18:29:21 UTC
I can do the review, but I'm not able to sponsor you.

Comment 3 Jason Tibbitts 2010-02-05 19:23:06 UTC
The review must be done by the sponsor, at least according to Fedora policy.

Comment 4 James Laska 2010-02-05 19:33:56 UTC
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#USRLIBLIBRARIESFORPROGRAMMINGANDPA) -

    "/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!)

Comment 5 James Laska 2010-02-05 19:35:40 UTC
(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.

Comment 6 James Laska 2010-02-05 19:37:47 UTC
Created attachment 389172 [details]
spec file patch

Comment 7 Petr Šplíchal 2010-02-08 09:44:55 UTC
> 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.

Comment 8 Michael Schwendt 2010-02-08 12:55:40 UTC
> 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.


> https://fedorahosted.org/git/beakerlib.git

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*

Comment 9 Petr Muller 2010-02-09 16:27:06 UTC
Thanks you for all the feedback! I'll fix everything and let know...

Comment 10 Petr Muller 2010-02-12 16:12:43 UTC
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.

> > https://fedorahosted.org/git/beakerlib.git> 
> 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

Comment 11 Petr Muller 2010-02-12 16:27:03 UTC
(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.

Comment 12 James Laska 2010-02-23 00:11:56 UTC
> 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/%{name}/attachment/wiki/tarballs/%{name}-%{version}.tar.gz
@@ -10 +10 @@
-URL:        https://fedorahosted.org/beakerlib/attachment/wiki/tarballs/beakerlib-1.0.tar.gz
+URL:        https://fedorahosted.org/%{name}

[  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-%{version}.tar.bz2

         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

Comment 13 Petr Muller 2010-03-22 17:11:38 UTC
(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/%{name}/%{name}-%{version}.tar.gz
URL:        https://fedorahosted.org/%{name}

$ 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/

Comment 14 James Laska 2010-03-24 17:59:20 UTC
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.

Comment 15 Tom "spot" Callaway 2010-03-24 18:07:40 UTC
Yeah, you really do want to package shunit2 independently.

Comment 16 Petr Muller 2010-04-08 13:59:35 UTC
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

Comment 17 James Laska 2010-04-13 16:45:06 UTC
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?

Comment 18 Tom "spot" Callaway 2010-04-13 16:59:58 UTC
Nope. :)

Comment 19 Terje Røsten 2010-04-13 21:34:14 UTC
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?

Comment 20 Kevin Fenzi 2010-04-19 18:22:11 UTC
Petr: Can you address the questions in comment #19?

I can look at reviewing this formally and sponsoring you after that...

Comment 21 Petr Muller 2010-04-29 14:14:55 UTC
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 :)

Comment 23 Kevin Fenzi 2010-05-02 22:34:55 UTC
Look for a full review in a bit.

Comment 24 Kevin Fenzi 2010-05-02 22:46:52 UTC
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?

Comment 25 Petr Muller 2010-05-12 11:51:44 UTC
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: 
> 
> 1. 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.
> 
> 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

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

> 3. Shouldn't this have a 'Requires: python2' ? 

Seems so. Added.

> 4. Should you add the examples as %doc files?

Good idea, thanks! Added.

Comment 26 Kevin Fenzi 2010-05-13 04:35:45 UTC
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_and_Set_Owner

Comment 27 Petr Muller 2010-05-14 13:59:33 UTC
Kevin, 

Thanks for sponsoring. My FAS account name is 'afri'.

Comment 28 Kevin Fenzi 2010-05-19 16:08:24 UTC
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_and_Set_Owner

If you have any questions at all, please feel free to email me or catch me on irc (my nick is 'nirik').

Comment 29 Petr Muller 2010-05-25 15:34:55 UTC
New Package CVS Request
=======================
Package Name: beakerlib
Short Description: A shell-level integration testing library
Owners: afri
Branches: F-13
InitialCC:

Comment 30 Dennis Gilmore 2010-05-25 20:57:43 UTC
CVS Done

Comment 31 Fedora Update System 2010-05-31 12:28:03 UTC
beakerlib-1.3-1.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/beakerlib-1.3-1.fc13

Comment 32 Fedora Update System 2010-06-01 18:16:03 UTC
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.

Comment 33 Petr Muller 2014-06-12 08:56:08 UTC
Package Change Request
======================
Package Name: beakerlib
New Branches: el5 el6 epel7
Owners: afri

Just want to maintain this package in EPEL too

Comment 34 Gwyn Ciesla 2014-06-12 12:03:56 UTC
Git done (by process-git-requests).


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