Bug 231005 - gnome-vfs2-obexftp: ObexFTP filesystem support for GNOME
gnome-vfs2-obexftp: ObexFTP filesystem support for GNOME
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Matthias Clasen
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-03-05 09:58 EST by Bastien Nocera
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-05-31 11:02:18 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mclasen: fedora‑review+
tcallawa: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Bastien Nocera 2007-03-05 09:58:39 EST
The GNOME Virtual File System provides an abstraction to common file
system operations like reading, writing and copying files, listing
directories and so on.  It is similar in spirit to the Midnight
Commander's VFS (as it uses a similar URI scheme) but it is designed
from the ground up to be extensible and to be usable from any
application.

This contains the obexftp split from the main gnome-vfs2 package.

Source RPM:
http://people.redhat.com/bnocera/gnome-vfs2-obexftp/gnome-vfs2-obexftp-0.2-1.src.rpm
Spec file:
http://people.redhat.com/bnocera/gnome-vfs2-obexftp/gnome-vfs2-obexftp.spec
Comment 1 Bastien Nocera 2007-03-05 10:35:06 EST
Note that:
hcid needs to be started with "-x"
and to obex:/// will only show paired devices. You can open a specific device with:
nautilus obex://[00:00:00:00:00]/
Comment 2 Ville Skyttä 2007-03-06 16:13:15 EST
Initial comments from just skimming the srpm, possibly a full review later
unless someone beats me to it:

- --with-compile-warnings=no to configure seems to be a no-op

- --disable-dependency-tracking to configure would result in cleaner build
output and possibly slightly faster build.

- License file missing from %doc - is the package GPL or LGPL?  Both files are
included in the upstream tarball, but it seems to me that this is LGPL and thus
COPYING.LIB should only be included.  Upstream could be interested in clarifying
this in future releases.

- Cosmetics: both %buildroot and $RPM_BUILD_ROOT used, should stick with one

- Doesn't parallel build (make %{?_smp_mflags}) work?

- %makeinstall used - doesn't 'make install DESTDIR=$RPM_BUILD_ROOT' work?

