This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 205032 - Review Request: perl-Test-AutoBuild - a framework for continuous, unatttended software builds
Review Request: perl-Test-AutoBuild - a framework for continuous, unatttended...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Chris Weyl
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-09-02 15:14 EDT by Daniel Berrange
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-14 19:06:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Daniel Berrange 2006-09-02 15:14:17 EDT
Spec URL: http://berrange.com/~dan/fedora-review/perl-Test-AutoBuild/Test-AutoBuild.spec
SRPM URL: 
http://berrange.com/~dan/fedora-review/perl-Test-AutoBuild/perl-Test-AutoBuild-1.2.0-1.src.rpm
Description: Test-AutoBuild is a Perl framework for performing continuous, unattended, automated software builds. It is targetted at upstream developers, rather than package distributors. It provides a workflow engine for performing continuous integration. It checks source out of an SCM repository; then builds it, tests it, installs it & packages it; Finally it publishes any build artifacts, build logs, code reports, packages & HTML status pages to HTTP/FTP site. This is repeated 24x7, scheduled via cron.

The core of software is written in Perl, but it calls out to command line utils for integration with a variety of SCM systems. The SCM systems supported are CVS, GNU Arch, Subversion, Mercurial, SVK and Perforce. The spec file is setup such that the modules for each SCM system are provided in separate sub-RPMs. This split was repeatedly requested by users, since if they're using one SCM system (eg Subversion), they did not want to have to install the other 6 SCM systems & their huge dependancy chains. 

In addition to the sub-RPMs for each SCM system, there is one further sub-RPM called '-account'. This creates a user account 'builder' and a directory structure under '/var/lib/builder' populated with all the files neccessary to get an instance of the autobuild engine up & running. rpmlint will flag files in this sub-RPM as using a non-standard user account, however, this user account is created in the %post script of the sub-RPM.  cf. mock RPM creating its own group & directory in /var/lib/mock.

I have also patched the upstream tar.gz to remove the module which calls 'yum-arch' since this no longer appears to be distributed in Fedora.

This is my second Fedora Extras submission, although I still need sponsership.

My other submission is bug 205029
Comment 1 Chris Weyl 2006-09-06 23:28:31 EDT
Removing FE-NEEDSPONSOR per bug 205029.
Comment 2 Chris Weyl 2006-09-07 01:30:21 EDT
A couple guideline issues, and a ton of rpmlint messages.

The rpmlint warnings/errors all look to be false positives, except for the
ones against the src package.  Those 4 need to be addressed.

With respect to macros, it's important to remain consistent across the board.
e.g., 'make' is used in one spot, and '%__make' in another; %__chmod is used
for chmod but %__cp isn't used for cp.  Everything either needs to be
macroized or not.

With the -svk subpackage, is there a reason to prefer the requires as coded to
the perl-SVK package, rather than to the virtual perl(SVK)?

