Bug 491331 - (spacewalk-config) Review Request: spacewalk-config - Spacewalk Configuration
Review Request: spacewalk-config - Spacewalk Configuration
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Ruediger Landmann
Fedora Extras Quality Assurance
:
Depends On:
Blocks: F-Spacewalk spacewalk-backend space12
  Show dependency treegraph
 
Reported: 2009-03-20 09:42 EDT by Miroslav Suchý
Modified: 2010-12-06 15:01 EST (History)
6 users (show)

See Also:
Fixed In Version: spacewalk-config-1.2.7-1.fc14
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-11-18 08:19:03 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
r.landmann: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Miroslav Suchý 2009-03-20 09:42:06 EDT
SRPM: http://miroslav.suchy.cz/fedora/spacewalk-config/spacewalk-config-0.5.7-1.src.rpm
SPEC: http://miroslav.suchy.cz/fedora/spacewalk-config/spacewalk-config.spec

Description:
Spacewalk Configuration Templates

Rpmlint write out several warning, which I would like to explain
dangling-relative-symlink - that is to make apache work with spacewalk ssl certificate
non-conffile-in-etc /etc/sysconfig/rhn-satellite-prep - it is not configuration file. It is in fact template, which we want to be replaced with package upgrade. I filled BZ 491301

dangerous-command-in-%pre perl - is ok. although we can use sed, the perl command is much more readible.
Comment 1 Rafael Aquini 2010-08-19 21:18:24 EDT
PING

It's been more than a year with no progress; This review should be closed soon
if there is no response, shouldn't it?
Comment 2 Miroslav Suchý 2010-09-01 12:11:51 EDT
It is not my fault that no one pick it up. Isn't it?
Comment 3 Xavier Bachelot 2010-09-06 11:25:13 EDT
This is definitely not your fault the package has not been picked up for review, indeed. And it can be quite depressing to wait for someone to take up the job ;-) However, you might want to update the spec and SRPM to the latest upstream version. I see there was several releases since 0.5.7, the latest being 1.2.1. This might help in the quest for a good willing reviewer. From a quick glance at the review queue, it seems at least some of the others spacewalk related reviews are also in need for updated specs and SRPMs (spacewalk-backend,rhnmd), although the gap is closer for them.
Comment 5 Ruediger Landmann 2010-11-02 03:10:47 EDT
Hi Miroslav -- 

All looks good, except the non-conffile-in-etc warnings

I also note that there's a vesion 1.2.4 upstream, so if you rebuild this package to take care of the config file warnings, perhaps you can use the latest version?

Cheers
Rudi





 - = N/A
 / = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [!] Rpmlint output is clean:

      $ rpmlint SPECS/spacewalk-config.spec 
      0 packages and 1 specfiles checked; 0 errors, 0 warnings.
      $ rpmlint SRPMS/spacewalk-config-1.2.3-1.fc13.src.rpm 
      1 packages and 0 specfiles checked; 0 errors, 0 warnings.
      $ rpmlint RPMS/noarch/spacewalk-config-1.2.3-1.fc13.noarch.rpm 
      spacewalk-config.noarch: E: non-standard-dir-perm /etc/rhn 0750L
      spacewalk-config.noarch: W: dangling-relative-symlink /etc/pki/tls/private/spacewalk.key ../../../httpd/conf/ssl.key/server.key
      spacewalk-config.noarch: W: dangling-relative-symlink /etc/pki/tls/certs/spacewalk.crt ../../../httpd/conf/ssl.crt/server.crt
      spacewalk-config.noarch: W: non-conffile-in-etc /etc/sysconfig/rhn-satellite-prep/etc/rhn/cluster.ini
      spacewalk-config.noarch: W: non-conffile-in-etc /etc/sysconfig/rhn-satellite-prep/etc/rhn/rhn.conf
      spacewalk-config.noarch: E: non-readable /etc/rhn/rhn.conf 0640L
      spacewalk-config.noarch: W: dangerous-command-in-%pre perl
      1 packages and 0 specfiles checked; 2 errors, 5 warnings.

 [/] Package is named according to the Package Naming Guidelines.
 [/] Spec file name must match the base package %{name}, in the format
%{name}.spec.
 [/] Package meets the Packaging Guidelines including the Language specific
items
 [/] Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging Guidelines.
 [/] License field in the package spec file matches the actual license.
     License type: GPLv2
 [/] 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 is included in %doc.
      %doc LICENSE
 [/] Spec file is legible and written in American English.
 [/] Sources used to build the package matches the upstream source, as provided
