Spec URL: https://ktdreyer.fedorapeople.org/reviews/mod_proxy_fcgi.spec SRPM URL: https://ktdreyer.fedorapeople.org/reviews/mod_proxy_fcgi-2.4.10-1.el6.src.rpm Description: mod_proxy_fcgi is a proxy module for the Apache 2.2 HTTP server. This package is only relevant for Apache 2.2, since Apache 2.4 includes this module as part of the main web server package. Fedora Account System Username: ktdreyer EPEL-6 scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=8629104
First, very nice proposal, mod_proxy_fcgi is really a nice feature of httpd 2.4, so nice to have in EPEL-6 for httpd 2.2. Small comment about the spec # mmn from httpd-devel-2.2.15-39.el6 Requires: httpd-mmn = 20051115 Should better use %{!?_httpd_mmn: %{expand: %%global _httpd_mmn %%(cat %{_includedir}/httpd/.mmn || echo missing-httpd-devel)}} Requires: httpd-mmn = %{_httpd_mmn} And, this is probably uneeded, as %{_httpd_mmn} should be enough. Requires: httpd < 2.3
Adding Jan in CC, as he can probably comment.
First test (httpd-2.2.15-39.el6, php-fpm-5.3.3-40.el6_6, mod_proxy_fcgi-2.4.10-1.el6.) ProxyPass /proxy/ fcgi://127.0.0.1:9000/var/www/html/ http://localhost/proxy/info.php => work But, with mod_proxy_fcgi we could expect to be able to use <FilesMatch \.php$> SetHandler "proxy:fcgi://127.0.0.1:9000" </FilesMatch> But this doesn't (seems to) work. Can you please confirm ? If this package only offers ProxyPass(Match) support, I think this should be explain in the %description. Notice: I don't think the "This package is only relevant for Apache 2.2..." have sense in the %description. I also think it is a bit confusing that this module is not enabled by default after installation (as mod_proxy is enabled, should not be an issue).
Thanks Remi for the review comments. This backport was developed primarily for Ceph's RADOS Gateway software, and I don't think SetHandler was part of the design goals. I've opened a pull request to clarify this in the README. https://github.com/ceph/mod-proxy-fcgi/pull/2 The reason I hardcoded the "< 2.3" and the "= 20051115" and added a note in the %description was because I wanted it to be crystal clear to users that this package is only for httpd 2.2 / RHEL 6, and users shouldn't try to rebuild the SRPM for anything else. Maybe I was overly concerned about that. I've made the adjustments you've requested in the new version. * Mon Jan 26 2015 Ken Dreyer <kdreyer> - 2.4.10-2 - Adjustments for package review (RHBZ #1182770) - Dynamically determine %%{_httpd_mmn} instead of hardcoding - Drop explicit requirement on httpd < 2.3 - Add a note to %%description regarding the lack of SetHandler support - Update .conf file to load the module by default Exact changes in Git: https://fedorapeople.org/cgit/ktdreyer/public_git/mod_proxy_fcgi.git/commit/?h=el6&id=6bd994d0c84697ffe02e8950e9bd31337e65e771 Spec URL: https://ktdreyer.fedorapeople.org/reviews/mod_proxy_fcgi.spec SRPM URL: https://ktdreyer.fedorapeople.org/reviews/mod_proxy_fcgi-2.4.10-2.el6.src.rpm
> The reason I hardcoded the "< 2.3" ... Make sense, for the BuildRequires ;) All changes look fines. I will try to do a formal review, but as I will be mostly offline (you know... dayjob, fosdem, devconf.cz...) I won't be able before mid-february If nobobody approve it before ;)
Hi Remi, any other concerns with this package? Or can it go into EPEL 6?
Both the SPEC file and mod_proxy_fcgi backport looks OK to me. SetHandler with "proxy" won't work unless you patch it into httpd, you can't do that just with a module, so that's expected.
Created attachment 1005709 [details] review.txt Generated by fedora-review 0.5.2 (63c24cb) last change: 2014-07-14 Command line :/usr/bin/fedora-review -b 1182770 Buildroot used: epel-6-x86_64
First, sorry for the very long delay... [!]: Development (unversioned) .so files in -devel subpackage, if present. => please filter mod_proxy_fcgi.so()(64bit) [!]: Final provides and requires are sane (see attachments). => same, see in httpd.spec # Drop automatic provides for module DSOs %{?filter_setup: %filter_provides_in %{_httpd_moddir}/.*\.so$ %filter_setup } [!]: Latest version is packaged. I understand version: 2.4.10 refers to httpd version from which the sources are pulled and is defined in configure.ac. But on github there is no version released, so this should be a prerelease. (or ask for a github tag ;) I think the correct URL should be the github one (this one is used in configure.ac) And will make easier for user to find where to find sources and report bugs. And perhaps, add in %description Documentation: http://httpd.apache.org/docs/2.4/mod/mod_proxy_fcgi.html
Thanks Remi. I've made the adjustments you've requested in this new version. * Wed Mar 25 2015 Ken Dreyer <ktdreyer> - 2.4.10-3.20150325git837d5b0 - Update to the tip of latest master - Adjustments for package review (RHBZ #1182770) - Adjust Release to indicate a git snapshot - Drop automatic provides for module DSO - Use the GitHub URL as the package URL - Add link to httpd.apache.org docs in %%description Exact changes in Git: https://fedorapeople.org/cgit/ktdreyer/public_git/mod_proxy_fcgi.git/commit/?h=el6&id=8618de77b9c874b73fd6478cf660ea9e36df0220 Spec URL: https://ktdreyer.fedorapeople.org/reviews/mod_proxy_fcgi.spec SRPM URL: https://ktdreyer.fedorapeople.org/reviews/mod_proxy_fcgi-2.4.10-3.20150325git837d5b0.el6.src.rpm EPEL 6 scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=9320528
Please just set release to 0.3.%{commitdate}git%{shortcommit}%{?dist} To confirm with Guidelines for prerelease. Evcerything else looks ok.
Thanks Remi. I've set the release as suggested in this version. * Wed Mar 25 2015 Ken Dreyer <ktdreyer> - 2.4.10-0.3.20150325git837d5b0 - Change Release numbering scheme to start with a zero in order to indicate a prerelease. (RHBZ #1182770) Exact changes in Git: https://fedorapeople.org/cgit/ktdreyer/public_git/mod_proxy_fcgi.git/commit/?h=el6&id=f7a8058bc4abc5bb60271709db91ac0f9f5d5e18 Spec URL: https://ktdreyer.fedorapeople.org/reviews/mod_proxy_fcgi.spec SRPM URL: https://ktdreyer.fedorapeople.org/reviews/mod_proxy_fcgi-2.4.10-0.3.20150325git837d5b0.el6.src.rpm
[x]: Development (unversioned) .so files in -devel subpackage, if present. [x]: Final provides and requires are sane (see attachments). [x]: Latest version is packaged. Everything is OK now, thanks. === APPROVED ===
Thanks very much! New Package SCM Request ======================= Package Name: mod_proxy_fcgi Short Description: FastCGI support module for mod_proxy 2.2 Upstream URL: https://github.com/ceph/mod-proxy-fcgi Owners: ktdreyer Branches: el6
Git done (by process-git-requests).
mod_proxy_fcgi-2.4.10-0.3.20150325git837d5b0.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/mod_proxy_fcgi-2.4.10-0.3.20150325git837d5b0.el6
mod_proxy_fcgi-2.4.10-0.3.20150325git837d5b0.el6 has been pushed to the Fedora EPEL 6 testing repository.
mod_proxy_fcgi-2.4.10-1.20150415gitd45a11f.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/mod_proxy_fcgi-2.4.10-1.20150415gitd45a11f.el6
mod_proxy_fcgi-2.4.10-1.20150415gitd45a11f.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.