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
Removing FE-NEEDSPONSOR per bug 205029.
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.
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
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.
Opps, I should have known I'd miss one of those directories! I'll fix that before importing to CVS.
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.