in the spec URL.
      $ md5sum SOURCES/spacewalk-config-1.2.3.tar.gz 
      3e095682d9863c5eb1f02ccb1c7313b5  SOURCES/spacewalk-config-1.2.3.tar.gz
      $ md5sum ~/Download/spacewalk-config-1.2.3.tar.gz 
      3e095682d9863c5eb1f02ccb1c7313b5  /home/rlandmann/Download/spacewalk-config-1.2.3.tar.gz
 [/] Package successfully compiles and builds into binary rpms on at least one
supported architecture.
     Tested: http://koji.fedoraproject.org/koji/taskinfo?taskID=2570954
 [/] Package is not known to require ExcludeArch
 [/] All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly (with the %find_lang macro)
 [-] ldconfig called in %post and %postun if required.
 [/] Package does not bundle copies of system libraries
 [/] Package is not relocatable.
 [/] Package must own all directories that it creates.
 [/] Package does not contain duplicates in %files.
 [/] Permissions on files are set properly
 [/] %files section includes a %defattr(...) line
 [/] Package consistently uses macros.
 [-] Large documentation files are in a -doc subpackage, if required.
 [/] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -static subpackage, if present.
 [-] Development .so files in -devel subpackage, if present.
 [-] -devel packages require base package with full versioning.
 [/] Package does not contain any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI
application.
 [/] Package does not own files or directories owned by other packages.
 [/] Filenames are valid UTF-8

=== SUGGESTED ITEMS ===

 [/] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
 [/] Reviewer should test that the package builds in mock.
     Tested through koji
 [/] Package should compile and build into binary rpms on all supported
architectures.
     Tested on: f13
 [?] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] Subpackages other than -devel require the base package as a fully versioned
dependency
 [-] The placement of pkgconfig(.pc) files is correct (normally in -devel)
 [-] File based requires are sane.
 [-] Package contains man pages for binaries and scripts.
Comment 6 Miroslav Suchý 2010-11-03 17:10:16 EDT
> All looks good, except the non-conffile-in-etc warnings
Addressed. I moved it to /var/lib/rhn, where those templates belong.

> perhaps you can use the latest version
I can. I'm member of upstream team. We are releasing new versions quite often...

Updated SRPM:
http://miroslav.suchy.cz/fedora/spacewalk-config/spacewalk-config-1.2.5-1.el6.src.rpm
SPEC: http://miroslav.suchy.cz/fedora/spacewalk-config/spacewalk-config.spec
Comment 7 Ruediger Landmann 2010-11-03 19:17:25 EDT
Thanks Miroslav -- rpmlint is quiet now except for the (understandable) warnings about permissions, symlinks, and perl.

ACCEPT

