Bug 452636
Summary: | Review Request: mod_proxy_html - Module to rewrite content as it passes through an apache proxy. | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Philip Prindeville <philipp> | ||||||||||||||||||||||||||||||||||||||||
Component: | Package Review | Assignee: | Jochen Schmitt <jochen> | ||||||||||||||||||||||||||||||||||||||||
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||||||||||||||||||||||||||||||||||||
Severity: | medium | Docs Contact: | |||||||||||||||||||||||||||||||||||||||||
Priority: | low | ||||||||||||||||||||||||||||||||||||||||||
Version: | rawhide | CC: | erik.labianca, fedora-package-review, gwync, igeorgex, itamar, jochen, jorton, notting, philipp, rayvd | ||||||||||||||||||||||||||||||||||||||||
Target Milestone: | --- | Flags: | jochen:
fedora-review+
kevin: fedora-cvs+ |
||||||||||||||||||||||||||||||||||||||||
Target Release: | --- | ||||||||||||||||||||||||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||||||||||||||||||||||||
OS: | Linux | ||||||||||||||||||||||||||||||||||||||||||
URL: | http://www.djhsolutions.net/pprindeville/mod_proxy_html-3.1.2-1.fc11.src.rpm | ||||||||||||||||||||||||||||||||||||||||||
Whiteboard: | |||||||||||||||||||||||||||||||||||||||||||
Fixed In Version: | 3.1.2-6.fc11 | Doc Type: | Bug Fix | ||||||||||||||||||||||||||||||||||||||||
Doc Text: | Story Points: | --- | |||||||||||||||||||||||||||||||||||||||||
Clone Of: | Environment: | ||||||||||||||||||||||||||||||||||||||||||
Last Closed: | 2009-12-18 04:24:31 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: | |||||||||||||||||||||||||||||||||||||||||||
Attachments: |
|
Description
Philip Prindeville
2008-06-24 07:41:05 UTC
Looks like I missed this and submitted one of my own a few days after you: Bug #453231 Yours was first so I am happy to defer, but take a glance at mine and let me know what you want to do. If you want, you can even use my spec and RPM as I think it's a little closer to being "Fedora'ized" than yours is. :-) If you folks want to work together to produce a package for review, that's great and I'll try to help. But I agree that Philip's spec needs more work than Ray's. Things like Fedora not using Vendor or Packager, the package name not matching what's used in Fedora (try "yum list mod_\*") and, honestly, "Excludeos: windows"? I have no problem merging them. That's fine. Some observations: * we don't need -Wl,"-lxml2" in the invocation of apxs. We can just use -lxml2 directly; * similarly, it would be preferable to install via "apxs -i -S LIBEXECDIR=$RPM_BUILD_ROOT/%{modulesdir} -n %{modname} %{modname}.la" as I've done; the .libs directory is an implementation detail of apxs using libtool that we shouldn't rely on; Do we really need to supply the path to apxs? It should be in the default search path. If it's not, it might be because someone wants to try out a new version of it (in which case we should use that anyway). Other than that, I'm fine with the rest of the changes. (In reply to comment #3) > I have no problem merging them. That's fine. > > Some observations: > > * we don't need -Wl,"-lxml2" in the invocation of apxs. We can just use -lxml2 > directly; I'm definitely not an expert on apxs, does -lxml2 get added automatically somewhere later in the process? I originally did not use it and resulted in a .so that wasn't linked against libxml2.so. Had to use LoadFile in Apache to get things working. > * similarly, it would be preferable to install via "apxs -i -S > LIBEXECDIR=$RPM_BUILD_ROOT/%{modulesdir} -n %{modname} %{modname}.la" as I've > done; the .libs directory is an implementation detail of apxs using libtool > that we shouldn't rely on; That makes sense to me! > Do we really need to supply the path to apxs? It should be in the default > search path. If it's not, it might be because someone wants to try out a new > version of it (in which case we should use that anyway). I don't think we do, I just typically prefer to be more specific than less. > Other than that, I'm fine with the rest of the changes. > I'll merge in your suggestions to my spec file. I don't have a burden to be the maintainer particularly if you are interested, just let me know. Alright, I see what you mean about passing -lxml2 directly. A much more obvious solution. :-) See my bug (#453231) for the latest spec and srpm. Do you want to close this one out as a duplicate of mine or vice versa (the request would need to be renamed to conform with other Fedora Apache modules)? (In reply to comment #4) > (In reply to comment #3) > > I have no problem merging them. That's fine. > > > > Some observations: > > > > * we don't need -Wl,"-lxml2" in the invocation of apxs. We can just use - lxml2 > > directly; > I'm definitely not an expert on apxs, does -lxml2 get added automatically > somewhere later in the process? I originally did not use it and resulted in a > .so that wasn't linked against libxml2.so. Had to use LoadFile in Apache to get > things working. I was just going by the example config file, which says that you need the LoadFile anyway... So the fact that the module wasn't linked (statically) against libxml2.a was a good thing. Plus it means that you can update versions of libxml2.so independently, which you lose if the module links statically against libxml2.a instead. > > * similarly, it would be preferable to install via "apxs -i -S > > LIBEXECDIR=$RPM_BUILD_ROOT/%{modulesdir} -n %{modname} %{modname}.la" as I've > > done; the .libs directory is an implementation detail of apxs using libtool > > that we shouldn't rely on; > That makes sense to me! > > Do we really need to supply the path to apxs? It should be in the default > > search path. If it's not, it might be because someone wants to try out a new > > version of it (in which case we should use that anyway). > I don't think we do, I just typically prefer to be more specific than less. Well, for the person developing a new version of apxs, they might appreciate being able to test their changes against all the Apache modules without having to modify each and every one of the .spec files to do so. > > Other than that, I'm fine with the rest of the changes. > > > I'll merge in your suggestions to my spec file. I don't have a burden to be the > maintainer particularly if you are interested, just let me know. Either way is fine with me. (In reply to comment #6) > (In reply to comment #4) > > I'm definitely not an expert on apxs, does -lxml2 get added automatically > > somewhere later in the process? I originally did not use it and resulted in a > > .so that wasn't linked against libxml2.so. Had to use LoadFile in Apache to > get > > things working. > I was just going by the example config file, which says that you need the > LoadFile anyway... So the fact that the module wasn't linked (statically) > against libxml2.a was a good thing. Plus it means that you can update versions > of libxml2.so independently, which you lose if the module links statically > against libxml2.a instead. Actually, scratch that. I just did a quick test of with and without -lxml2 in the initial compile of apxs. In either case, libxml2.a is not statically linked. The only difference seems to be that if you specify -lxml2, then that library is added to the dynamic load list for the shared object... and if you don't, then it must be explicitly loaded with the LoadFile called out in the config file. Since the config file calls it out explicitly, and since it's normally loaded up manually anyway on other platforms, I'm inclined to stick with the slightly more complicated way of loading libxml2.so up explictly... just to be compatible with other distros. I'd like to get the opinion of the reviewer on that. My feeling is that linking libxml2.so against the module itself might be more Fedora-ish than using LoadFile. And as this RPM is for Fedora specifically.... Are you set up with a FAS account and all that good stuff? *** Bug 453231 has been marked as a duplicate of this bug. *** I have closed my bug as a duplicate of this one. Spec: http://rayvd.fedorapeople.org/mod_proxy_html/mod_proxy_html.spec SRPM: http://rayvd.fedorapeople.org/mod_proxy_html/mod_proxy_html-3.0.1-5.src.rpm Phil, feel free to use my spec and SRPM there. Would you be willing to add me as a co-maintainer on this once it is approved? (In reply to comment #8) > I'd like to get the opinion of the reviewer on that. My feeling is that linking > libxml2.so against the module itself might be more Fedora-ish than using > LoadFile. And as this RPM is for Fedora specifically.... > Are you set up with a FAS account and all that good stuff? No, no account yet. I'll wait for an opinion of a reviewer. Yeah, loading it might be more Fedoraish, but that's not necessarily a good thing. Sometimes just having configs that move easily from one platform to another without changes is a bigger win. (In reply to comment #10) > Phil, feel free to use my spec and SRPM there. Would you be willing to add me > as a co-maintainer on this once it is approved? Sure, that would be fine. Asked on #apache-modules, and the consensus was that having explicit control of where a shared-object library got loaded from was a good thing, especially if you might want to try out a newer version of that library, etc. I've added a "--with xml" option to build that adds it to the explicit list of dependencies for the shared-object, but this is off by default and should probably not be enabled in the distro release. Created attachment 310748 [details]
Replacement .spec file
Created attachment 310749 [details]
Replacement .src.rpm file
Created attachment 310960 [details]
Replacement .spec file
Created attachment 311550 [details]
Replacement .spec file
Cleaned up 64-bit architecture issue. Made it pass rpmlint.
Created attachment 311551 [details]
Replacement .src.rpm file
Built under FC8 on i386 processor.
Ran rpmlint on file. It generates one warning, which is actually erroneous. $ rpmlint mod_proxy_html.spec mod_proxy_html.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 45, tab: line 40) 0 packages and 1 specfiles checked; 0 errors, 1 warnings. $ Created attachment 311672 [details]
Replacement .src.rpm file
Created attachment 311673 [details]
Replacement .spec file
Fixed gcc warning about missing braces/ambiguous "else".
New rpmlint run: $ rpmlint mod_proxy_html.spec ../RPMS/i386/mod_proxy_html-3.0.1-5.fc8.i386.rpm mod_proxy_html.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 47, tab: line 42) 1 packages and 1 specfiles checked; 0 errors, 1 warnings. $ non-formal review: 1) remove the Vendor, Packager, Provides and Excludeos tags 2) remove Requires: httpd, libxml2 - only Requires necessary should be: Requires: httpd-mmn = %(cat %{_includedir}/httpd/.mmn || echo missing) 3) use an approved BuildRoot tag, see wiki packaging guidelines 4) Source: http://apache.webthing.com/mod_proxy_html/mod_proxy_html.tgz is bad - do upstream not provide versioned URLs? 5) using %{_sbindir}/apxs throughout is probably a good idea 6) License: GPL should follow the licensing conventions on the wiki http://fedoraproject.org/wiki/Licensing 7) the package should be called simply mod_proxy_html, no httpd- prefix necessary (In reply to comment #22) > non-formal review: > > 1) remove the Vendor, Packager, Provides and Excludeos tags > 2) remove Requires: httpd, libxml2 - only Requires necessary should be: > > Requires: httpd-mmn = %(cat %{_includedir}/httpd/.mmn || echo missing) > > 3) use an approved BuildRoot tag, see wiki packaging guidelines > 4) Source: http://apache.webthing.com/mod_proxy_html/mod_proxy_html.tgz > is bad - do upstream not provide versioned URLs? > 5) using %{_sbindir}/apxs throughout is probably a good idea > > 6) License: GPL should follow the licensing conventions on the wiki > http://fedoraproject.org/wiki/Licensing > > 7) the package should be called simply mod_proxy_html, no httpd- prefix > necessary The most recent version of the .rpm and .spec files are attached directly to this bug report. I'll make the remaining fixes that you suggest that haven't been made (most already have). Created attachment 314111 [details]
Replacement .src.rpm file
Created attachment 314112 [details]
Replacement .spec file
(In reply to comment #22) > non-formal review: > > 4) Source: http://apache.webthing.com/mod_proxy_html/mod_proxy_html.tgz > is bad - do upstream not provide versioned URLs? Unfortunately, they do not. > 5) using %{_sbindir}/apxs throughout is probably a good idea Ok. I thought that allowing for the potential of testing an alternate version of apxs might be a good thing. All other comments have been addressed. (In reply to comment #26) > (In reply to comment #22) > > non-formal review: > > > > 4) Source: http://apache.webthing.com/mod_proxy_html/mod_proxy_html.tgz > > is bad - do upstream not provide versioned URLs? > > Unfortunately, they do not. upstream should be educated then ;) You'll need to work around that and version the tarball manually, I think this is covered in the wiki somewhere. > > 5) using %{_sbindir}/apxs throughout is probably a good idea > > Ok. I thought that allowing for the potential of testing an alternate version > of apxs might be a good thing. I'm not sure it really makes a difference, but it reduces predictability if apxs is picked from $PATH. > All other comments have been addressed. more nit-picking^W^Wreview: 1) this stuff is unnecessary obfuscation: %define base proxy_html %define modname mod_%{base} Name: %{modname} the spec file is for building mod_proxy_html, not an abstract httpd module; so use "Name: mod_proxy_html" and hard-code proxy_html as necessary and mod_proxy_html or %{name} otherwise. 2) the with-xml options should go; the module should be linked against -lxml2 and the conf file purged of LoadFile unconditionally. The upstream method of providing a deliberately broken module is totally crazy and not something that should be supported (even as an option) in Fedora. otherwise looks fine. (In reply to comment #27) > (In reply to comment #26) > > (In reply to comment #22) > > > non-formal review: > > > > > > 4) Source: http://apache.webthing.com/mod_proxy_html/mod_proxy_html.tgz > > > is bad - do upstream not provide versioned URLs? > > > > Unfortunately, they do not. > > upstream should be educated then ;) That might be beyond the scope of this bug fix. ;-) In any case, I've tried contacting the owners but not gotten any traction with them. > You'll need to work around that and version the tarball manually, I think this > is covered in the wiki somewhere. I looked at: http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Non-Numeric_Version_in_Release but didn't find it there. Nor is it in: http://fedoraproject.org/wiki/Packaging/SourceURL Can you point me at the right guidelines? Also, rpmlint doesn't seem to have issue with Source0: not containing %{version}. Is that a shortcoming of rpmlint? > > > 5) using %{_sbindir}/apxs throughout is probably a good idea > > > > Ok. I thought that allowing for the potential of testing an alternate version > > of apxs might be a good thing. > > I'm not sure it really makes a difference, but it reduces predictability if > apxs is picked from $PATH. > > > All other comments have been addressed. > > more nit-picking^W^Wreview: > > 1) this stuff is unnecessary obfuscation: > > %define base proxy_html > %define modname mod_%{base} > > Name: %{modname} > > the spec file is for building mod_proxy_html, not an abstract httpd module; so > use "Name: mod_proxy_html" and hard-code proxy_html as necessary and > mod_proxy_html or %{name} otherwise. > > 2) the with-xml options should go; the module should be linked against -lxml2 > and the conf file purged of LoadFile unconditionally. The upstream method of > providing a deliberately broken module is totally crazy and not something that > should be supported (even as an option) in Fedora. Not sure I follow you. Why does Apache support LoadFile then? Or is it a Fedora policy to discourage using it? (It's not documented in: http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Addon_Packages_.28httpd.2C_pam.2C_and_SDL.29 so where should I be looking?) > otherwise looks fine. I think either way would end up functionally the same; however, if we link against libxml2, we then get an implicit requires against libxml2 although I suppose you could just manually add an explicit Requires as well. I'm not a technical expert on the subject at all however, so I'd be interested in hearing why LoadFile would be a better alternative than linking? (In reply to comment #29) > I think either way would end up functionally the same; however, if we link > against libxml2, we then get an implicit requires against libxml2 although I > suppose you could just manually add an explicit Requires as well. > > I'm not a technical expert on the subject at all however, so I'd be interested > in hearing why LoadFile would be a better alternative than linking? Using a loadfile is certainly not the way to go in fedora. Linking against libxml2 doesn't only add an automatic requires, this is a mere side effect of adding the soname dependency on the right libxml2 version in the binary (something along libxml2.so.2). The binary will refuse to run if the libxml2 shared object with the right version isn't found. There are ways to use another library at runtime (certainly by setting LD_LIBRARY_PATH), using LoadFile (which certainly does a dlopen) is not right. Ther are cases where using LoadFile or similar are right (like plugins for example), but here it doesn't seems to be the case at all. Created attachment 315602 [details]
Replacement .spec file
Created attachment 315603 [details]
Replacement .src.rpm file
(In reply to comment #27) > (In reply to comment #26) > > (In reply to comment #22) > > > non-formal review: > > > > > > 4) Source: http://apache.webthing.com/mod_proxy_html/mod_proxy_html.tgz > > > is bad - do upstream not provide versioned URLs? > > > > Unfortunately, they do not. > > upstream should be educated then ;) > > You'll need to work around that and version the tarball manually, I think this > is covered in the wiki somewhere. Upstream has cooperated in adding numbered versions. All other issues have been addressed. Results from rpmlint: $ rpmlint mod_proxy_html.spec ../RPMS/i386/mod_proxy_html-debuginfo-3.0.1-7.fc8.i386.rpm 1 packages and 1 specfiles checked; 0 errors, 0 warnings. $ Created attachment 315605 [details]
Replacement .spec file
Forgot to update ChangeLog.
Created attachment 315606 [details]
Replacement .src.rpm file
Forgot to edit ChangeLog.
Quick question: should the LoadModule that enables this functionality be uncommented from the config file by default by installation? Or is that something that should be enabled explicitly by the end-user uncommenting that line? Created attachment 317204 [details]
Replacement .spec file
Created attachment 317205 [details]
Replacement .src.rpm file
You lost "BuildRequires: libxml2-devel httpd-devel" somewhere along the way - if that was a result of my comments about losing the Requires, my apologies. The LoadModule for the module should be uncommented by default; i.e. the module should be loaded by default, when installed. If someone doesn't want the module loaded, they can remove the package :) Other than that, this looks great, thanks for persevering! I'm not an Official Sponsor so I can't do the formal review unfortunately. Created attachment 317207 [details]
Replacement .spec file
Put back BuildRequires: as before.
Created attachment 317209 [details]
Replacement .src.rpm file
Just as a point of interest, I downloaded the .src.rpm from comment #1 and was able to build and use it perfectly on CentOS 5. Posting to fedora-devel today to ask for a sponsor to be assigned. This fix has been good to go for 10 months now. new version 8.4.0 http://ispbrasil.com.br/pgsql/mingw32-postgresql.spec http://ispbrasil.com.br/pgsql/mingw32-postgresql-8.4.0-1.fc11.src.rpm (In reply to comment #44) oh my god! , I am sorry, posted in wrong bug, please ignore my previous post :-( my mistake Philip, I can sponsor and do the formal review. I'll start on that today. While I'm on that, can you point me to a few practice reviews you've done? Formal review: # MUST: rpmlint must be run on every package. The output should be posted in the review.[1] Clean. # MUST: The package must be named according to the Package Naming Guidelines . OK. Might want to change Source: to Source0: # MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption. [2] . OK. # MUST: The package must meet the Packaging Guidelines . OK. # MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines . OK. # MUST: The License field in the package spec file must match the actual license. [3] OK. # MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.[4] OK. # MUST: The spec file must be written in American English. [5] OK. # MUST: The spec file for the package MUST be legible. [6] OK. # MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this. OK. # MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture. [7] OK. # MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line. [8] N/A # MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense. OK. # MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.[9] OK. # MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. [10] N/A # MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker. [11] N/A # MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. [12] OK. # MUST: A Fedora package must not list a file more than once in the spec file's %files listings. [13] OK. # MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line. [14] OK. # MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [15] OK. # MUST: Each package must consistently use macros. [16] OK. # MUST: The package must contain code, or permissable content. [17] OK. # MUST: Large documentation files must go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity). [18] N/A # MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. [18] OK. # MUST: Header files must be in a -devel package. [19] N/A # MUST: Static libraries must be in a -static package. [20] N/A # MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). [21] N/A # MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. [19] Apache module, N/A # MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} [22] N/A # MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built.[20] OK. # MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation. [23] N/A # MUST: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time. [24] OK. # MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [25] OK. # MUST: All filenames in rpm packages must be valid UTF-8. [26] OK. Looks great and can be approved once I'm satisfied enough with practice reviews to sponsor. That said, it looks like you've learned a lot in this review, so I'm thinking this shouldn't take long at all. FYI, Philip, I attempted to reply to your email, but: ----- The following addresses had permanent fatal errors ----- <philipp> (reason: 554 mail.redfish-solutions.com ESMTP not accepting messages) ----- Transcript of session follows ----- ... while talking to mail.redfish-solutions.com.: <<< 554 mail.redfish-solutions.com ESMTP not accepting messages 554 5.0.0 Service unavailable (In reply to comment #48) > FYI, Philip, I attempted to reply to your email, but: > > ----- The following addresses had permanent fatal errors ----- > <philipp> > (reason: 554 mail.redfish-solutions.com ESMTP not accepting messages) > > ----- Transcript of session follows ----- > ... while talking to mail.redfish-solutions.com.: > <<< 554 mail.redfish-solutions.com ESMTP not accepting messages > 554 5.0.0 Service unavailable I added whitelisting for 76.204.165.225. OK, I have take a short look on your package and have to recorgnise, that the packaged release is obsolete, because this is an old review. So it may be nice, if you can create a package for the current release 3.1. If this package may ok, I'm willing to sponsor you. Best Regards: Jochen Schmitt Created attachment 377563 [details]
Replacement .spec file
Bump to 3.1.2.
Created attachment 377565 [details]
Replacement .src file
Good: + Could download upstream sources via spectool -g + Packaged tar ball matches with upstr (md5sum: d6497b48d2bffc4150472472004618c3) + Local build works fine + Rpmlint is silent on source package + Rpmlint is silent on binary package + Rpmlint is silent on debuginfo package + Scratch build on koji works fine *** APPROVED *** Please do the following step: 1.) Create a FAS account 2.) Fullfill the CLA 3.) Request membership in the packages group 4.) Send me your FAS-id for sponsoring. A additional advice for future reviews: Please try to get a webspace and upload the SPEC file and the source rpms to it. and post the URLs in the bug report. Of course you can take the space on fedorapeople.org which you will get after you has a valid FAS account. Best Regards Jochen Schmitt Jochen: id is "philipp". I just sent in the packagers group request. Thanks. I have approve your membership request. so you can now create a CVSAdmin request for your package. URLs: http://www.djhsolutions.net/pprindeville/mod_proxy_html-3.1.2-1.fc11.src.rpm http://www.djhsolutions.net/pprindeville/mod_proxy_html.spec *** Bug 547930 has been marked as a duplicate of this bug. *** New Package CVS Request ======================= Package Name: mod_proxy_html Short Description: reverse-proxy output filter for HTML Owners: philipp Branches: F-11 F-12 InitialCC: s4504kr limb Please branch for EL-4 and EL-5 as well (if willing). New Package CVS Request ======================= Package Name: mod_proxy_html Short Description: reverse-proxy output filter for HTML Owners: philipp Branches: F-11 F-12 EL-4 EL-5 InitialCC: s4504kr limb cvs done. mod_proxy_html-3.1.2-5.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/mod_proxy_html-3.1.2-5.fc11 mod_proxy_html-3.1.2-5.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/mod_proxy_html-3.1.2-5.fc12 mod_proxy_html-3.1.2-6.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/mod_proxy_html-3.1.2-6.fc11 mod_proxy_html-3.1.2-6.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/mod_proxy_html-3.1.2-6.fc12 mod_proxy_html-3.1.2-6.fc12 has been pushed to the Fedora 12 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update mod_proxy_html'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F12/FEDORA-2009-13279 mod_proxy_html-3.1.2-5.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report. mod_proxy_html-3.1.2-5.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report. mod_proxy_html-3.1.2-6.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report. mod_proxy_html-3.1.2-6.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report. |