- If hcid needs to be started with -x using the bluetooth init script, a bug
should be opened against bluez-utils asking to make it possible to set hcid
options in a /etc/sysconfig/* snippet, and have that bug block this one.  Or did
you mean starting hcid some other way?
Comment 3 Ville Skyttä 2007-03-06 16:38:43 EST
More about licensing:
- dbus-glib is AFL/GPL.  AFL 2.1 is not compatible with GPL, I don't know if it's
  compatible with LGPL or not.
- gnome-vfs2 is LGPL or GPL (bug 225843).
- openobex is a mixture of LGPL and GPL code, probably making it GPL (bug 226215).

Summarizing from the above, looks like GPL would be the only safe bet when
distributing this package.  See also bug 228296 (which is a more straight
forward case) for how the licensing in that case was clarified to users of that
package.
Comment 4 Bastien Nocera 2007-03-12 07:15:46 EDT
(In reply to comment #2)
> Initial comments from just skimming the srpm, possibly a full review later
> unless someone beats me to it:
> 
> - --with-compile-warnings=no to configure seems to be a no-op

configure options are also passed to subconfigures (in this case osso-gwobex).
Not adding this option results in this bug:
https://maemo.org/bugzilla/show_bug.cgi?id=1137

> - --disable-dependency-tracking to configure would result in cleaner build
> output and possibly slightly faster build.

Done.

> - License file missing from %doc - is the package GPL or LGPL?  Both files are
> included in the upstream tarball, but it seems to me that this is LGPL and thus
> COPYING.LIB should only be included.  Upstream could be interested in clarifying
> this in future releases.

I ping'ed James Henstridge about it.

> - Cosmetics: both %buildroot and $RPM_BUILD_ROOT used, should stick with one

Fixed.

> - Doesn't parallel build (make %{?_smp_mflags}) work?

Should do, fixed.

> - %makeinstall used - doesn't 'make install DESTDIR=$RPM_BUILD_ROOT' work?

Should do, fixed.

> - If hcid needs to be started with -x using the bluetooth init script, a bug
> should be opened against bluez-utils asking to make it possible to set hcid
> options in a /etc/sysconfig/* snippet, and have that bug block this one.  Or did
> you mean starting hcid some other way?

Right now, it means editing /etc/init.d/bluetooth and modifying the parameters.
The "-x" means that it enables experimental features in hcid's D-Bus support,
and probably wouldn't be supported by us. I intend on blocking this particular
package until we can get a version of gnome-vfs-obexftp that works out of the box.

Updated packages at:
http://people.redhat.com/bnocera/gnome-vfs2-obexftp/gnome-vfs2-obexftp-0.2-2.src.rpm
http://people.redhat.com/bnocera/gnome-vfs2-obexftp/gnome-vfs2-obexftp.spec
Comment 5 Bastien Nocera 2007-05-26 13:12:01 EDT
Some fixes:
- Remove --with-compile-warnings=no, and add patch to fix the warning (fixed
upstream)
- Add README.licence file, with the comments from comment 3
- Add untested patch to use the new serial service (means that we shouldn't need
to have users add "-x" to the hcid line).

Untested, but compiles. Will need to wait until bluez-utils 3.11 gets into devel.

Updated packages at:
http://people.redhat.com/bnocera/gnome-vfs2-obexftp/gnome-vfs2-obexftp-0.2-3.src.rpm
http://people.redhat.com/bnocera/gnome-vfs2-obexftp/gnome-vfs2-obexftp.spec
Comment 7 Bastien Nocera 2007-05-27 07:33:31 EDT
Don't try to use this just yet, it causes kernel oopses on all the systems I've
tried it on.
Comment 8 Bastien Nocera 2007-05-29 10:35:08 EDT
Marcel tells me that it would be a much better idea to use RFCOMM sockets
directly rather than relying on the serial service, as rfcomm sockets are more
reliable than rfcomm ttys, and this would avoid people being able to muck about
with the devices.
Comment 10 Bastien Nocera 2007-05-30 09:55:39 EDT
Updated the package, same actual code, just updated the changelog and the name
of the patch.

s/gnome-vfs2-obexftp-use-new-serial-service-2.patch/gnome-vfs2-obexftp-use-rfcomm-sockets.patch/
and added:
- List all known devices in obex:/// not just bonded ones
Comment 11 Bastien Nocera 2007-05-30 12:43:38 EDT
Should add a TODO file:
- filter to only show obex transfer enabled devices in obex:/// (but only from
cache!)
- deal nicely with adapters being removed/added, right now it hangs listings or
copies
- make delete while creating thumbnails work (concurrency--)

But it worked well enough for me to copy plenty of photos from my phone.
Comment 12 Matthias Clasen 2007-05-30 13:27:10 EDT
rpmlint output:
Package name: ok
spec name: ok
packaging guidelines: ok
license: there is a clarification of the current status in README.license,
  which is good; we should really get upstream to clarify this though.
license field: follows the discussion earlier in this bug, ok
license files included: ok
spec file language: excellent
spec file legibility: had no problems
upstream source: ok
buildable: 
complete buildrequires: 
locale handling: n/a
shared libs: n/a
relocatable: n/a
directory ownership: ok
duplicate files: ok
file permissions: 
%clean: ok
macro use: ok
package contains code: ok
large docs: n/a
%doc: ok
headers: n/a
static libs: n/a
.pc files: n/a
shared libs: n/a
-devel package: n/a
.la files: ok
desktop files: n/a
directory ownership again: still ok
%install cleaning buildroot: ok
utf8 filenames: ok


suggestions: 
- use %{?dist} 
- require perl(XML::Parser), which is the more perl-correct version of 
  that requires
- Would be nice if the %description mentioned Bluetooth. 
- Might also be nice if the description did _not_ mention Midnight
  Commander... 
- The capitalization of ObexFTP should be consistent between summary
  and description
Comment 13 Ville Skyttä 2007-05-30 13:37:16 EDT
rpmlint (upstream development version):
gnome-vfs2-obexftp.x86_64: W: conffile-without-noreplace-flag
/etc/gnome-vfs-2.0/modules/obex-module.conf
Any particular reason for not making that noreplace?

About %description: is this really split from the main gnome-vfs package?  Looks
like a separate package, ditto upstream.
Comment 14 Matthias Clasen 2007-05-30 14:45:31 EDT
my mock build failed with 

configure.ac:20: warning: macro `AM_DISABLE_STATIC' not found in library
configure.ac:21: warning: macro `AM_PROG_LIBTOOL' not found in library
configure.ac:13: error: possibly undefined macro: AC_PROG_LIBTOOL
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
autoreconf: /usr/bin/autoconf failed with exit status: 1

I guess we are missing some 

BuildRequires: libtool, automake

Comment 15 Bastien Nocera 2007-05-30 18:49:12 EDT
(In reply to comment #12)
<snip>
> license: there is a clarification of the current status in README.license,
>   which is good; we should really get upstream to clarify this though.

I'm not sure what more can be done. Each piece of code making up the package is
under a specific licence, or dual-licenced. Linking (at build-time or run-time)
against the other libraries restrict that dual-licencing. I believe that stating
that each piece of code retain their licence, and that the whole package, as a
binary, when run, is of GPL licence should be enough.

> suggestions: 
> - use %{?dist} 

Done.

> - require perl(XML::Parser), which is the more perl-correct version of 
>   that requires

Done (although a number of other packages just require perl-XML-Parser...)

> - Would be nice if the %description mentioned Bluetooth. 

Done.

> - Might also be nice if the description did _not_ mention Midnight
>   Commander... 

Yeah, this is broken. Done.

> - The capitalization of ObexFTP should be consistent between summary
>   and description

Done.

(In reply to comment #13)
> rpmlint (upstream development version):
> gnome-vfs2-obexftp.x86_64: W: conffile-without-noreplace-flag
> /etc/gnome-vfs-2.0/modules/obex-module.conf
> Any particular reason for not making that noreplace?

It's not really a config file, it's a file that advertises a particular plugin
as supporting a specific gnome-vfs scheme. Nobody ever changes this.

> About %description: is this really split from the main gnome-vfs package?  Looks
> like a separate package, ditto upstream.

It's not, fixed.

(In reply to comment #14)
> my mock build failed with 
> 
> configure.ac:20: warning: macro `AM_DISABLE_STATIC' not found in library
> configure.ac:21: warning: macro `AM_PROG_LIBTOOL' not found in library
> configure.ac:13: error: possibly undefined macro: AC_PROG_LIBTOOL
>       If this token and others are legitimate, please use m4_pattern_allow.
>       See the Autoconf documentation.
> autoreconf: /usr/bin/autoconf failed with exit status: 1
> 
> I guess we are missing some 
> 
> BuildRequires: libtool, automake

automake was already there. I added libtool, hope this is enough to fix the problem.

* Wed May 30 2007 - Bastien Nocera <bnocera@redhat.com> - 0.2-6
- Fix description, this package isn't split from gnome-vfs2
- Add the dist to the release
- Use the "perl correct" build requires for intltool's dependencies
- Add missing BRs for the autoreconf
- Add TODO file

http://people.redhat.com/bnocera/gnome-vfs2-obexftp/gnome-vfs2-obexftp-0.2-6.src.rpm
http://people.redhat.com/bnocera/gnome-vfs2-obexftp/gnome-vfs2-obexftp.spec
Comment 16 Matthias Clasen 2007-05-31 00:26:29 EDT
Packages build nicely now, all BRs are there, and rpmlint only warns about the
non-config file in /etc, which is fine, I think.

Approved.
Comment 17 Bastien Nocera 2007-05-31 04:33:56 EDT
New Package CVS Request
=======================
Package Name: gnome-vfs2-obexftp
Short Description: ObexFTP over Bluetooth support for GNOME
Owners: bnocera@redhat.com
Branches: FC-6 F-7
InitialCC: 
Comment 18 Tom "spot" Callaway 2007-05-31 10:18:34 EDT
cvs done.
Comment 19 Bastien Nocera 2007-05-31 11:02:18 EDT
Committed and built in F-7 and devel.

I'll need to build for FC6 and probably push through the updates system for F7,
but it's all there.

Thanks for the help.
Comment 20 Ville Skyttä 2007-05-31 15:36:35 EDT
(In reply to comment #15)
> > Any particular reason for not making that noreplace?
> It's not really a config file, it's a file that advertises a particular plugin
> as supporting a specific gnome-vfs scheme. Nobody ever changes this.

Ok, any reason for including it as a %config file rather than just a normal one?
 (If that's the right thing to do, please ignore rpmlint if it whines about it.)
Comment 21 Bastien Nocera 2007-06-01 10:59:20 EDT
(In reply to comment #20)
> (In reply to comment #15)
> > > Any particular reason for not making that noreplace?
> > It's not really a config file, it's a file that advertises a particular plugin
> > as supporting a specific gnome-vfs scheme. Nobody ever changes this.
> 
> Ok, any reason for including it as a %config file rather than just a normal one?
>  (If that's the right thing to do, please ignore rpmlint if it whines about it.)

Because that's what gnome-vfs2 uses:
%config %{_sysconfdir}/gnome-vfs-2.0/modules/*.conf
Comment 22 Ville Skyttä 2007-06-02 06:44:50 EDT
I don't think that's a valid reason.  If gnome-vfs2 uses it and it has a reason
for them being %config, fine.  If not, it's probably a packaging bug there as
well.  I don't think bug compatibility buys us anything.

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