Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 223618 - Review Request: obexftp - Tool to access devices via the OBEX protocol
Summary: Review Request: obexftp - Tool to access devices via the OBEX protocol
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tom "spot" Callaway
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2007-01-20 21:05 UTC by Dominik 'Rathann' Mierzejewski
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-01-26 02:33:35 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Dominik 'Rathann' Mierzejewski 2007-01-20 21:05:32 UTC
Spec URL: http://rpm.greysector.net/extras/obexftp.spec
SRPM URL: http://rpm.greysector.net/extras/obexftp-0.20-1.src.rpm
Description:
The overall goal of this project is to make mobile devices featuring the OBEX
protocol and adhering to the OBEX FTP standard accessible by an open source
implementation. The common usage for ObexFTP is to access your mobile phones
memory to store and retrieve e.g. your phonebook, logos, ringtones, music,
pictures and alike.

Comment 1 Tom "spot" Callaway 2007-01-20 21:42:10 UTC
Good:

- rpmlint checks return:
W: obexftp-python no-documentation
W: obexftp-perl no-documentation
W: obexftp-devel no-documentation

All safe to ignore.

- package meets naming guidelines
- package meets packaging guidelines
- license (GPL) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- devel package ok
- no .la files
- post/postun ldconfig ok
- devel requires base package n-v-r 

Looks good, APPROVED.

Comment 2 Michael Schwendt 2007-01-20 22:06:46 UTC
%defattr missing in -perl package

Comment 3 Ville Skyttä 2007-01-20 22:41:18 UTC
Requires: perl(:MODULE_COMPAT_...) missing as well.

Subpackages should probably be called python-obexftp and perl-obexftp instead of
obexftp-* according to package naming guidelines.

_obexftp.so.X and _obexftp.so.X.X for a Python extension are probably just
cruft, and could be replaced with a plain _obexftp.so.

%configure --disable-dependency-tracking would result in cleaner build output
and possibly speed up the build.

The minimum openobex version this works with is 1.2, making the build dep on
openobex-devel versioned (>= 1.2) would save some trouble from people using
older distros.

Requires: openobex-devel >= 1.2 missing from -devel, see #includes in installed
header files.

Comment 4 Dominik 'Rathann' Mierzejewski 2007-01-21 23:41:16 UTC
(In reply to comment #3)
> Requires: perl(:MODULE_COMPAT_...) missing as well.

Any... pointers to some docs on that? I've searched through packaging guidelines
and there's nothing about that. I only found some old IRC logs mentioning this,
but nothing tangible.

> Subpackages should probably be called python-obexftp and perl-obexftp instead
> of obexftp-* according to package naming guidelines.

Perl naming guidelines say that only about CPAN-originated modules. Plus we have
lots of other packages named something-perl, for example openbabel (also mine).

> _obexftp.so.X and _obexftp.so.X.X for a Python extension are probably just
> cruft, and could be replaced with a plain _obexftp.so.

I don't know much about python. Are you sure I can do that?

> %configure --disable-dependency-tracking would result in cleaner build output
> and possibly speed up the build.

OK.

> The minimum openobex version this works with is 1.2, making the build dep on
> openobex-devel versioned (>= 1.2) would save some trouble from people using
> older distros.
> 
> Requires: openobex-devel >= 1.2 missing from -devel, see #includes in
> installed header files.

OK.


Comment 5 Ville Skyttä 2007-01-22 07:49:38 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Requires: perl(:MODULE_COMPAT_...) missing as well.
> 
> Any... pointers to some docs on that?

Not that I'm aware of.  Just copy/paste the line from
/etc/rpmdevtools/spectemplate-perl.spec

> > Subpackages should probably be called python-obexftp and perl-obexftp 
> > instead of obexftp-* according to package naming guidelines.
> 
> Perl naming guidelines say that only about CPAN-originated modules.

None of the other cases talk anything about where the software originates from,
and I don't see why it would make a difference, nor why the general rule
wouldn't apply, see Addon packages (general) in naming guidelines.

> > _obexftp.so.X and _obexftp.so.X.X for a Python extension are probably just
> > cruft, and could be replaced with a plain _obexftp.so.
> 
> I don't know much about python. Are you sure I can do that?

Pretty sure.  Will need to verify this though.

Comment 6 Ville Skyttä 2007-01-23 18:41:35 UTC
To clarify, I don't consider foo-obexftp vs obexftp-foo for foo={perl,python} a
blocker as there are some interpretation differences about the naming guidelines
and quite a few packages going either way in the repo.  This is something that
the packaging committee should work on, and can be fixed later if need be.

Regarding _obexftp.so.*.*, a simple "python -c 'import obexftp'" will work with
only the *.so present, it doesn't need the *.so.*.  And 'find /usr/lib/python2.4
-name "*.so.*"' finds nothing here.  So I *guess* it's safe to install the
extension just as *.so and drop the links.  This is neither a blocker though,
but is something upstream could perhaps clarify.

Comment 7 Dominik 'Rathann' Mierzejewski 2007-01-25 10:33:54 UTC
OK, included all suggestions except _obexftp.so.*.* renaming in python
subpackage. I'll import the package sometime tonight. Thanks for comments, everyone.

Comment 8 Dominik 'Rathann' Mierzejewski 2007-01-26 02:33:35 UTC
imported 0.20-2 with the following changelog:
- added missing defattr
- require openobex-devel > 1.2
- added missing MODULE_COMPAT Requires: to perl subpackage
- renamed subpackages to perl/python-obexftp
- fixed rpaths

Thanks to Toshio for some help with rpaths.

Built for devel, FC-6 branch requested. Thanks for the review, Tom.

Comment 9 Ville Skyttä 2007-01-26 19:02:37 UTC
Just in case it wasn't intentional: the package doesn't have a disttag so some
manual release tag adjustments will be needed so that the FC-6 and devel builds
don't end up with identical NEVR's.

Comment 10 Dominik 'Rathann' Mierzejewski 2007-01-26 20:58:58 UTC
I noticed that while trying to build for FC-6 and fixed it.


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