[Bug 190139] New: Review Request: rapidsvn - Graphical interface for the Subversion version-control system
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
Summary: Review Request: rapidsvn - Graphical interface for the Subversion version-control system Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: bugzilla-sink@leemhuis.info ReportedBy: rpm@timj.co.uk QAContact: fedora-package-review@redhat.com
Spec URL: http://www.timj.co.uk/linux/specs/rapidsvn.spec SRPM URL: http://www.timj.co.uk/linux/srpms/rapidsvn-0.9.1-1.src.rpm Description: RapidSVN is a GUI front-end for the Subversion revision control system. It allows access to most of the features of Subversion through a user-friendly interface.
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: rapidsvn - Graphical interface for the Subversion version-control system
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|bugzilla-sink@leemhuis.info |tibbs@math.uh.edu OtherBugsDependingO|163776 |163778 nThis| |
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: rapidsvn - Graphical interface for the Subversion version-control system
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
------- Additional Comments From tibbs@math.uh.edu 2006-04-27 21:55 EST ------- Oops, it seems I took this package by mistake, but I'll go ahead and do a review.
Right off, there's a build failure on x86_64: checking for Subversion headers... found checking for Subversion libraries... not found configure: error: Subversion libraries are required. Try --with-svn-lib. error: Bad exit status from /var/tmp/rpm-tmp.54929 (%build)
RPM build errors: Bad exit status from /var/tmp/rpm-tmp.54929 (%build) Error building package from rapidsvn-0.9.1-1.src.rpm, See build log ending
However, the build succeeds on i386. My assumption is that it isn't checking /usr/lib64 for the subversion libraries. You can probably pass --with-svn-lib="/usr/lib /usr/lib64" to build everywhere.
Issues: You include the COPYING file, but it contains only the string "Fill me in with licensing information". The actual license is more complex than just GPL, it seems. LICENSE.txt contains all of the details, so it should probably be packaged. I think "GPL and LGPL" would be a more appropriate license tag.
Is autoconf really required? I didn't see it being called in the build log, and removing it doesn't seem to hurt anything.
rpmlint complains: E: rapidsvn library-without-ldconfig-postin /usr/lib/libsvncpp.so.0.0.0 E: rapidsvn library-without-ldconfig-postun /usr/lib/libsvncpp.so.0.0.0
You need to call ldconfig in %post and %postun: see http://fedoraproject.org/wiki/ScriptletSnippets (the "Shared Libraries" section).
W: rapidsvn devel-file-in-non-devel-package /usr/lib/libsvncpp.so
The guidelines indicate that if you have a versioned .so, the unversioned one must go in a -devel package.
It seems there's a bunch of headers included in /usr/include; those should probably go in a -devel package as well. (Well, they're .hpp files, which I haven't seen before, but they look like C++ class definitions.)
There does seem to be some kind of test suite (in src/tests/svncpp), but I'm not sure how you would go about calling it. This isn't a blocker, but you might want to take a look.
Review: * package meets naming and packaging guidelines. * specfile is properly named, is cleanly written and uses macros consistently. X license field matches the actual license. X license is open source-compatible and included upstream but is not included in the package. * source files match upstream: ba03034db35912c7b51b146cc7e6090e rapidsvn-0.9.1.tar.gz ba03034db35912c7b51b146cc7e6090e rapidsvn-0.9.1.tar.gz-srpm ? BuildRequires are proper (not sure about autoconf) X package fails to build in mock (development, x86_64). X rpmlint is silent. O final provides and requires are sane. X shared libraries are present; ldconfig should be called but isn't. * package is not relocatable. * owns the directory it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * %clean is present. O %check not present; there seems to be an upstream test suite, but it's not immediately obvious how it would be called. * code, not content. * documentation is small, so no -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package. X headers present in main package. * no pkgconfig files. * no libtool .la droppings. * a GUI app; desktop file properly installed with desktop-file-install.
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: rapidsvn - Graphical interface for the Subversion version-control system
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
------- Additional Comments From rpm@timj.co.uk 2006-04-29 06:42 EST ------- Thanks very much indeed for your comments Jason. You've raised a number of interesting points there. I'll take them each in turn.
1. Licensing (inclusion of license documents): fixed, though see also below about subpackages
2. Build on x86_64; I don't use this architecture but I have added --with-svn-lib=%_libdir which I think should fix it. Please give it a go.
3. ldconfig: done
4. autoconf: removed from BR; I don't think it's needed either; it's a leftover from the spec I originally cannibalised.
5. %check: the tests in src/tests/svncpp are now run
Now onto the bigger issues: looking more closely, rapidsvn bundles (as you noted) libsvncpp.so*. This is a library which is not strictly related to rapidsvn, providing as it does an abstract C++ API for Subversion. Now, in an ideal world we would have svncpp as a totally separate package, but at the moment it is not distributed as such upstream; only as a bundled part of rapidsvn. However, it's quite forseeable that that might change (if, for example, other apps started linking to svncpp then it would quickly become annoying having it bundled with rapidsvn). So, we should package it reflecting the fact that it is functionally independent of rapidsvn. Therefore, my updated spec builds three separate packages:
- svncpp (dynamic, versioned libsvncpp .sos) - svncpp-devel (headers and unversioned .so) - rapidsvn (GUI app linked to svncpp)
This also neatly solves the issue of what to put in the License field and what license docs to include; svncpp and svncpp-devel are LGPL and marked as such and rapidsvn is GPL.
An open question which I would appreciate advice on (PackageNamingGuidelines has none) is whether the svncpp package should be called "svncpp" or "libsvncpp". It's referred to as "svncpp" upstream but many other similar libraries are packaged named as "libXXX". I don't have any strong feelings either way.
New spec/SRPM: Spec URL: http://www.timj.co.uk/linux/specs/rapidsvn.spec SRPM URL: http://www.timj.co.uk/linux/srpms/rapidsvn-0.9.1-2.src.rpm
Tested and builds in mock i386. rpmlint is now quiet for the SRPM and all three output packages.
There's a bit of an open question about what I should do with LICENSE.txt as this refers to all three packages (rapidsvn, svncpp, svncpp-devel). I could include it in all, but even then should it be patched to split off the bits relevant to each package? Again, advice welcome.
Thanks for your time.
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: rapidsvn - Graphical interface for the Subversion version-control system
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
------- Additional Comments From tibbs@math.uh.edu 2006-04-29 16:49 EST ------- I have no issues with the svncpp name. I guess if you considered this to be an addon package for subversion you would call it subversion-cpp, but I'm not sure it fits into that category.
I don't think you should go editing LICENSE.txt; it should be fine as is as long as it describes what is being licensed (which it does).
As far as I can tell, the only remaining issue is this new rpmlint warning, which I've not encountered before so I'll include the full explanation:
E: rapidsvn binary-or-shlib-defines-rpath /usr/bin/rapidsvn ['/usr/lib64'] The binary or shared library defines `RPATH'. Usually this is a bad thing because it hardcodes the path to search libraries and so makes it difficult to move libraries around. Most likely you will find a Makefile with a line like: gcc test.o -o test -Wl,--rpath. Also, sometimes configure scripts provide a --disable-rpath flag to avoid this.
No --disable-rpath in configure, but lots of one libtool and two g++ invocations that reference it. Unfortunately I'm not sure how to get rid of them; autoconf is completely opaque and hideous to me.
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: rapidsvn - Graphical interface for the Subversion version-control system
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
------- Additional Comments From tibbs@math.uh.edu 2006-04-29 17:50 EST ------- Every recommendation outside of patching configure given on http://fedoraproject.org/wiki/Extras/Schedule/RpathCheckBuildsys with no luck. export LIBTOOL=libtool didn't work; calling libtoolize --force didn't work (but maybe that wouldn't be enough to make it work).
I even deleted every instance of -rpath from configure and aclocal.m4 and while g++ is never called with it, it still shows up in the final executable. I give up.
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: rapidsvn - Graphical interface for the Subversion version-control system
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
------- Additional Comments From rpm@timj.co.uk 2006-04-29 18:17 EST ------- Hmm, this doesn't seem to show up on x86; rpmlint is quiet. I also know nothing about this issue other than what I've read on the page you cited, so I can't help unfortunately :(
The page you cited calls it a "minor" issue (for standard RPATHs) so I presume it doesn't necessarily stand in the way of inclusion.
Perhaps one to throw at extras-list and see if anyone else can help?
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: rapidsvn - Graphical interface for the Subversion version-control system
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
------- Additional Comments From tibbs@math.uh.edu 2006-04-29 21:52 EST ------- edhill on IRC pointed out that I had missed an -rpath call in src/svncpp/Makefile.in; removing it breaks the build in a rather odd way; the library is build but it seems some libtool thing doesn't get generated (.libs/libsvncpp.lai) and things die.
So I really am out of options. I'll ask extras-list if this can be ignored.
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: rapidsvn - Graphical interface for the Subversion version-control system
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
------- Additional Comments From tibbs@math.uh.edu 2006-05-01 05:58 EST ------- Based on input from extras-list, if you append LIBTOOL=/usr/bin/libtool to the make line, and then in %install delete the extra libsvncpp.a that crops up, everything comes out OK (FC5 and development, i386 and x86_64).
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: rapidsvn - Graphical interface for the Subversion version-control system
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
------- Additional Comments From rpm@timj.co.uk 2006-05-04 07:05 EST ------- OK, I've added the libtool fix. Thanks very much for all your investigation work on this unusual problem, Jason. Can you give the following a go?
New spec/SRPM: Spec URL: http://www.timj.co.uk/linux/specs/rapidsvn.spec SRPM URL: http://www.timj.co.uk/linux/srpms/rapidsvn-0.9.1-3.src.rpm
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: rapidsvn - Graphical interface for the Subversion version-control system
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO|163778 |163779 nThis| |
------- Additional Comments From tibbs@math.uh.edu 2006-05-04 16:27 EST ------- OK, with the libtool fix everything is fixed. Rawhide is broken at the moment so no development x86_64 builds (keep that in mind when you check in).
So, to recap the initial issues:
X license field matches the actual license. Fixed.
X license is open source-compatible and included upstream but is not included in the package. Fixed.
X package fails to build in mock (development, x86_64). Fixed (bulds properly; not your fault rawhide is busted)
X rpmlint is silent. It's been quieted.
X shared libraries are present; ldconfig should be called but isn't. ldconfig is called properly.
X headers present in main package. Headers have been moved off to a -devel subpackage.
I have no remaining objections.
APPROVED
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: rapidsvn - Graphical interface for the Subversion version-control system
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
rpm@timj.co.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
------- Additional Comments From rpm@timj.co.uk 2006-05-07 04:35 EST ------- Built successfully for FC-5 and devel (jobs #8854 and #8949)
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: rapidsvn - Graphical interface for the Subversion version-control system
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190139
mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |paul@all-the-johnsons.co.uk
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2006-10-13 19:22 EST ------- *** Bug 210722 has been marked as a duplicate of this bug. ***
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=190139
Tim Jackson rpm@timj.co.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #12 from Tim Jackson rpm@timj.co.uk 2011-11-16 14:24:56 EST --- Package Change Request ====================== Package Name: rapidsvn New Branches: el6
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=190139
--- Comment #13 from Jon Ciesla limb@jcomserv.net 2011-11-16 14:57:09 EST --- No owners listed.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=190139
Tim Jackson rpm@timj.co.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #14 from Tim Jackson rpm@timj.co.uk 2011-11-16 15:07:49 EST --- Package Change Request ====================== Package Name: rapidsvn New Branches: el6 Owners: timj
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=190139
--- Comment #15 from Jon Ciesla limb@jcomserv.net 2011-11-16 15:11:59 EST --- Git done (by process-git-requests).
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla