Bug 429435 - Review Request: gnubversion - Gnome interface to Subversion
Review Request: gnubversion - Gnome interface to Subversion
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Lubomir Rintel
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-19 19:15 EST by Xavier Lamien
Modified: 2009-01-08 07:49 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-01-08 07:49:29 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lkundrak: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Xavier Lamien 2008-01-19 19:15:32 EST
Spec URL: http://laxathom.fedorapeople.org/RPMS/gnubversion/gnubversion.spec
SRPM URL: http://laxathom.fedorapeople.org/RPMS/gnubversion/gnubversion-0.5-1.fc8.src.rpm
                                                                                                                                              
Description:
GnubVersion is a GNOME interface to Subversion. It integrates with the
Nautilus file manager to allow access to (eventually) all subversion
client-side functions, without having to resort to the command line.

Provides graphical equivalents to "svn checkout", "svn update" etc.
Comment 1 Parag AN(पराग) 2008-01-19 23:12:51 EST
- 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.
Comment 2 Parag AN(पराग) 2008-01-19 23:17:50 EST
missing gtk-update-icon-cache scriptlet usage
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-7103f6c38d1b5735e8477bdd569ad73ea2c49bda
Comment 3 Xavier Lamien 2008-01-20 04:04:31 EST
from comment #1 :
The *.so are nautilus extension plugins, they are nothing to do in an -devel
package ;)

From comment #2 :
hm...that should be done, could you check the spec shipped with srpm ?
Comment 4 Parag AN(पराग) 2008-01-20 05:47:16 EST
(In reply to comment #3)
> from comment #1 :
> The *.so are nautilus extension plugins, they are nothing to do in an -devel
> package ;)

 OK.
> 
> From comment #2 :
> hm...that should be done, could you check the spec shipped with srpm ?

 Yup already checked SPEC built in SRPM from comment #1 and found no use of icon
cache scriptlet.
Comment 7 Krzysztof Kurzawski 2008-01-23 17:17:35 EST
The spec looks clearly. All package looks nice. I can't try this package,
because I use KDE.
Rpmlint warns: 
gnubversion.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 14).
Comment 8 Krzysztof Kurzawski 2008-01-27 07:24:27 EST
So now I use gnome and I checked this package. The package built fine, but
didn't work:

gvn-checkout: error while loading shared libraries: libgnubversion.so.0: cannot
open shared object file: No such file or directory

I moved library from %_libdir/gnubversion to %_libdir and now is ok.

In other way you should remove libglade2-devel gnome-vfs2-devel and apr-devel
because there are unnecessary.
Comment 9 Xavier Lamien 2008-01-27 12:02:15 EST
> gvn-checkout: error while loading shared libraries: libgnubversion.so.0: cannot
> open shared object file: No such file or directory

Hm...you no need to move them to _libdir.
once this package is installed, you have to restart your X environment to update
nautilus. perhaps your ldconfig needs to be update as well.
Currently works fine on mine without move any lib to _lidir. I'll chroot another
one to check closer this issue.

Removed redundnat BR, updated release will follow.
Comment 10 Lubomir Rintel 2008-07-23 21:25:51 EDT
Ping?
Comment 12 Lubomir Rintel 2008-07-28 11:16:11 EDT
Thanks for the package, Xavier.

Three things, all of them fairly trivial (and optional -- you may want to choose
to address them before commiting), so they won't block the approval. I'll
approve this once this builds in mock for me and inc case no more issues arise.

1.) Vendor tag

desktop-file-install                                    \
        --vendor ""                                     \


You probably want to set Vendor to "fedora" as per [1] (given this is a new
package it won't break the updates). 

[1] http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files

2.) Encoding key

#Fix .desktop file
echo "Encoding=UTF-8" >>
$RPM_BUILD_ROOT%{_datadir}/applications/gvn-checkout.desktop

Actually the comment here is misleading. The Encoding key is in fact deprecated
[2]. This seems useless -- older desktop-file-utils implementations just issue a
warning here, not breaking the build (unless I am mistaken...).

[2]
http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html

3.) A typo in changelog

* Sun Jul 27 2008 Xavier Lamien <lxtnow[at]gmail.com - 0.5-3

You may want a ">" character after your e-mail address.
Comment 13 Lubomir Rintel 2008-07-28 11:37:10 EDT
Does not build in el5.
Trying fc10.
Comment 14 Lubomir Rintel 2008-07-28 12:53:19 EDT
Built fine.
Rpmlint is silent.
SPEC file is clean and legible.
Filelist is sane.
Provides and requires are ok.
Proper compiler flags are used.

APPROVED
Comment 15 Xavier Lamien 2008-07-29 04:03:54 EDT
Thanks,

New Package Request
=====================
Name:               gnubversion
Short Description:  Gnome interface to Subversion
Owners:             laxathom
Branch:             F-8 F-9
InitialCC:
Comment 16 Kevin Fenzi 2008-07-29 11:39:15 EDT
cvs done.
Comment 17 Erik van Pienbroek 2008-12-29 06:15:14 EST
This package doesn't work in Rawhide (possibly also other Fedora versions).

$ ldd /usr/lib64/nautilus/extensions-1.0/libnautilus-gnubversion.so
	linux-vdso.so.1 =>  (0x00007fff861fe000)
	libgnubversion.so.0 => not found
	libc.so.6 => /lib64/libc.so.6 (0x0000000000315000)
	/lib64/ld-linux-x86-64.so.2 (0x000000363ce00000)

$ locate libgnubversion.so.0
/usr/lib64/gnubversion/libgnubversion.so.0
/usr/lib64/gnubversion/libgnubversion.so.0.0.0