Please go ahead and make your SCM request.
Comment 8 Christoph Wickert 2010-11-03 21:27:51 EDT
(In reply to comment #5)
>  [/] Package meets the Packaging Guidelines including the Language specific
> items

- Incorrect BuildRoot tag. For Fedora it can be omitted and for EPEL5 it should be one of http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag
- Should use %global instead of %define
- %description should end with a dot (and could be a little more elaborate).
- startup.pl and satidmap.pl should be tagged %config or not be in %{_sysconfdir}. The latter is something that be fixed upstream but not in packaging.

>  [/] License field in the package spec file matches the actual license.
>      License type: GPLv2

- Should be GPLv2+. If no version of the license is specified, then it usually means "or any later versions", see section 9 of the GPLv2.

>  [/] Package must own all directories that it creates.

But it doesn't:

%{_sysconfdir}/pki/tls/
%{_sysconfdir}/pki/tls/certs/
%{_sysconfdir}/pki/tls/private/
%{_var}/lib/cobbler/
%{_var}/lib/cobbler/kickstarts/
%{_var}/lib/cobbler/snippets/

are not owned by this package or one of it's requirements. And %{_var}/lib/ should be %{_sharedstatedir}.

>  [/] Package does not contain duplicates in %files.

It does:

warning: File listed twice: /var/lib/rhn/rhn-satellite-prep
warning: File listed twice: /var/lib/rhn/rhn-satellite-prep/etc
warning: File listed twice: /var/lib/rhn/rhn-satellite-prep/etc/rhn
warning: File listed twice: /var/lib/rhn/rhn-satellite-prep/etc/rhn/cluster.ini
warning: File listed twice: /var/lib/rhn/rhn-satellite-prep/etc/rhn/rhn.conf

%{prepdir} is a subdirectory %{_sharedstatedir}/rhn

>  [/] Permissions on files are set properly
>  [/] %files section includes a %defattr(...) line
>  [/] Package consistently uses macros.

>  [/] Package does not own files or directories owned by other packages.

Is %{_sharedstatedir}/rhn really not owned by something else?
Comment 9 Ruediger Landmann 2010-11-03 23:04:14 EDT
(In reply to comment #8)

Thanks Christopher for your corrections; I have of course set the review flag back to ? so that Miroslav can take another look at these.

Kind regards
Rudi
Comment 10 Miroslav Suchý 2010-11-04 11:12:45 EDT
>- Incorrect BuildRoot tag. For Fedora it can be omitted and for EPEL5 it should
>be one of
>http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag
>- Should use %global instead of %define
>- %description should end with a dot (and could be a little more elaborate).
all addressed

> - startup.pl and satidmap.pl should be tagged %config or not be in
>%{_sysconfdir}. The latter is something that be fixed upstream but not in
>packaging.
I moved satidmap.pl to /usr/share/rhn. No problem here.
But I have problem with startup.pl. It is perl executable. Not configuration file. This is file which mod_perl call during its start. And it always put in apache configuration. I tried to search for some mod_perl aplication in Fedora, but find none.
I hesitate to mark this one file as config and also move it to other place.

> - Should be GPLv2+. 
No. It was released as GPLv2. We intentionaly did not released it with "or later" appendix.
http://fedoraproject.org/wiki/Licensing#Good_Licenses list GPLv2 as good license.

>  [/] Package must own all directories that it creates.
Addressed.

>  [/] Package does not contain duplicates in %files
Addressed.

> Is %{_sharedstatedir}/rhn really not owned by something else?
No. On fully installed Spacewalk:
# rpm -qf /var/lib/rhn
file /var/lib/rhn is not owned by any package

Updated SRPM:
http://miroslav.suchy.cz/fedora/spacewalk-config/spacewalk-config-1.2.6-1.el6.src.rpm
SPEC: http://miroslav.suchy.cz/fedora/spacewalk-config/spacewalk-config.spec
Comment 11 Ruediger Landmann 2010-11-09 18:36:00 EST
(In reply to comment #10)

> > - startup.pl and satidmap.pl should be tagged %config or not be in
> >%{_sysconfdir}. The latter is something that be fixed upstream but not in
> >packaging.
> I moved satidmap.pl to /usr/share/rhn. No problem here.
> But I have problem with startup.pl. It is perl executable. Not configuration
> file. This is file which mod_perl call during its start. And it always put in
> apache configuration. I tried to search for some mod_perl aplication in Fedora,
> but find none.
> I hesitate to mark this one file as config and also move it to other place.

Could you place it in /usr/share/rhn too, and load it with a config file in /etc/httpd/conf.d? The /etc/httpd/conf.d/README file also notes:

"Files are processed in alphabetical order, so if using configuration
directives which depend on, say, mod_perl being loaded, ensure that
these are placed in a filename later in the sort order than "perl.conf"."
Comment 12 Miroslav Suchý 2010-11-15 02:33:46 EST
I dig up more info about usual location of startup.pl and find that it is indeed sometimes located in /usr/share. So I moved it.

Updated SRPM:
http://miroslav.suchy.cz/fedora/spacewalk-config/spacewalk-config-1.2.7-1.el6.src.rpm
SPEC: http://miroslav.suchy.cz/fedora/spacewalk-config/spacewalk-config.spec
Comment 13 Ruediger Landmann 2010-11-15 21:08:05 EST
(In reply to comment #12)
> I dig up more info about usual location of startup.pl and find that it is
> indeed sometimes located in /usr/share. So I moved it.

Thanks Miroslav; I think this clears up the remaining issues. Thanks again also to Christoph for the catch earlier.

ACCEPT -- please go ahead and make your SCM request.
Comment 14 Miroslav Suchý 2010-11-16 05:57:28 EST
New Package CVS Request
=======================
Package Name: spacewalk-config
Short Description: Spacewalk Configuration
Owners: msuchy
Branches: F-13, F-14, EL-5, EL-6
InitialCC:
Comment 15 Jason Tibbitts 2010-11-16 08:04:54 EST
Git done (by process-git-requests).
Comment 16 Fedora Update System 2010-11-18 08:24:39 EST
spacewalk-config-1.2.7-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/spacewalk-config-1.2.7-1.fc14
Comment 17 Fedora Update System 2010-11-18 08:26:22 EST
spacewalk-config-1.2.7-1.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/spacewalk-config-1.2.7-1.fc13
Comment 18 Fedora Update System 2010-12-06 14:57:20 EST
spacewalk-config-1.2.7-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 19 Fedora Update System 2010-12-06 15:00:56 EST
spacewalk-config-1.2.7-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

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