Bug 214830
Summary: | Review Request: Limph - PHP network host monitor | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Gwyn Ciesla <gwync> | ||||
Component: | Package Review | Assignee: | Orion Poplawski <orion> | ||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | Flags: | kevin:
fedora-cvs+
|
||||
Target Milestone: | --- | ||||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2007-01-12 12:41:21 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: | |||||||
Bug Depends On: | |||||||
Bug Blocks: | 163779 | ||||||
Attachments: |
|
Description
Gwyn Ciesla
2006-11-09 17:26:57 UTC
Initial comments: - BAD: md5sum does not match upstream. - Need full URL for source (http://prdownloads.sf.net/limph/limph-%{version}.tar.gz - Please, BuildArch: not BuildArchitectures: $ rpmlint ~/Desktop/limph-1.9.3-1.src.rpm W: limph strange-permission limph-hostagent 0755 - perhaps just change the permission with "install"? E: limph use-of-RPM_SOURCE_DIR - Instead of "$RPM_SOURCE_DIR/limph.conf", use %SOURCE1, etc. - should add to limph.conf: <Directory /usr/share/limph/> Order Deny,Allow Deny from all Allow from 127.0.0.1 </Directory> - More rpmlint: # rpmlint limph E: limph file-in-usr-marked-as-conffile /usr/share/limph/config.php E: limph file-in-usr-marked-as-conffile /usr/share/limph/input.php - move to /etc/limph. Is input.php really a config file? E: limph world-writable /usr/share/limph/tmp 0777 E: limph non-standard-dir-perm /usr/share/limph/tmp 0777 - These should only be writable by limph. # rpmlint limph-hostagent E: limph-hostagent file-in-usr-marked-as-conffile /usr/share/limph/host_agent.php - same here. Maybe put $secret into a config file in /etc/limph and include that? Spec URL: http://gryffindor.jcomserv.net/extras/limph/limph-2.spec SRPM URL: http://gryffindor.jcomserv.net/extras/limph/limph-1.9.3-2.src.rpm I addressed the source BuildArch, URL, RPM_SOURCE_DIR, limph.conf issues. I'm trying to solve the conf file issues with symlinks and it's not working, nor is my %attr for the tmp directory applying the desired ownership. What am I doing wrong? The reason the MD5 doesn't match upstream is the folder name in the source tarball is limph, not limph-1.9.3, so I untarred, renamed and retarred to get it to build. Should I re-release a 1.9.3.1 with this fixed, or is there a way around it in the SPEC? Thanks for the help, BTW. Created attachment 141960 [details]
patch to spec file
Here's a patch that resolves some issues.
To fix the tar ball issue:
%setup -q -n %{name}
Then go back to %setup -q when you fix that.
Other issues:
- still got some config files in /usr. See my earlier suggestion about maybe
having them include the password information from a common file in /etc/limph.
- tmp dir is still wrong. Do you really need your own tmp dir? Can't be in
/usr in any case.
Thanks, terribly helpful patch. I've applied it, released upstream version 1.9.4 to address the config file, tmp dir and tarball issues, updated spec accordingly, rebuilt, and posted the results: Spec URL: http://gryffindor.jcomserv.net/extras/limph/limph-3.spec SRPM URL: http://gryffindor.jcomserv.net/extras/limph/limph-1.9.4-1.src.rpm (In reply to comment #3) > Created an attachment (id=141960) [edit] > patch to spec file > > Here's a patch that resolves some issues. > > To fix the tar ball issue: > > %setup -q -n %{name} > > Then go back to %setup -q when you fix that. > > Other issues: > - still got some config files in /usr. See my earlier suggestion about maybe > having them include the password information from a common file in /etc/limph. > - tmp dir is still wrong. Do you really need your own tmp dir? Can't be in > /usr in any case. > Hang on, scratch those, I forgot to migrate one thing to a different file. Give me a few minutes. . . . (In reply to comment #4) > Thanks, terribly helpful patch. I've applied it, released upstream version > 1.9.4 to address the config file, tmp dir and tarball issues, updated spec > accordingly, rebuilt, and posted the results: > > Spec URL: http://gryffindor.jcomserv.net/extras/limph/limph-3.spec > SRPM URL: http://gryffindor.jcomserv.net/extras/limph/limph-1.9.4-1.src.rpm > > (In reply to comment #3) > > Created an attachment (id=141960) [edit] [edit] > > patch to spec file > > > > Here's a patch that resolves some issues. > > > > To fix the tar ball issue: > > > > %setup -q -n %{name} > > > > Then go back to %setup -q when you fix that. > > > > Other issues: > > - still got some config files in /usr. See my earlier suggestion about maybe > > having them include the password information from a common file in /etc/limph. > > - tmp dir is still wrong. Do you really need your own tmp dir? Can't be in > > /usr in any case. > > > > Ok, the above links are again valid. I had to re-do upstream since I forgot to migrate a chunk of host_agent.php to config.php. All better now, but I didn't bump the release number, if I need to, let me know. (In reply to comment #5) > Hang on, scratch those, I forgot to migrate one thing to a different file. Give > me a few minutes. . . . > > (In reply to comment #4) > > Thanks, terribly helpful patch. I've applied it, released upstream version > > 1.9.4 to address the config file, tmp dir and tarball issues, updated spec > > accordingly, rebuilt, and posted the results: > > > > Spec URL: http://gryffindor.jcomserv.net/extras/limph/limph-3.spec > > SRPM URL: http://gryffindor.jcomserv.net/extras/limph/limph-1.9.4-1.src.rpm > > > > (In reply to comment #3) > > > Created an attachment (id=141960) [edit] [edit] [edit] > > > patch to spec file > > > > > > Here's a patch that resolves some issues. > > > > > > To fix the tar ball issue: > > > > > > %setup -q -n %{name} > > > > > > Then go back to %setup -q when you fix that. > > > > > > Other issues: > > > - still got some config files in /usr. See my earlier suggestion about maybe > > > having them include the password information from a common file in /etc/limph. > > > - tmp dir is still wrong. Do you really need your own tmp dir? Can't be in > > > /usr in any case. > > > > > > > > > Is there anything further I need to do here, or were my most recent changes sufficient? It's been over a month since the last action on this review, mine or otherwise. I recognize that we're just coming off the holidays, but this http://fedoraproject.org/wiki/Extras/Policy/StalledReviews would seem indicate that this review is stalled. I think I've fulfilled the stated requests, and welcome correction if I have not. Sorry for the delay. On travel and then the holidays. - $ rpmlint -i limph-hostagent-1.9.4-1.fc6.noarch.rpm E: limph-hostagent executable-marked-as-config-file /etc/cron.hourly/limph-hostagent Executables must not be marked as config files because that may prevent upgrades from working correctly. If you need to be able to customize an executable, make it for example read a config file in /etc/sysconfig. - You're still creating the tmp dir: mkdir -p %{buildroot}%{limphdir}/tmp - Both limph and limph-hostagent own /usr/share/limph/host_agent.php. You need an %exclude in the limph files section. - Does limph-hostagent require limph to run? Looks like it does need config.php. Add the appropriate requires. Thanks for the quick response. I've addressed the config and tmp issues, and the exclude. Limph-hostagent does not require Limph to run, it includes it's own config.php. Is this a problem? Do they need to be in two seperate places, like /etc/limph and /etc/limph-hostagent for example? Links to the current versions: Spec URL: http://gryffindor.jcomserv.net/extras/limph/limph-4.spec > > SRPM URL: http://gryffindor.jcomserv.net/extras/limph/limph-1.9.4-2.src.rpm But both limph and limph-hostagent use and own the same config file. This is not allowed. What you could do is create limph-common to own /etc/limph/config.php and have both limph and limph-hostagent require that. I see. I've done that. Now rpmlint -i is clean for all 3. Links to the current versions: Spec URL: http://gryffindor.jcomserv.net/extras/limph/limph-5.spec SRPM URL: http://gryffindor.jcomserv.net/extras/limph/limph-1.9.4-3.src.rpm Requires should be: Requires: limph-common = %{version}-%{release} And one more problem, %limphdir needs to be owned by limph-common as well and not by limph. Ok. Does this do it for %limphdir? I think I specified the files and the directory properly. I also re-included the config.php symlink. Spec URL: http://gryffindor.jcomserv.net/extras/limph/limph-6.spec SRPM URL: http://gryffindor.jcomserv.net/extras/limph/limph-1.9.4-4.src.rpm In main %files section, should be: %{limphdir}/* so it doesn't own the directory. So like this?: Spec URL: http://gryffindor.jcomserv.net/extras/limph/limph-7.spec SRPM URL: http://gryffindor.jcomserv.net/extras/limph/limph-1.9.4-5.src.rpm That's the ticket. Good: - rpmlint checks return nothing - package meets naming guidelines - package meets packaging guidelines - license (GPL) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream - package compiles on devel (x86) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file APPROVED * %{_sysconfdir}/limph/ is not included (In reply to comment #18) > * %{_sysconfdir}/limph/ is not included Gah, thanks Michael! Spec URL: http://gryffindor.jcomserv.net/extras/limph/limph-8.spec SRPM URL: http://gryffindor.jcomserv.net/extras/limph/limph-1.9.4-6.src.rpm Done. So I'm sponsored in cvsextras (thank you!), I've got the various certs, CVS and plague set up. However: [limb@zanoni cvs]$ cvs co common Permission denied (publickey,keyboard-interactive). cvs [checkout aborted]: end of file from server (consult above messages if any) Any thoughts? I'm using limb as the username in my cvsroot env entry in .bash_profile, and not limb. I've tried both, however. I'm guessing limb is correct due to syntax (I'm a Subversion user and emarrasingly new to CVS). Has my new permission status just not propagated yet? Imported, owners.list updated, branches requested. Build in devel. Will build in FC-6/5 when branches are complete. Closing. Package Change Request ====================== Package Name: limph New Branches: EL-4 EL-5 cvs done. |