Bug 226457 - Merge Review: system-config-httpd
Merge Review: system-config-httpd
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 16:04 EST by Nobody's working on this, feel free to take it
Modified: 2010-08-23 00:57 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-08-23 00:57:31 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 16:04:54 EST
Fedora Merge Review: system-config-httpd

http://cvs.fedora.redhat.com/viewcvs/devel/system-config-httpd/
Initial Owner: pknirsch@redhat.com
Comment 1 Parag AN(पराग) 2010-07-21 01:01:23 EDT
1) rpmlint reported

system-config-httpd.src:16: W: unversioned-explicit-obsoletes apacheconf
system-config-httpd.src:17: W: unversioned-explicit-provides apacheconf
system-config-httpd.src:18: W: unversioned-explicit-obsoletes redhat-config-httpd
system-config-httpd.src:19: W: unversioned-explicit-provides redhat-config-httpd
==> These are old names now. Can these be removed? See http://fedoraproject.org/wiki/Upgrade_paths_%E2%80%94_renaming_or_splitting_packages#Do_I_need_to_Provide_my_old_package_names.3F


system-config-httpd.src: W: invalid-url Source0: system-config-httpd-1.5.2.tar.gz
==> Add comment in SPEC like
This is internally maintained project by Red Hat.

system-config-httpd.noarch: E: explicit-lib-dependency libglade2
== >ok

system-config-httpd.noarch: W: self-obsoletion apacheconf obsoletes apacheconf
system-config-httpd.noarch: W: self-obsoletion redhat-config-httpd obsoletes redhat-config-httpd
==> Remove these from SPEC

system-config-httpd.noarch: W: conffile-without-noreplace-flag /etc/alchemist/namespace/system-config-httpd/rpm.adl
system-config-httpd.noarch: E: non-readable /etc/alchemist/namespace/system-config-httpd/rpm.adl 0600L
system-config-httpd.noarch: E: non-readable /etc/alchemist/switchboard/system-config-httpd.switchboard.adl 0600L
==> If this is intended, can comments be added why noreplace flag and 600 permission needed?

system-config-httpd.noarch: E: non-executable-script /usr/share/system-config-httpd/ForgeBlackBox.py 0644L /usr/bin/python
system-config-httpd.noarch: E: non-executable-script /usr/share/system-config-httpd/ApacheControl.py 0644L /usr/bin/python
system-config-httpd.noarch: E: non-executable-script /usr/share/system-config-httpd/CacheBlackBox.py 0644L /usr/bin/python
system-config-httpd.noarch: E: non-executable-script /usr/share/system-config-httpd/FileBlackBox.py 0644L /usr/bin/python
system-config-httpd.noarch: E: non-executable-script /usr/share/system-config-httpd/PyAlchemist.py 0644L /usr/bin/python
system-config-httpd.noarch: E: non-executable-script /usr/share/system-config-httpd/URLBlackBox.py 0644L /usr/bin/python
system-config-httpd.noarch: E: non-executable-script /usr/share/system-config-httpd/CheckList.py 0644L /usr/bin/python
system-config-httpd.noarch: E: non-executable-script /usr/share/system-config-httpd/ApacheConf.py 0644L /usr/bin/python
system-config-httpd.noarch: E: non-executable-script /usr/share/system-config-httpd/Alchemist.py 0644L /usr/bin/python
system-config-httpd.noarch: E: non-readable /etc/alchemist/namespace/system-config-httpd/local.adl 0600L
system-config-httpd.noarch: E: non-executable-script /usr/share/system-config-httpd/ApacheGizmo.py 0644L /usr/bin/python
==> http://fedoraproject.org/wiki/Packaging_tricks#Remove_shebang_from_Python_libraries

system-config-httpd.noarch: W: dangerous-command-in-%pre mv
system-config-httpd.noarch: W: dangerous-command-in-%preun rm
==> Is this needed here?

