Bug 183290 - Review Request: libipoddevice
Summary: Review Request: libipoddevice
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Brian Pepple
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT 183291 183292
TreeView+ depends on / blocked
 
Reported: 2006-02-27 23:03 UTC by Christopher Aillon
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-03-03 00:44:33 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Christopher Aillon 2006-02-27 23:03:48 UTC
Spec and SRPM from: http://people.redhat.com/caillon/RPMS/rawhide/banshee/
Description: libipoddevice provides device-level support for Apple's iPods. libipoddevice is written in GObject/C.

Comment 1 Ralf Corsepius 2006-02-28 07:33:24 UTC
One minor nit, which gets exposed when trying to build this package on FC4:
...
error: Package requirements (gobject-2.0         glib-2.0        dbus-1 
dbus-glib-1     hal >= 0.5.2  libgtop-2.0 >= 2.12.0) were not met:

Requested 'libgtop-2.0 >= 2.12.0' but version of libgtop is 2.10.1

=> Missing "BuildRequires: libgtop-devel >= 2.12.0"

Comment 2 Brian Pepple 2006-03-02 00:13:52 UTC
MD5Sums:
000b3c7b82026bf052a16811ba34c515  libipoddevice-0.4.1.tar.gz

Good:
* Upstream source tarball verified
* Package name conforms to the Fedora Naming Guidelines
* Group Tag is from the official list
* Buildroot has all required elements
* All paths begin with macros
* All necessary BuildRequires listed.
* Package builds in Mock.
* Package installs and uninstalls cleanly on FC5.
* Make succeeds even when %{_smp_mflags} is defined
* Rpmlint does not find problems

Needs Work:
* Source URL isn't canonical. Should be
http://banshee-project.org/files/%{name}/%{name}-%{version}.tar.gz
* Refer to Comment #1, if planning to release for FC4.
* License is LGPL, not GPL.
* Add COPYING file to package.

Once these items are fixed, consider it approved.

Comment 3 Ignacio Vazquez-Abrams 2006-03-02 00:24:37 UTC
(In reply to comment #2)
> * Source URL isn't canonical. Should be
> http://banshee-project.org/files/%{name}/%{name}-%{version}.tar.gz

Don't use %{name}, %{version}, etc. in the pathname, only in the filename.

Comment 4 Brian Pepple 2006-03-02 00:36:56 UTC
(In reply to comment #3)
> Don't use %{name}, %{version}, etc. in the pathname, only in the filename.

Why, has something changed recently in the wiki?



Comment 5 Ignacio Vazquez-Abrams 2006-03-02 02:41:09 UTC
It's never actually been put into the wiki, but that's been an unwritten rule
for as long as I can remember.

Comment 6 Brian Pepple 2006-03-02 03:10:31 UTC
(In reply to comment #5)
> It's never actually been put into the wiki, but that's been an unwritten rule
> for as long as I can remember.

Really?  I've been doing packages since back in the fedora.us days, and I'm not
familar with that.

Comment 7 Ralf Corsepius 2006-03-02 04:10:15 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > It's never actually been put into the wiki, but that's been an unwritten rule
> > for as long as I can remember.
> 
> Really?  I've been doing packages since back in the fedora.us days, and I'm not
> familar with that.
Neither am I nor do I agree with Ignacio.

Comment 8 Nicolas Mailhot 2006-03-02 09:49:36 UTC
(In reply to comment #5)
> It's never actually been put into the wiki, but that's been an unwritten rule
> for as long as I can remember.

Actually it's never actually been into the wiki because the proponents of this
rule never managed to convince other packagers. So
1. today it's not mandatory
2. I very much doubt it would be accepted if someone tried to make it mandatory

Comment 9 Michael Schwendt 2006-03-02 15:18:43 UTC
Well, it's been a recommendation at fedora.us, and many packagers
have been so nice and accepted it.

Its background is that

 (1) packagers would simply copy URLs into the .spec file, making sure
 the download location from within their browser/downloader really matches
 the spec file

 (2) reviewers could copy the URL during review and would not need to
 use spectool or other forms of expanding the macros and retrieving a
 valid URL -- remember that reviewers also need to verify download
 locations, so visiting web pages, looking up download sections and
 comparing the URLs with spec files

With macros in download URLs, what often happens is that packagers don't
verify them, not even when a tarball changes from .tar.gz to .tar.bz2 and
moves to a different directory. In that case they simply change the file
extension to make the src.rpm work again. ;)


Comment 10 Nicolas Mailhot 2006-03-02 16:05:34 UTC
(In reply to comment #9)
> Well, it's been a recommendation at fedora.us, and many packagers
> have been so nice and accepted it.

Need I remind you fedora.us pushed epochs everywhere?
fedora.us rules can and have been revisited, I fact I distinctly remember this
specific issue being discussed on fedora-extras or fedora-maintainers, and as a
result of the discussion it was NOT added to FE packaging rules


Comment 11 Michael Schwendt 2006-03-02 16:49:19 UTC
I don't understand what you're trying to point out, I'm afraid.
There's a big difference between guidelines and policies.
Explicit Epoch 0 was a necessity and a completely different topic, btw.


Comment 12 Christopher Aillon 2006-03-03 00:44:33 UTC
Okay, this discussion should go elsewhere.  I didn't use any macros for
directories, mainly out of personal preference.  Module successfully imported
into extras.

Comment 13 Ralf Corsepius 2006-03-03 06:13:13 UTC
Some recommendations for you spec file:

* Using
%configure --disable-static
and removing the "%exclude *.a" would speed up compilation by almost factor 2 

* %configure --disable-dependency-tracking
would further speed up building.

* I'd use
make DESTDIR=${RPM_BUILD_ROOT) install
instead of %makeinstall (This avoids breaking libtool archives for some versions
of libtool ;) )


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