With the -perforce subpackage, I was initially uncomfortable with it depending
on software not in core/extras.  However, there are no explicit rpm deps
generated from it, and it's an optional subpackage that works iff perforce is
installed (the way spamassassin uses razor or dcc if present, rhythmbox will
play mp3's if the right packages are installed, etc, etc).

While using the usermgmt tools is not required by the guidelines, a good case
is made for it.  This isn't a blocker, just a nudge :)
  http://www.fedoraproject.org/wiki/PackageUserCreation
  http://www.fedoraproject.org/wiki/PackageUserRegistry

There also appears to be a docs/ directory that isn't packaged; it's only 44k,
why not %doc it in the main package?

For the %pre accounts scriptlet, I'm running into errors:

  [cweyl@zeus tmp]$ sudo rpm -ivh perl-Test-AutoBuild-account-1.2.0-1.fc5.noarch.rpm
  Preparing...                ########################################### [100%]
  useradd: cannot create directory /var/lib/builder
  usermod: user builder does not exist
  error: %pre(perl-Test-AutoBuild-account-1.2.0-1.fc5.noarch) scriptlet failed,
exit status 6
  error:   install: %pre scriptlet failed (2), skipping
perl-Test-AutoBuild-account-1.2.0-1.fc5

There are also two tests that are skipped due to missing buildrequires:
  t/005-pod......................skipped
          all skipped: Test::Pod 1.00 required for testing POD
  t/010-pod-coverage.............skipped
          all skipped: Test::Pod::Coverage 1.00 required for testing POD coverage

So, to recap:
* address rpmlint messages on src.rpm
* make macros consistent (either all %__ or none)
* fix the %pre accounts scriptlet
* add doc/ to %doc in the main package
* add buildrequires on perl(Test::Pod::Coverage) and perl(Test::Pod)

+ package meets naming and packaging guidelines.
X specfile is not properly named (needs to be perl-Test-AutoBuild.spec)
X specfile is cleanly written but does not use macros consistently.
+ dist tag is present.
X build root is not correct.
    should be:  %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
+ license field matches the actual license.
+ license is open source-compatible. (GPL) License text included in package.
+ source files match upstream:
7c1d0e9aaa9f9b19b363f31963d5f240  Test-AutoBuild-1.2.0.tar.gz
7c1d0e9aaa9f9b19b363f31963d5f240  Test-AutoBuild-1.2.0.tar.gz.srpm
+ latest version is being packaged.
X BuildRequires are proper.
+ package builds in mock (5/x86_64).
X rpmlint is silent. (see below)
 final provides and requires are sane:
** perl-Test-AutoBuild-1.2.0-1.fc5.src.rpm
== rpmlint
W: perl-Test-AutoBuild strange-permission Test-AutoBuild.spec 0600
E: perl-Test-AutoBuild invalid-spec-name Test-AutoBuild.spec
W: perl-Test-AutoBuild hardcoded-path-in-buildroot-tag
/var/tmp/%{appname}-%{version}-root
W: perl-Test-AutoBuild patch-not-applied Patch1:
Test-AutoBuild-remove-yumarch.patch
** perl-Test-AutoBuild-1.2.0-1.fc5.noarch.rpm
== rpmlint
== provides
config(perl-Test-AutoBuild) = 1.2.0-1.fc5
perl(Test::AutoBuild) = 1.2.0
perl(Test::AutoBuild::Archive)
perl(Test::AutoBuild::Archive::File)
perl(Test::AutoBuild::Archive::Memory)
perl(Test::AutoBuild::ArchiveManager)
perl(Test::AutoBuild::ArchiveManager::File)
perl(Test::AutoBuild::ArchiveManager::Memory)
perl(Test::AutoBuild::Change)
perl(Test::AutoBuild::Counter)
perl(Test::AutoBuild::Counter::ChangeList)
perl(Test::AutoBuild::Counter::Time)
perl(Test::AutoBuild::Counter::Timestamp)
perl(Test::AutoBuild::ErrorReport)
perl(Test::AutoBuild::Group)
perl(Test::AutoBuild::Lib)
perl(Test::AutoBuild::Lock)
perl(Test::AutoBuild::Module)
perl(Test::AutoBuild::Monitor)
perl(Test::AutoBuild::Monitor::CommandLine)
perl(Test::AutoBuild::Monitor::Log4perl)
perl(Test::AutoBuild::Monitor::Pipe)
perl(Test::AutoBuild::Package)
perl(Test::AutoBuild::PackageType)
perl(Test::AutoBuild::Platform)
perl(Test::AutoBuild::Publisher)
perl(Test::AutoBuild::Publisher::Copy)
perl(Test::AutoBuild::Publisher::XSLTransform)
perl(Test::AutoBuild::Repository)
perl(Test::AutoBuild::Repository::Disk)
perl(Test::AutoBuild::Result)
perl(Test::AutoBuild::Runtime) = 1.1.0
perl(Test::AutoBuild::Stage)
perl(Test::AutoBuild::Stage::Apt)
perl(Test::AutoBuild::Stage::ArtifactCopier)
perl(Test::AutoBuild::Stage::Build)
perl(Test::AutoBuild::Stage::CleanArchive)
perl(Test::AutoBuild::Stage::CleanBuildRoots)
perl(Test::AutoBuild::Stage::CleanPackages)
perl(Test::AutoBuild::Stage::Copier)
perl(Test::AutoBuild::Stage::CreateArchive)
perl(Test::AutoBuild::Stage::CreateRepo)
perl(Test::AutoBuild::Stage::EmailAlert)
perl(Test::AutoBuild::Stage::GetSource)
perl(Test::AutoBuild::Stage::Group)
perl(Test::AutoBuild::Stage::HTMLStatus)
perl(Test::AutoBuild::Stage::ISOBuilder)
perl(Test::AutoBuild::Stage::Iterator)
perl(Test::AutoBuild::Stage::LogCopier)
perl(Test::AutoBuild::Stage::PackageCopier)
perl(Test::AutoBuild::Stage::SetNice)
perl(Test::AutoBuild::Stage::TemplateGenerator)
perl(Test::AutoBuild::Stage::Test)
perl(Test::AutoBuild::Stage::Yum)
perl-Test-AutoBuild = 1.2.0-1.fc5
== requires
/bin/sh
/usr/bin/createrepo
/usr/bin/genbasedir
/usr/bin/mkisofs
/usr/bin/perl
/usr/bin/xsltproc
config(perl-Test-AutoBuild) = 1.2.0-1.fc5
perl(:MODULE_COMPAT_5.8.8)
perl(BSD::Resource)
perl(Carp)
perl(Class::MethodMaker)
perl(Config)
perl(Config::Record)
perl(Cwd)
perl(Data::Dumper)
perl(Date::Manip)
perl(Digest::MD5)
perl(Fcntl)
perl(File::Copy)
perl(File::Find)
perl(File::Glob)
perl(File::Path)
perl(File::Spec)
perl(File::Spec::Functions)
perl(File::Temp)
perl(File::stat)
perl(Getopt::Long)
perl(IO::File)
perl(IO::Scalar)
perl(List::Util)
perl(Log::Log4perl)
perl(Net::SMTP)
perl(POSIX)
perl(Storable)
perl(Sys::Hostname)
perl(Template)
perl(Test::AutoBuild)
perl(Test::AutoBuild::Archive::File)
perl(Test::AutoBuild::Archive::Memory)
perl(Test::AutoBuild::ErrorReport)
perl(Test::AutoBuild::Group)
perl(Test::AutoBuild::Lib)
perl(Test::AutoBuild::Package)
perl(Test::AutoBuild::PackageType)
perl(Test::AutoBuild::Platform)
perl(Test::AutoBuild::Publisher)
perl(Test::AutoBuild::Result)
perl(Test::AutoBuild::Runtime)
perl(Test::AutoBuild::Stage::Group)
perl(UNIVERSAL)
perl(base)
perl(integer)
perl(overload)
perl(strict)
perl(vars)
perl(warnings)
** perl-Test-AutoBuild-account-1.2.0-1.fc5.noarch.rpm
== rpmlint
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/RPMS/x86_64 builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/RPMS/x86_64 builder
E: perl-Test-AutoBuild-account non-standard-uid /var/lib/builder/.cvspass
builder
E: perl-Test-AutoBuild-account non-standard-gid /var/lib/builder/.cvspass
builder
W: perl-Test-AutoBuild-account hidden-file-or-dir /var/lib/builder/.cvspass
E: perl-Test-AutoBuild-account non-readable /var/lib/builder/.cvspass 0600
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/debian builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/debian builder
E: perl-Test-AutoBuild-account non-standard-uid /var/lib/builder builder
E: perl-Test-AutoBuild-account non-standard-gid /var/lib/builder builder
E: perl-Test-AutoBuild-account non-standard-uid /var/lib/builder/package-root
builder
E: perl-Test-AutoBuild-account non-standard-gid /var/lib/builder/package-root
builder
E: perl-Test-AutoBuild-account non-standard-uid /var/lib/builder/install-root
builder
E: perl-Test-AutoBuild-account non-standard-gid /var/lib/builder/install-root
builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/RPMS builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/RPMS builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/zips builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/zips builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/SRPMS builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/SRPMS builder
E: perl-Test-AutoBuild-account non-standard-uid /var/lib/builder/source-root
builder
E: perl-Test-AutoBuild-account non-standard-gid /var/lib/builder/source-root
builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/tars builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/tars builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/RPMS/i686 builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/RPMS/i686 builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/RPMS/ia32e builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/RPMS/ia32e builder
E: perl-Test-AutoBuild-account non-standard-uid /var/lib/builder/public_html
builder
E: perl-Test-AutoBuild-account non-standard-gid /var/lib/builder/public_html
builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/RPMS/ia64 builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/RPMS/ia64 builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/BUILD builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/BUILD builder
E: perl-Test-AutoBuild-account non-standard-uid /var/lib/builder/public_ftp
builder
E: perl-Test-AutoBuild-account non-standard-gid /var/lib/builder/public_ftp
builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/RPMS/sparc builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/RPMS/sparc builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/SPECS builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/SPECS builder
E: perl-Test-AutoBuild-account non-standard-uid /var/lib/builder/build-archive
builder
E: perl-Test-AutoBuild-account non-standard-gid /var/lib/builder/build-archive
builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/RPMS/noarch builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/RPMS/noarch builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/RPMS/i486 builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/RPMS/i486 builder
E: perl-Test-AutoBuild-account non-standard-uid /var/lib/builder/log-root
builder
E: perl-Test-AutoBuild-account non-standard-gid /var/lib/builder/log-root
builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/RPMS/i586 builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/RPMS/i586 builder
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/SOURCES builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/SOURCES builder
E: perl-Test-AutoBuild-account non-standard-uid /var/lib/builder/.rpmmacros
builder
E: perl-Test-AutoBuild-account non-standard-gid /var/lib/builder/.rpmmacros
builder
W: perl-Test-AutoBuild-account hidden-file-or-dir /var/lib/builder/.rpmmacros
E: perl-Test-AutoBuild-account non-standard-uid
/var/lib/builder/package-root/rpm/RPMS/i386 builder
E: perl-Test-AutoBuild-account non-standard-gid
/var/lib/builder/package-root/rpm/RPMS/i386 builder
== provides
config(perl-Test-AutoBuild-account) = 1.2.0-1.fc5
perl-Test-AutoBuild-account = 1.2.0-1.fc5
== requires
/bin/sh
config(perl-Test-AutoBuild-account) = 1.2.0-1.fc5
perl-Test-AutoBuild = 1.2.0-1.fc5
** perl-Test-AutoBuild-cvs-1.2.0-1.fc5.noarch.rpm
== rpmlint
== provides
perl(Test::AutoBuild::Repository::CVS)
perl-Test-AutoBuild-cvs = 1.2.0-1.fc5
== requires
cvs >= 1.11
perl(Log::Log4perl)
perl(POSIX)
perl(base)
perl(strict)
perl(warnings)
perl-Test-AutoBuild = 1.2.0-1.fc5
** perl-Test-AutoBuild-mercurial-1.2.0-1.fc5.noarch.rpm
== rpmlint
== provides
perl(Test::AutoBuild::Repository::Mercurial)
perl-Test-AutoBuild-mercurial = 1.2.0-1.fc5
== requires
mercurial >= 0.7
perl(Date::Manip)
perl(Log::Log4perl)
perl(Test::AutoBuild::Change)
perl(base)
perl(strict)
perl(warnings)
perl-Test-AutoBuild = 1.2.0-1.fc5
** perl-Test-AutoBuild-perforce-1.2.0-1.fc5.noarch.rpm
== rpmlint
== provides
perl(Test::AutoBuild::Repository::Perforce)
perl-Test-AutoBuild-perforce = 1.2.0-1.fc5
== requires
perl(Date::Manip)
perl(File::Spec::Functions)
perl(Log::Log4perl)
perl(POSIX)
perl(Test::AutoBuild::Change)
perl(base)
perl(strict)
perl-Test-AutoBuild = 1.2.0-1.fc5
** perl-Test-AutoBuild-subversion-1.2.0-1.fc5.noarch.rpm
== rpmlint
== provides
perl(Test::AutoBuild::Repository::Subversion)
perl-Test-AutoBuild-subversion = 1.2.0-1.fc5
== requires
perl(Date::Manip)
perl(Log::Log4perl)
perl(POSIX)
perl(Test::AutoBuild::Change)
perl(base)
perl(strict)
perl-Test-AutoBuild = 1.2.0-1.fc5
subversion >= 1.0.0
** perl-Test-AutoBuild-svk-1.2.0-1.fc5.noarch.rpm
== rpmlint
== provides
perl(Test::AutoBuild::Repository::SVK)
perl-Test-AutoBuild-svk = 1.2.0-1.fc5
== requires
perl(Carp)
perl(base)
perl(strict)
perl-SVK >= 1.0
perl-Test-AutoBuild = 1.2.0-1.fc5
** perl-Test-AutoBuild-tla-1.2.0-1.fc5.noarch.rpm
== rpmlint
== provides
perl(Test::AutoBuild::Repository::GNUArch)
perl-Test-AutoBuild-tla = 1.2.0-1.fc5
== requires
perl(Carp)
perl(Date::Manip)
perl(Log::Log4perl)
perl(Test::AutoBuild::Change)
perl(base)
perl(strict)
perl-Test-AutoBuild = 1.2.0-1.fc5
tla >= 1.1.0
+ no shared libraries are present.
+ package is not relocatable.
X owns the directories it creates.
 main package missing:
 /etc/auto-build.d/
 /etc/auto-build.d/engine/
 ..etc
 %{perl_vendorlib}/Test/
 %{perl_vendorlib}/Test/AutoBuild/
 %{perl_vendorlib}/Test/AutoBuild/Repository/
 ...etc
+ doesn't own any directories it shouldn't.
+ no duplicates in %files.
+ file permissions are appropriate.
+ %clean is present.
+ %check is present and all tests pass (skips were expected):
All tests successful, 1 test and 36 subtests skipped.
Files=47, Tests=814, 43 wallclock secs (11.99 cusr +  5.49 csys = 17.48 CPU)
X sane scriptlets present.
+ code, not content.
+ documentation is small, so no -docs subpackage is necessary.
+ %docs are not necessary for the proper functioning of the package.
+ no headers.
+ no pkgconfig files.
+ no libtool .la droppings.
+ not a GUI app.
+ not a web app.
Comment 3 Daniel Berrange 2006-09-10 17:35:12 EDT
Thanks for the review feedback. I have addressed the following issues:

 - Renamed the SPEC file to match SRPM name
 - Use macros for all path prefixes
 - Added buildrequires on Test::Pod and Test::Pod::Coverage
 - Added docs/ dir to documentation files
 - Use macros for all programs used in build/install where available
 - Fixed buildroot to comply with Fedora standards
 - Make use of fedora-usermgmt package for creating builder account
 - Ensure main package owns all directories it creates in /etc/ & perl lib dir
 - Fixed %pre script to create dir for account
 - Fixed permissions of SPEC file
 - Fixed patch which was not applied

The reason I have a Requires on 'perl-SVK' instead of 'perl(SVK)' is because the
SVK integration actually uses the 'svk' script. A requires on 'perl(SVK)' would
only  represent the 'SVK.pm' module, not the script, hence I have the dependancy
on the package instead. 

Updated SPEC & SRPMs are available at the following new URLs:

http://berrange.com/~dan/fedora-review/perl-Test-AutoBuild/perl-Test-AutoBuild.spec
http://berrange.com/~dan/fedora-review/perl-Test-AutoBuild/perl-Test-AutoBuild-1.2.0-2.src.rpm
Comment 4 Chris Weyl 2006-09-14 01:08:35 EDT
perl-Test-AutoBuild still needs to own the directory %{perl_vendorlib}/Test.

Aside from that, looks good! :)

APPROVED, on the condition %dir %{perl_vendorlib}/Test is added to the main
package's %files.
Comment 5 Daniel Berrange 2006-09-14 17:04:30 EDT
Opps, I should have known I'd miss one of those directories! I'll fix that
before importing to CVS.
Comment 6 Daniel Berrange 2006-09-14 19:06:07 EDT
Ok, imported perl-Test-AutoBuild-1.2.0-3.src.rpm into CVS with the fixed
mentioned in c #4 incorporated. The plague build suceeded so I'm closing this
ticket now.

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