2)timestamps should be preserved.Use INSTALL="install -p" when installing to
preserve timestamps.

3) I will suggest this pacakge to follow current packaging guidelines and
remove buildroot, %clean section and cleaning of build root in %install
Comment 2 Phil Knirsch 2010-07-23 12:46:54 EDT
Hi Parag.

Thanks for doing the review. Here my comments/suggestions on your initial review.

(In reply to comment #1)
> 1) rpmlint reported
> 
> system-config-httpd.src:16: W: unversioned-explicit-obsoletes apacheconf
> system-config-httpd.src:17: W: unversioned-explicit-provides apacheconf
> system-config-httpd.src:18: W: unversioned-explicit-obsoletes
> redhat-config-httpd
> system-config-httpd.src:19: W: unversioned-explicit-provides
> redhat-config-httpd
> ==> These are old names now. Can these be removed? See
> http://fedoraproject.org/wiki/Upgrade_paths_%E2%80%94_renaming_or_splitting_packages#Do_I_need_to_Provide_my_old_package_names.3F
> 

Jup, just double checked, those can all be removed. Fixed in HG.

> system-config-httpd.src: W: invalid-url Source0:
> system-config-httpd-1.5.2.tar.gz
> ==> Add comment in SPEC like
> This is internally maintained project by Red Hat.
> 

I've actually fixed this so that the URL now points to the mercurial repo and added a comment for the Source0 how you can generate your own tarball which should match the upstream one 1:1 for a given tag:

URL: http://hg.fedoraproject.org/hg/system-config-httpd/
# No release tarballs typically available. To recreate your own tarball for a specific release follow these
# steps:
# 1) hg clone http://hg.fedoraproject.org/hg/system-config-httpd/
# 2) cd system-config-httpd
# 3) hg tags
# 4) Select the release you want to create a tarball for, in this example system-config-httpd-1_4_0-1
# 2) hg checkout system-config-httpd-1_4_0-1
# 3) ./autogen.sh
# 4) make dist
# Afterwards you should have a tarball called system-config-http-1.4.0.tar.gz in the working directory
Source0: system-config-httpd-%{version}.tar.gz

Sounds good?

> system-config-httpd.noarch: E: explicit-lib-dependency libglade2
> == >ok
> 
> system-config-httpd.noarch: W: self-obsoletion apacheconf obsoletes apacheconf
> system-config-httpd.noarch: W: self-obsoletion redhat-config-httpd obsoletes
> redhat-config-httpd
> ==> Remove these from SPEC
>

Fixed with the above removal of the apacheconf and redhat-config-httpd Provides/Obsoletes already.

> system-config-httpd.noarch: W: conffile-without-noreplace-flag
> /etc/alchemist/namespace/system-config-httpd/rpm.adl
> system-config-httpd.noarch: E: non-readable
> /etc/alchemist/namespace/system-config-httpd/rpm.adl 0600L
> system-config-httpd.noarch: E: non-readable
> /etc/alchemist/switchboard/system-config-httpd.switchboard.adl 0600L
> ==> If this is intended, can comments be added why noreplace flag and 600
> permission needed?
> 

Well, the problem is that both rpm.adl and system-config-httpd.switchboard.adl should probably never be modified by admins as they provide the basic config of the tool (which might change for never versions).

