[Bug 170131] Review Request: php-extras - Additional PHP modules from the standard PHP distribution

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: php-extras - Additional PHP modules from the standard PHP distribution


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


matthias at rpmforge.net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |matthias at rpmforge.net




------- Additional Comments From matthias at rpmforge.net  2005-10-14 11:38 EST -------
The spec file is way too messy :
- The conditionals (_with_*/_without_*) are used in a wrong way since modules
cannot be enabled or disabled on the rpmbuild command line
- Setting the php extension dir to /var/tmp if the php-config execution to get
it fails is a bad idea (as it will lead to a really broken package)
- Same goes for the version defaulting to "Unknown"... eventually, just make the
build fail instead, as a correct php version will be mandatory
- Using %{sumary} in the %description should be avoided, since nothing in the
spec file syntax says if %{summary} references the last set one or the main
one... so if it works, it's pure luck and might break some day
- Add the sub-package headers (from %package to %description, included) don't
need to be wrapped in %if/%endif as they will simply be ignored if they don't
have a matching %files section
- The CFLAGS export seems unneeded, unless -fno-strict-aliasing is mandatory, in
which case it probably should be considered a bug
- The readline LIBS hack should probably only be set temporarily in the for loop
when the current module it the readline one in order to avoid any nasty side
effects (other modules could get linked to ncurses?)
- The %check section is ugly, probably needs cleaning up, and could at least use
some explicit comments about what is being done and why
- The per .so file %attr(755,root,root) is completely redundant with the above
"install -m755" + %defattr(-,root,root) and should be removed for a better
readability

This all needs to be improved/fixed. Please let me know if you want me to take a
stab at it.

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.