Hello Roozbeh, I cannot approve this package but I have some suggestions.

* Strip the math executable with:
      install -s math/texvc $RPM_BUILD_ROOT%{wikidir}/math
* go ahead and make the math portion a different package
* I noticed mediawiki-install.txt is included as a source but is not included in
the RPM after it gets built, perhaps that should go under %docs?

(In reply to comment #1)
> Hello Roozbeh, I cannot approve this package but I have some suggestions.
> 
> * Strip the math executable with:
>       install -s math/texvc $RPM_BUILD_ROOT%{wikidir}/math

Executables should not be stripped like this because it results in useless
debuginfo packages. The rpmbuild process should automatically strip out the
debug symbols and put them in the debuginfo package.

So this rpmlint message should be ignored?

W: mediawiki unstripped-binary-or-object /var/www/mediawiki/math/texvc

(I am fairly new at reviewing packages)

(In reply to comment #3)
> So this rpmlint message should be ignored?
> 
> W: mediawiki unstripped-binary-or-object /var/www/mediawiki/math/texvc

Most likely no; it should be fixed instead so that rpmbuild can strip it.  Some
cases where rpmbuild is (was) not able to strip binaries include files installed
without write permissions for the package builder, and setuid/setgid binaries
(bug 117858).

(In reply to comment #1)
> * go ahead and make the math portion a different package

Apparently it's not possible to use a single SPEC file to build a noarch package
and a platform-specific in a single run, so the main package which is actually
noarch will become platform-specific. Anyway, I did it for the sake of the exercise:

Spec Url: http://guava.farsiweb.info/~roozbeh/mediawiki.spec
SRPM Url: http://guava.farsiweb.info/~roozbeh/mediawiki-1.5.5-2.src.rpm

> * I noticed mediawiki-install.txt is included as a source but is not included in
> the RPM after it gets built, perhaps that should go under %docs?

It already goes in the %docs. It gets named "INSTALL.fedora" in the final RPM.

> W: mediawiki unstripped-binary-or-object /var/www/mediawiki/math/texvc

That is weird. rpmlint doesn't give me any such warning on my own machines, and
a proper *-debuginfo RPM is also created. Would you please attach a build log?

Created attachment 123206 [details]
Build log that resulted in an unstripped binary

I've tested this on two machines, both FC4 and both of them gave me the
un-stripped binary warning with rpmlint.

(In reply to comment #6)
> Created an attachment (id=123206) [edit]
> Build log that resulted in an unstripped binary
> 
> I've tested this on two machines, both FC4 and both of them gave me the
> un-stripped binary warning with rpmlint.

Apparently rpm-build's find-debuginfo.sh is never run during your build process.
It's very probably your rpm settings. May I suggest that you use the build
environment created using fedora-rpmdevtools's fedora-buildrpmtree in a clean
user account, or use mock?

New version:
Spec Url: http://guava.farsiweb.info/~roozbeh/mediawiki.spec
SRPM Url: http://guava.farsiweb.info/~roozbeh/mediawiki-1.5.5-3.src.rpm

%changelog
- Add PreReq for httpd, since we use the apache user
- Make mediawiki-math dependencies more specific
- Package documentation for mediawiki-math

New version:
Spec Url: http://guava.farsiweb.info/~roozbeh/mediawiki.spec
SRPM Url: http://guava.farsiweb.info/~roozbeh/mediawiki-1.5.6-1.src.rpm

%changelog
* Tue Jan 31 2006 Roozbeh Pournader <roozbeh> - 1.5.6-1
- Update to upstream 1.5.6

-%files math needs a %defattr
-%description change - "it has an wide" to "it has a wide"

New version:
Spec Url: http://guava.farsiweb.info/~roozbeh/mediawiki.spec
SRPM Url: http://guava.farsiweb.info/~roozbeh/mediawiki-1.5.6-2.src.rpm

%changelog
* Tue Jan 31 2006 Roozbeh Pournader <roozbeh> - 1.5.6-2
- Add %%defattr for -math subpackage
- Fixed typo in description

-You could condense your %files section into:

%config(noreplace) %{_sysconfdir}/httpd/conf.d/mediawiki.conf
%attr(770,root,apache) %dir %{wikidir}/config
%{wikidir}/config/index.php
%dir %{wikidir}
%{wikidir}/[^c]*[^h]

with a chmod for %{wikidir}/maintenance/*.pl in %install.

-Replace instances of "etc" with %{_sysconfdir}
-Also, including doc's in the main package wouldn't increase the package size
much.  Just don't delete doc's, install them, use %doc to mark them and change
[^c] to ^[cd]

I don't know 100% what best practice is on consolidating %files other than I was
asked to do it for Nagios (#176613), If no one else comments on it I'll ask IRC.  

Forgot something, replace PreReq with plain old Requires, soon the difference
between Requires and PreReq will be phased out:

http://fedoraproject.org/wiki/Packaging/Guidelines#tags

(In reply to comment #12)
> -You could condense your %files section into [...]

Makes real sense. Thanks a lot.

> -Replace instances of "etc" with %{_sysconfdir}

Done.

> -Also, including doc's in the main package wouldn't increase the package size
> much.  Just don't delete doc's, install them [...]

OK, but I put them under /usr/share/doc/mediawiki-x.y.z/internals, since they
are internal documents.

(In reply to comment #12)
> replace PreReq with plain old Requires, soon the difference
> between Requires and PreReq will be phased out

I somehow need to make sure httpd is installed before mediawiki, or RPM won't
recognize the user "apache". Any other way to do this? (Not fixing this for the
moment.)

Will post new version soon.

New version:
Spec Url: http://guava.farsiweb.info/~roozbeh/mediawiki.spec
SRPM Url: http://guava.farsiweb.info/~roozbeh/mediawiki-1.5.6-3.src.rpm

%changelog
- Refactor the %%files section (Mike McGrath)
- Replace "/etc" with macro
- Package docs under %%{_defaultdocdir}/internals
- Minor change in description

I believe you can use Requires(pre) instead of PreReq to achieve the same
affect.  I was looking at some that are already in Extras and some of them do
them different ways.

(In reply to comment #16)
> I believe you can use Requires(pre) instead of PreReq to achieve the same
> affect.

Yes, that would work, and the package will be forward-compatible that way.

New version:
Spec Url: http://guava.farsiweb.info/~roozbeh/mediawiki.spec
SRPM Url: http://guava.farsiweb.info/~roozbeh/mediawiki-1.5.6-4.src.rpm

%changelog
- Use Requires(pre) instead of PreReq (Mike McGrath)

-rpmlint (mediawiki):
E: mediawiki no-binary
E: mediawiki non-standard-gid /var/www/mediawiki/config apache
E: mediawiki non-standard-dir-perm /var/www/mediawiki/config 0770
     These are safe to ignore
-rpmlint (mediawiki-math): No output
-Matches naming requirements
-is %{name}.spec
-Meets packaging guidelines
-License is GPL
-Spec file is written in English and is easy to follow
-Source matches upstream
-Successfully builds on i386
-All build requires listed, no duplicates
-Locale not used
-Package ownes all directories it creates
-Contains no duplicate file listings
-Both files sections contain proper %defattr
-Cleans $RPM_BUILD_ROOT
-%macro's used consistantly
-Builds in mock

Everything looks good, I look forward to using this package.
One thing before you upload it, take my name out of the comments :-D  

APPROVED

(In reply to comment #18)
> Everything looks good, I look forward to using this package.

Thanks a lot for the review. I really appreciated it, and I believe your
comments have resulted in an easier time in maintaining it for the long term.

> One thing before you upload it, take my name out of the comments :-D  

Well, usually someone who provides a patch (some of your comments were really a
patch) is credited. I will take your name out if you don't wish to be credited,
but these were really your patches, you were really helpful, and I'd really like
to credit you for them.

Since this has now successfully been built in devel, it should be closed.  Note
also, however, bug #180177, which I just opened against mediawiki.