We can either make those "normal" files instead of config files (as they shouldn't be modified in 99% of the cases) or i can add a comment, whatever you prefer.

> system-config-httpd.noarch: E: non-executable-script
> /usr/share/system-config-httpd/ForgeBlackBox.py 0644L /usr/bin/python
> system-config-httpd.noarch: E: non-executable-script
> /usr/share/system-config-httpd/ApacheControl.py 0644L /usr/bin/python
> system-config-httpd.noarch: E: non-executable-script
> /usr/share/system-config-httpd/CacheBlackBox.py 0644L /usr/bin/python
> system-config-httpd.noarch: E: non-executable-script
> /usr/share/system-config-httpd/FileBlackBox.py 0644L /usr/bin/python
> system-config-httpd.noarch: E: non-executable-script
> /usr/share/system-config-httpd/PyAlchemist.py 0644L /usr/bin/python
> system-config-httpd.noarch: E: non-executable-script
> /usr/share/system-config-httpd/URLBlackBox.py 0644L /usr/bin/python
> system-config-httpd.noarch: E: non-executable-script
> /usr/share/system-config-httpd/CheckList.py 0644L /usr/bin/python
> system-config-httpd.noarch: E: non-executable-script
> /usr/share/system-config-httpd/ApacheConf.py 0644L /usr/bin/python
> system-config-httpd.noarch: E: non-executable-script
> /usr/share/system-config-httpd/Alchemist.py 0644L /usr/bin/python
> system-config-httpd.noarch: E: non-readable
> /etc/alchemist/namespace/system-config-httpd/local.adl 0600L
> system-config-httpd.noarch: E: non-executable-script
> /usr/share/system-config-httpd/ApacheGizmo.py 0644L /usr/bin/python
> ==>
> http://fedoraproject.org/wiki/Packaging_tricks#Remove_shebang_from_Python_libraries
> 

Fixed.

> system-config-httpd.noarch: W: dangerous-command-in-%pre mv
> system-config-httpd.noarch: W: dangerous-command-in-%preun rm
> ==> Is this needed here?
>

The mv is definitely not needed anymore, so removed that.

The rm commands i use to clean up leftover stuff in /var/cache and the pyc and md5 check file i use. I can probably drop the pyc rm, but i'm not aware of how else to fix the other leftover files other than removing them manually in one of the uninstall sections of rpm. If there is i'll gladly use that.

> 2)timestamps should be preserved.Use INSTALL="install -p" when installing to
> preserve timestamps.

Added that to the make install part, looks like it's working (tested it in a trial build locally here).

> 3) I will suggest this pacakge to follow current packaging guidelines and
> remove buildroot, %clean section and cleaning of build root in %install    

Alright, dropped it from the %install section and using %{buildroot} now instead everywhere.

Thanks & regards, Phil
Comment 3 Parag AN(पराग) 2010-08-03 02:47:55 EDT
sorry for later reply.

(In reply to comment #2)

> 
> > system-config-httpd.noarch: W: conffile-without-noreplace-flag
> > /etc/alchemist/namespace/system-config-httpd/rpm.adl
> > system-config-httpd.noarch: E: non-readable
> > /etc/alchemist/namespace/system-config-httpd/rpm.adl 0600L
> > system-config-httpd.noarch: E: non-readable
> > /etc/alchemist/switchboard/system-config-httpd.switchboard.adl 0600L
> > ==> If this is intended, can comments be added why noreplace flag and 600
> > permission needed?
> > 
> 
> Well, the problem is that both rpm.adl and system-config-httpd.switchboard.adl
> should probably never be modified by admins as they provide the basic config of
> the tool (which might change for never versions).
> 
> We can either make those "normal" files instead of config files (as they
> shouldn't be modified in 99% of the cases) or i can add a comment, whatever you
> prefer.

If there is no harm in making those files as normal files then make those files as normal files.


> 
> > system-config-httpd.noarch: W: dangerous-command-in-%pre mv
> > system-config-httpd.noarch: W: dangerous-command-in-%preun rm
> > ==> Is this needed here?
> >
> 
> The mv is definitely not needed anymore, so removed that.
> 
> The rm commands i use to clean up leftover stuff in /var/cache and the pyc and
> md5 check file i use. I can probably drop the pyc rm, but i'm not aware of how
> else to fix the other leftover files other than removing them manually in one
> of the uninstall sections of rpm. If there is i'll gladly use that.
> 

Ok you can keep rest rm commands then.

Please provide updated SRPM for further review. Once I see package is good you can commit and close this review.
Comment 4 Phil Knirsch 2010-08-04 14:05:33 EDT
Hi Parag.

Finally got around to do the changes. Update srpm can be found here:

http://pknirsch.fedorapeople.org/src/system-config-httpd-1.5.3-1.el6.src.rpm

I also fixed the URL and repo link as hg.fedoraproject.org seems to have disappeared. Additionally i switched from using libxslt-python to python-lxml for the XSLT processing for the output.

Thanks again for doing the review!

Regards, Phil
Comment 5 Parag AN(पराग) 2010-08-05 03:28:47 EDT
Review:
+ package builds on koji (f14).
koji Build =>http://koji.fedoraproject.org/koji/taskinfo?taskID=2380714
+ rpmlint output for SRPM and for RPM.
system-config-httpd.src: W: invalid-url URL: http://hg.fedoraproject.org/hg/system-config-httpd/ <urlopen error [Errno -2] Name or service not known>
system-config-httpd.src: W: no-cleaning-of-buildroot %install
system-config-httpd.src: W: invalid-url Source0: system-config-httpd-1.5.3.tar.gz
system-config-httpd.noarch: E: explicit-lib-dependency libglade2
system-config-httpd.noarch: W: incoherent-version-in-changelog 1.5.3-1 ['5:1.5.3-1.fc14', '5:1.5.3-1']
system-config-httpd.noarch: W: invalid-url URL: http://hg.fedoraproject.org/hg/system-config-httpd/ <urlopen error [Errno -2] Name or service not known>
system-config-httpd.noarch: E: non-readable /etc/alchemist/namespace/system-config-httpd/rpm.adl 0600L
system-config-httpd.noarch: W: non-conffile-in-etc /etc/alchemist/namespace/system-config-httpd/rpm.adl
system-config-httpd.noarch: E: non-readable /etc/alchemist/switchboard/system-config-httpd.switchboard.adl 0600L
system-config-httpd.noarch: W: non-conffile-in-etc /etc/alchemist/switchboard/system-config-httpd.switchboard.adl
system-config-httpd.noarch: E: non-readable /etc/alchemist/namespace/system-config-httpd/local.adl 0600L
system-config-httpd.noarch: W: dangerous-command-in-%preun rm
2 packages and 0 specfiles checked; 4 errors, 8 warnings.


Suggestions:
1) If this package is needed to be built for F-13 and above then please follow
  a) buildroot tag should be removed
  b) %clean section is not needed

2) You don't need to write BR: and R: for python >= 2.2
   See https://fedoraproject.org/wiki/Packaging/Python
   Add BR: python2-devel only

3) Can you please tell me what is usage of %{_datadir}/kontrol-panel ?

4) rpmlint messages
system-config-httpd.noarch: E: non-readable /etc/alchemist/namespace/system-config-httpd/rpm.adl 0600L
system-config-httpd.noarch: W: non-conffile-in-etc /etc/alchemist/namespace/system-config-httpd/rpm.adl
system-config-httpd.noarch: E: non-readable /etc/alchemist/switchboard/system-config-httpd.switchboard.adl 0600L
system-config-httpd.noarch: W: non-conffile-in-etc /etc/alchemist/switchboard/system-config-httpd.switchboard.adl
system-config-httpd.noarch: E: non-readable /etc/alchemist/namespace/system-config-httpd/local.adl 0600L
==> I think comment should be added in spec file if these files needed to keep 600 permissions.

5) Currently unable to create source tarball

