[Bug 734248] Review Request: apf - Adventure PHP Framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=734248

--- Comment #7 from Reiner Rottmann <reiner@xxxxxxxxxxx> ---
First of all thank you very much for sharing all your knowledge about proper
packaging for Fedora. This really helps me getting comfortable with the Fedora
specifics.

I've updated the apf.spec file and created new rpm packages. Answers to your 
comment #6 may be found below.

Spec URL:
http://www.rottmann.it/apf/review/apf.spec

SRPM URL:
http://www.rottmann.it/apf/review/apf-1.15-2.fc17.src.rpm

RPM URL:
http://www.rottmann.it/apf/review/apf-1.15-2.fc17.noarch.rpm






MOCKBUILD:
[rottmrei@fedora SPECS]$ mock
/home/rottmrei/rpmbuild/SRPMS/apf-1.15-2.fc17.src.rpm
INFO: mock.py version 1.1.22 starting...
State Changed: init plugins
INFO: selinux enabled
State Changed: start
INFO: Start(/home/rottmrei/rpmbuild/SRPMS/apf-1.15-2.fc17.src.rpm) 
Config(fedora-17-x86_64)
State Changed: lock buildroot
State Changed: clean
INFO: chroot (/var/lib/mock/fedora-17-x86_64) unlocked and deleted
State Changed: unlock buildroot
State Changed: init
State Changed: lock buildroot
Mock Version: 1.1.22
INFO: Mock Version: 1.1.22
INFO: calling preinit hooks
INFO: enabled root cache
State Changed: unpacking root cache
INFO: enabled yum cache
State Changed: cleaning yum metadata
INFO: enabled ccache
State Changed: running yum
State Changed: unlock buildroot
INFO: Installed packages:
State Changed: setup
State Changed: build
INFO: Done(/home/rottmrei/rpmbuild/SRPMS/apf-1.15-2.fc17.src.rpm)
Config(default) 1 minutes 33 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-17-x86_64/result
State Changed: end
[rottmrei@fedora SPECS]$ fg
-bash: fg: current: no such job





RPMLINT:
[rottmrei@fedora SPECS]$ rpmlint -i
/home/rottmrei/rpmbuild/RPMS/noarch/apf-1.15-2.fc17.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[rottmrei@fedora SPECS]$ rpmlint -i
/home/rottmrei/rpmbuild/SRPMS/apf-1.15-2.fc17.src.rpm
apf.src: W: no-%build-section
The spec file does not contain a %build section.  Even if some packages don't
directly need it, section markers may be overridden in rpm's configuration to
provide additional "under the hood" functionality, such as injection of
automatic -debuginfo subpackages.  Add the section, even if empty.

1 packages and 0 specfiles checked; 0 errors, 1 warnings.





(In reply to comment #6)
> Pretty much everything in this package is installed in the wrong place.  You
I have heavily changed all the target locations according the filesystem
hierarchy standard and symlinked them.

> BuildRoot, the first line of %install, the entire %clean section and the
> %defattr line in %files are completely unnecessary in Fedora and should be
> removed.
Removed them.

> You don't need an empty %build section at all.
Removed it, however rpmlint shows this as a warning.

> Packages should not depend on the base php package.  There's a draft about
> this here: http://fedoraproject.org/wiki/PackagingDrafts/PHP which should
> make its way into the actual guideline relatively soon.  Since this does
> include an apache conf file, depending on httpd is OK.
I've changed the php dependency to mod_php

> Why does this package require a local mysql server?  Wouldn't it work with a
> remote server?
Good point. Only php-mysql is needed.

> Macro forms of basic comments like %{__rm} should not be used.  Just use
> "rm" instead; it's a whole lot less typing, and we're not going to remove rm
> from the path pretty much ever.
Got rid of them.

> You call dos2unix without having any dependency on it.
Added dos2unix under BuildRequires.

> It's pretty suboptimal to do selinux setup in scriptlets.  It would be
> better, once a proper location for the files is chosen, to get the base
> selinux package to include the proper file contexts.  The selinux folks are
> very responsive.
Good point. Will get in touch with the selinux folks. Until the proper rules
get included, I will leave the selinux config in place.

> You should use a proper systemd call to restart httpd, and include the
> proper dependencies for it, if indeed you really feel like doing that. 
> Personally I don't think that installing this package should mess with a
> running httpd, and the other webapps I checked don't do that.
Removed it. You're right. This is much safer.
>
> There's no need at all to do this:
>   %dir %{_localstatedir}/www/apf-%{version}/
>   %{_localstatedir}/www/apf-%{version}/*
> Instead, just use one line:
>   %{_localstatedir}/www/apf-%{version}/
Tried to remove them, however to follow the FHS, things got more complex.

> You shoud put the README files somewhere under docdir instead of spreading
> through all through the application directory, unless there's some specific
> reason they need to be where they are.
Done.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]