I think this is caused by the rpath hack which is performed in the .spec file. Without using the rpath, the dynamic linker doesn't know where to find the library libgnubversion.so.0 (as it is in a non-standard path). To work around this, users have to add '/usr/lib64/gnubversion' to /etc/ld.so.conf and re-run /sbin/ldconfig, but such a thing shouldn't be necessary for regular users.
Comment 18 Lubomir Rintel 2008-12-29 07:23:05 EST
(In reply to comment #17)
> /usr/lib64/gnubversion/libgnubversion.so.0
> /usr/lib64/gnubversion/libgnubversion.so.0.0.0

Whoops, this is something I probably overlooked.

(In reply to comment #9)
> > gvn-checkout: error while loading shared libraries: libgnubversion.so.0: cannot
> > open shared object file: No such file or directory
> 
> Hm...you no need to move them to _libdir.
> once this package is installed, you have to restart your X environment to
> update
> nautilus. perhaps your ldconfig needs to be update as well.
> Currently works fine on mine without move any lib to _lidir. I'll chroot
> another
> one to check closer this issue.

You're wrong here. The libgnubversion.so.0 really needs to go to _libdir, unlike 
libnautilus-gnubversion.so, which should stay in nautilus extension directory.

Please move it. Thanks!
Comment 19 Mamoru TASAKA 2008-12-29 10:21:31 EST
(In reply to comment #18)
> (In reply to comment #9)
> > > gvn-checkout: error while loading shared libraries: libgnubversion.so.0: cannot
> > > open shared object file: No such file or directory
> > 
> > Hm...you no need to move them to _libdir.
> > once this package is installed, you have to restart your X environment to
> > update
> > nautilus. perhaps your ldconfig needs to be update as well.
> > Currently works fine on mine without move any lib to _lidir. I'll chroot
> > another
> > one to check closer this issue.
> 
> You're wrong here. The libgnubversion.so.0 really needs to go to _libdir,
> unlike 
> libnautilus-gnubversion.so, which should stay in nautilus extension directory.
> 
> Please move it. Thanks!

Well, no. It can be guessed that the fact that this software
(or the upstream) tries to install libgnubversion.so.0 under %_libdir/%name
means that this software should work (at least for the upstream)
without moving the library into %_libdir (well, there are not a few
cases in which upstream make mistakes, however this is not
the case).

The culprit is the following lines.
----------------------------------------------------------
#Remove Rpath
sed -i 's|^hardcode_libdir_flag_spec="\\${wl}--rpath \\${wl}\\$libdir"|hardcode_libdir_flag_spec=""|g' libtool
sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool
-----------------------------------------------------------
Well, I see these lines in many spec files, however this method
removes _all_ rpaths, which sometimes makes the software
unusable like this case and bug 432468. 

The unneeded rpath _only_ should be removed 
(on x86_64 it is /usr/lib64, on i386 there is nothing on this package) and %_libdir/%name rpath should be preserved, and libgnubversion.so.0 
should not be moved to %_libdir. Refer to anjuta spec file about
how to deal with this for example.
Comment 20 Lubomir Rintel 2008-12-29 11:50:40 EST
(In reply to comment #19)
> > You're wrong here. The libgnubversion.so.0 really needs to go to _libdir,
> > unlike 
> > libnautilus-gnubversion.so, which should stay in nautilus extension directory.
> > 
> > Please move it. Thanks!
> 
> Well, no. It can be guessed that the fact that this software
> (or the upstream) tries to install libgnubversion.so.0 under %_libdir/%name
> means that this software should work (at least for the upstream)
> without moving the library into %_libdir (well, there are not a few
> cases in which upstream make mistakes, however this is not
> the case).
> 
> The culprit is the following lines.
> ----------------------------------------------------------
> #Remove Rpath
> sed -i 's|^hardcode_libdir_flag_spec="\\${wl}--rpath
> \\${wl}\\$libdir"|hardcode_libdir_flag_spec=""|g' libtool
> sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool
> -----------------------------------------------------------
> Well, I see these lines in many spec files, however this method
> removes _all_ rpaths, which sometimes makes the software
> unusable like this case and bug 432468. 
> 
> The unneeded rpath _only_ should be removed 
> (on x86_64 it is /usr/lib64, on i386 there is nothing on this package) and
> %_libdir/%name rpath should be preserved, and libgnubversion.so.0 
> should not be moved to %_libdir. Refer to anjuta spec file about
> how to deal with this for example.

I still tend to disagree. There's really no good reason for that library not being in libdir. Well, upstream does that, but that's a pretty bad excuse for using RPATH, though a good reason for a bug report upstream.
Comment 21 Mamoru TASAKA 2008-12-29 15:32:36 EST
(In reply to comment #20)
> I still tend to disagree. There's really no good reason for that library not
> being in libdir. Well, upstream does that, but that's a pretty bad excuse for
> using RPATH, though a good reason for a bug report upstream.

There is no need to install libraries only used by the software
to system-wide library directory. Such files should be installed under
its software specific directory (usually %_libdir/%name) to avoid
name space pollution as much as possible.

If upstream intends to make the library also used by other applications
(with providing header files for API, for example),
such file must be installed to system wide library directory. Otherwise
it is better to hide files into software specific directories if possible.
Comment 22 Lubomir Rintel 2009-01-08 07:49:29 EST
The issue discussed above was fixed by moving the library, given a consensus was reached by developers on #fedora-devel which were presented this bug, that made me confident using libdir is a more correct solution to the problem than rpath.

Note You need to log in before you can comment on or make changes to this bug.