hg clone http://hg.fedoraproject.org/hg/system-config-httpd/
=> abort: error: Name or service not known
Comment 6 Phil Knirsch 2010-08-05 08:17:30 EDT
(In reply to comment #5)
> Review:
> + package builds on koji (f14).
> koji Build =>http://koji.fedoraproject.org/koji/taskinfo?taskID=2380714
> + rpmlint output for SRPM and for RPM.
> system-config-httpd.src: W: invalid-url URL:
> http://hg.fedoraproject.org/hg/system-config-httpd/ <urlopen error [Errno -2]
> Name or service not known>

Doh, my mistake, i had a buggy line in the specfile. Fixed.

> system-config-httpd.src: W: no-cleaning-of-buildroot %install

According to http://fedoraproject.org/wiki/Packaging/Guidelines#tags this isn't necessary anymore:

  BuildRoot tag

Fedora (as of F-10) does not require the presence of the BuildRoot tag in the spec and if one is defined it will be ignored. The provided buildroot will automatically be cleaned before commands in %install are called. 

Therefore i left it like it is.

> system-config-httpd.src: W: invalid-url Source0:
> system-config-httpd-1.5.3.tar.gz

See comment above Source0 how to create the tarball.

> system-config-httpd.noarch: E: explicit-lib-dependency libglade2

Ah: pygtk2-libglade requires libglade-2.0.so.0()(64bit) which is provided by libglade2, so i've dropped the requirement as it should be properly pulled in via the pygtk2-libglade package. Fixed.

> system-config-httpd.noarch: W: incoherent-version-in-changelog 1.5.3-1
> ['5:1.5.3-1.fc14', '5:1.5.3-1']
> system-config-httpd.noarch: W: invalid-url URL:
> http://hg.fedoraproject.org/hg/system-config-httpd/ <urlopen error [Errno -2]
> Name or service not known>

See above, fixed.

> system-config-httpd.noarch: E: non-readable
> /etc/alchemist/namespace/system-config-httpd/rpm.adl 0600L
> system-config-httpd.noarch: W: non-conffile-in-etc
> /etc/alchemist/namespace/system-config-httpd/rpm.adl
> system-config-httpd.noarch: E: non-readable
> /etc/alchemist/switchboard/system-config-httpd.switchboard.adl 0600L
> system-config-httpd.noarch: W: non-conffile-in-etc
> /etc/alchemist/switchboard/system-config-httpd.switchboard.adl
> system-config-httpd.noarch: E: non-readable
> /etc/alchemist/namespace/system-config-httpd/local.adl 0600L
> system-config-httpd.noarch: W: dangerous-command-in-%preun rm
> 2 packages and 0 specfiles checked; 4 errors, 8 warnings.
> 
> 
> Suggestions:
> 1) If this package is needed to be built for F-13 and above then please follow
>   a) buildroot tag should be removed
>   b) %clean section is not needed
> 

Both removed now.

> 2) You don't need to write BR: and R: for python >= 2.2
>    See https://fedoraproject.org/wiki/Packaging/Python
>    Add BR: python2-devel only
> 

Also fixed those.

> 3) Can you please tell me what is usage of %{_datadir}/kontrol-panel ?
> 

Erh, odd. That shouldn't be there. Removed aka fixed, too.

> 4) rpmlint messages
> system-config-httpd.noarch: E: non-readable
> /etc/alchemist/namespace/system-config-httpd/rpm.adl 0600L
> system-config-httpd.noarch: W: non-conffile-in-etc
> /etc/alchemist/namespace/system-config-httpd/rpm.adl
> system-config-httpd.noarch: E: non-readable
> /etc/alchemist/switchboard/system-config-httpd.switchboard.adl 0600L
> system-config-httpd.noarch: W: non-conffile-in-etc
> /etc/alchemist/switchboard/system-config-httpd.switchboard.adl
> system-config-httpd.noarch: E: non-readable
> /etc/alchemist/namespace/system-config-httpd/local.adl 0600L
> ==> I think comment should be added in spec file if these files needed to keep
> 600 permissions.
> 

Added a comment now above the respective lines:

# This file is not supposed to be read by users, only root, so 0600 permission.

