Bug 226457
| Summary: | Merge Review: system-config-httpd | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Nobody's working on this, feel free to take it <nobody> |
| Component: | Package Review | Assignee: | Parag AN(पराग) <panemade> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | panemade, pknirsch |
| Target Milestone: | --- | Flags: | panemade:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2010-08-23 04:57:31 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Nobody's working on this, feel free to take it
2007-01-31 21:04:54 UTC
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 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 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. 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 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 (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 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? (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 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 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. 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
Thanks to you also for replying quickly and cleaning this package as per current fedora packaging guidelines. APPROVED. |