Spec URL: http://www.bachelot.org/fedora/SPECS/websvn.spec SRPM URL: http://www.bachelot.org/fedora/SRPMS/websvn-2.1.0-3.fc10.src.rpm Description: WebSVN offers a view onto your subversion repositories that's been designed to reflect the Subversion methodology. You can view the log of any file or directory and see a list of all the files changed, added or deleted in any given revision. You can also view the differences between two versions of a file so as to see exactly what was changed in a particular revision.
Source1: websvn-httpd.conf Patch1: websvn-2.2.0-use_system_libs.patch What is source for patch1? Where do you get websvn-httpd.conf? in spec file you have 2.2.0-1 but srpm is for 2.1.0-3 could you please give a link to 2.2.0 srpm?
(In reply to comment #1) > Source1: websvn-httpd.conf > Patch1: websvn-2.2.0-use_system_libs.patch > > What is source for patch1? > Where do you get websvn-httpd.conf? > I wrote both of them. The patch allows to use external php-geshi and php-feedcreator. The source is the conf needed for httpd. > in spec file you have 2.2.0-1 but srpm is for 2.1.0-3 > could you please give a link to 2.2.0 srpm? Sorry, I updated the package but forgot to update the ticket... Spec URL: http://www.bachelot.org/fedora/SPECS/websvn.spec SRPM URL: http://www.bachelot.org/fedora/SRPMS/websvn-2.2.0-1.fc10.src.rpm 2.2.0 needs a bit of work because an option to move the config file is now provided and the symlink to /etc trick is not needed anymore. I'll fix that asap.
1] rpmlint is not silent on actual package 2] do you really need patch for replacing 3 lines? sed with comment in %prep will be imho better readable. 3] I think that it would be good to add webserver into requires. Because websnv wouldn't work without webserver.
If the provided config files works with httpd I think it's better to require it rather than a generic web server. On the other hand, those _not_ wanting to use httpd as web server will be disappointed... A possible solution to this (used in the mantis package) is to split the config file in a subpackage and let that require httpd.
(In reply to comment #3) > 1] rpmlint is not silent on actual package websvn.noarch: W: non-standard-uid /var/cache/websvn apache websvn needs this to be able to temporarily cache some generated files. The owner is indeed apache. websvn.noarch: W: dangling-relative-symlink /usr/share/websvn/temp ../../../var/tmp I believe this warning is bogus. /var/tmp exists on the file system. I've tried to fix this, but failed. I'd love to see it fixed though and I welcome any suggestion. > 2] do you really need patch for replacing 3 lines? sed with comment in %prep > will be imho better readable. Well, the patch is perfectly legible imo. And also, I'd rather not use sed for this kind of thing, it would be too easy to break things w/o noticing. I'd rather keep a patch and possibly rebase it if the underlying code changes, rather than silently breaking it with sed. > 3] I think that it would be good to add webserver into requires. Because websnv > wouldn't work without webserver. Right, and also the package wouldn't even install because the apache user wouldn't exist. I'll add : Requires(pre): httpd (In reply to comment #4) > If the provided config files works with httpd I think it's better to require it > rather than a generic web server. On the other hand, those _not_ wanting to use > httpd as web server will be disappointed... > > A possible solution to this (used in the mantis package) is to split the config > file in a subpackage and let that require httpd. It looks a bit too much to me to have a subpackage just for the apache conf and I'm not sure this would be enough anyway, because some files are owned by the apache user. Actually, I've never used any other webserver, and I'm not sure how, say lighttpd, works (user, conf file, etc...). I'll dig the guidelines for advices. Any pointers welcome.
Additional note: websvn 2.2.0 now uses a php pear module to do the diff between files, rather than the system command diff. I've packaged it now, I will file a review request asap. I'll fix websvn Requires: soon.
Hi Xavier, Could you share latest spec file and SRPMS for (hope so) final review?
Sorry for the delay. The package is ready for a bit, but I hadn't had time to test it yet... Here are the links anyway : http://www.bachelot.org/fedora/SPECS/websvn.spec http://www.bachelot.org/fedora/SRPMS/websvn-2.2.0-2.fc10.src.rpm
While at it, new upstream version : http://www.bachelot.org/fedora/SPECS/websvn.spec http://www.bachelot.org/fedora/SRPMS/websvn-2.2.1-1.fc10.src.rpm
Jan, did you had time to take a look ? The new version seems to work nicely.
i will do review today/tommorow, but so far it looks good, i will test if it works correctly on my system.
MUST: rpmlint must be run on every package. The output should be posted in the review. - rpmlint /home/makerpm/rpmbuild/SRPMS/websvn-2.2.1-1.fc10.src.rpm /home/makerpm/rpmbuild/RPMS/noarch/websvn-2.2.1-1.fc10.noarch.rpm SPECS/websvn.spec websvn.noarch: W: non-standard-uid /var/cache/websvn apache websvn.noarch: W: dangling-relative-symlink /usr/share/websvn/temp ../../../var/tmp 2 packages and 1 specfiles checked; 0 errors, 2 warnings. - OK, non-standard-uid caused by owning files with apache user MUST: The package must be named according to the Package Naming Guidelines - OK MUST: The spec file name must match the base package %{name} - 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. - 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. - OK MUST: The spec file must be written in American English. - OK MUST: The spec file for the package MUST be legible. - OK MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. - OK MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture. - OK, builded and working on fc10@x86 MUST: If the package does not successfully compile, build or work on an architecture, ... - OK MUST: All build dependencies must be listed in BuildRequires - OK builded ok in KOJI http://koji.fedoraproject.org/koji/taskinfo?taskID=1390448 MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. - 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 - OK, no binary package MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review. - OK 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. - OK MUST: A Fedora package must not list a file more than once in the spec file's %files listings. - OK MUST: Permissions on files must be set properly. - OK MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). - OK MUST: Each package must consistently use macros. - OK MUST: The package must contain code, or permissable content. - OK MUST: Large documentation files must go in a -doc subpackage. - OK MUST: If a package includes something as %doc, it must not affect the runtime of the application. - OK MUST: Header files must be in a -devel package. - OK, no header files MUST: Static libraries must be in a -static package. - OK, no static libraries MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). - OK 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 - OK, no .so files present MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} - OK MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built. - OK, no .la archives MUST: Packages containing GUI applications must include a %{name}.desktop file - OK, not gui application MUST: Packages must not own files or directories already owned by other packages. - OK MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). - OK MUST: All filenames in rpm packages must be valid UTF-8. - OK Conclusion: APPROVED
Thanks Jan. New Package CVS Request ======================= Package Name: websvn Short Description: Owners: xavierb Branches: F-10 F-11 EL-4 EL-5 InitialCC:
oops, sorry forgot the short description. Here's the full template again. New Package CVS Request ======================= Package Name: websvn Short Description: Online subversion repository browser Owners: xavierb Branches: F-10 F-11 EL-4 EL-5 InitialCC:
CVS done.
websvn-2.2.1-1.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/websvn-2.2.1-1.fc10
websvn-2.2.1-1.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/websvn-2.2.1-1.fc11
websvn-2.2.1-1.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
websvn-2.2.1-1.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.