> 5) Currently unable to create source tarball
> 
> hg clone http://hg.fedoraproject.org/hg/system-config-httpd/
> => abort: error: Name or service not known    

Aye, see at the start: hg.fedoraproject.org doesn't exist anymore, it's now called hg.fedorahosted.org. I've fixed all the occurrences of the URL already.

New srpm here: http://pknirsch.fedorapeople.org/src/system-config-httpd-1.5.4-1.el6.src.rpm

Thanks & regards, Phil
Comment 7 Parag AN(पराग) 2010-08-06 05:45:51 EDT
Review:
+ Built successfully in koji for F14
=> http://koji.fedoraproject.org/koji/taskinfo?taskID=2384447
+ No tags found for 1.5.4 in upstream so unable to verify source but I think that is ok.

Oh! wait looks some problem with SPEC file. Seems what you said above as fixed is not matching to spec in given srpm.
here are those issues
1) please remove line for pyc files removal in preun in comment#2
I can see that
rpm -qf /usr/share/system-config-httpd/*.pyc
==> system-config-httpd-1.5.2-1.fc13.noarch

2)I think I still see %{_datadir}/kontrol-panel in spec. 

3) I think BR: and R: for this package should be
BuildRequires: python2-devel, libglade2-devel, gettext, intltool
Requires: pygtk2-libglade, gnome-python2-canvas, gnome-python2-gnome, httpd, usermode, python-lxml


Can we have updated package here for final review?
Comment 8 Phil Knirsch 2010-08-06 11:49:03 EDT
(In reply to comment #7)
> Review:
> + Built successfully in koji for F14
> => http://koji.fedoraproject.org/koji/taskinfo?taskID=2384447
> + No tags found for 1.5.4 in upstream so unable to verify source but I think
> that is ok.
> 
> Oh! wait looks some problem with SPEC file. Seems what you said above as fixed
> is not matching to spec in given srpm.
> here are those issues
> 1) please remove line for pyc files removal in preun in comment#2
> I can see that
> rpm -qf /usr/share/system-config-httpd/*.pyc
> ==> system-config-httpd-1.5.2-1.fc13.noarch
> 

Ah yes, just saw those as well and thats automatically handled by RPM these days. Fixed in latest version.

> 2)I think I still see %{_datadir}/kontrol-panel in spec. 
> 

Odd, i could have sworn i fixed that but it wasn't. Fixed now though.

> 3) I think BR: and R: for this package should be
> BuildRequires: python2-devel, libglade2-devel, gettext, intltool
> Requires: pygtk2-libglade, gnome-python2-canvas, gnome-python2-gnome, httpd,
> usermode, python-lxml
> 

Looks good, have exactly those in the spec file now.

> 
> Can we have updated package here for final review?    

Done: http://pknirsch.fedorapeople.org/src/system-config-httpd-1.5.5-1.el6.src.rpm

Thanks & regards, Phil
Comment 9 Phil Knirsch 2010-08-06 11:52:12 EDT
Oh, before i forget: I'm on PTO next week, so the final build for Fedora will probably have to wait till Monday 16th.

Thanks & regards, Phil
Comment 10 Parag AN(पराग) 2010-08-09 01:04:45 EDT
Thanks for the changes in updated SRPM. Package looks ok to me now.

Once I see package is built in rawhide, I will approve this review.
Comment 11 Phil Knirsch 2010-08-20 06:40:22 EDT
Hi Parag.

I've had to do revert 1 tiny thing for the final build:

%{_datadir}/kontrol-panel/* is back in as in there is the KDE desktop file, similar to the one for gnome.

Other than that build has finished successfully: 

http://koji.fedoraproject.org/koji/taskinfo?taskID=2409421

Thanks again for the great review,

Regards, Phil
Comment 12 Parag AN(पराग) 2010-08-23 00:57:16 EDT
Thanks to you also for replying quickly and cleaning this package as per current fedora packaging guidelines.

APPROVED.

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