Bug 384191 (libdiscid)

Summary: Review Request: libdiscid - A library for creating MusicBrainz DiscIDs
Product: [Fedora] Fedora Reporter: Alex Lancaster <alexl>
Component: Package ReviewAssignee: Nicolas Chauvet (kwizart) <kwizart>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: bnocera, fedora-package-review, i, ismael, jeff, notting, rdieter
Target Milestone: ---Flags: kwizart: fedora‑review+
limburgher: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-11-16 21:35:43 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 248308, 384251, 1065583    

Description Alex Lancaster 2007-11-15 04:23:44 EST
Spec URL: http://alexlan.fedorapeople.org/reviews/libdiscid.spec
SRPM URL: http://alexlan.fedorapeople.org/reviews/libdiscid-0.1.1-2.fc7.src.rpm
Description: 
libdiscid is a C library for creating MusicBrainz DiscIDs from audio CDs. It
reads a CD's table of contents (TOC) and generates an identifier which can be
used to lookup the CD at MusicBrainz. Additionally, it provides a submission
URL for adding the DiscID to the database.
Comment 1 Nicolas Chauvet (kwizart) 2007-11-15 07:41:17 EST
starting review
Comment 2 Nicolas Chauvet (kwizart) 2007-11-15 10:53:37 EST
Well... from README
NOTE: This library is also bundled with libmusicbrainz version 3 or later.
      If you already have libmusicbrainz installed, there is no need to
      install libdiscid separately.

This means the package will be obsoleted soon... So I would say if
libmusicbrainz2 could have libdiscid support, that would be fine to add it from
within the libmusicbrainz2 package and not as a separate package...

If any have futher advices....?


Comment 3 Nicolas Chauvet (kwizart) 2007-11-15 11:03:05 EST
Well to avoid missunderstanding, the main reason why I think it could be
provided within the libmusicbrainz2 package is that there is no headers in it...
So it is not meant to be used alone... (unless dlopened, but i think not...)
Comment 4 Alex Lancaster 2007-11-16 01:42:30 EST
(In reply to comment #2)
> Well... from README
> NOTE: This library is also bundled with libmusicbrainz version 3 or later.
>       If you already have libmusicbrainz installed, there is no need to
>       install libdiscid separately.
> 
> This means the package will be obsoleted soon... So I would say if
> libmusicbrainz2 could have libdiscid support, that would be fine to add it from
> within the libmusicbrainz2 package and not as a separate package...
> 
> If any have futher advices....?

We should probably consult the libmusicbrainz maintainer, Bastien and/or the
python-musicbrainz2 maintainer about whether to upgrade libmusicbrainz to version 3.

It's my understanding also that python-musicbrainz2 should use libdiscid, but
whether it's sufficient to have that in libmusicbrainz, or a standalone library,
I'm not sure.  I'll check on the #musicbrainz IRC channel on freenode.

Meanwhile since picard doesn't directly detect or require libdiscid (either at
build-time or run-time), and runs perfectly fine without it, please don't let
this package review stop the picard review from proceeding...

Comment 6 Nicolas Chauvet (kwizart) 2007-11-16 07:02:28 EST
* Archive file has a better timestamp now - OK

* In Description %{name} is used.. As description need to be a full phrase, we
"usually" begin phrase with a cap...(ie Libdiscid or use This C Library is
for...) - NEED_WORK

* Requires:  pkgconfig is missing in -devel - NEED_WORK

* -devel provide a header into it owns directory, but libdiscid.pc do not
mention this directory...  - SHOULD
This wouldn't cost to have .pc.in patched so it can point to this directory...
Packages that will be built upon this one may fails if they only has #include
<discid.h> - 
What i understand for now is that none app currently directly links to it...:
"this is not a direct wrapper, but contains functions to access libdiscid using
it's own API" - But it would be fine to have it point the right
_includedir/discid if any want to acces it directly later... (I leave this point
up to you..)

* Please save header timestramp to change having : - NEED_WORK
 make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p" 
This will prevents multilibs systems to consider the file different has
timestamp will be changed at build time...

* As a COPYING file is present you must include it...  - NEED_WORK
Comment 7 Alex Lancaster 2007-11-16 07:22:54 EST
* Fri Nov 16 2007 Alex Lancaster <alexl@users.sourceforge.net> - 0.1.1-5
- Fix description
- devel package Requires: pkgconfig
- save header timestamps

* Fri Nov 16 2007 Alex Lancaster <alexl@users.sourceforge.net> - 0.1.1-4
- Remove unneeded doc directive in -devel package, add COPYING file

Spec URL: http://alexlan.fedorapeople.org/reviews/libdiscid.spec
SRPM URL: http://alexlan.fedorapeople.org/reviews/libdiscid-0.1.1-5.fc7.src.rpm

Comment 8 Alex Lancaster 2007-11-16 07:31:29 EST
Updated files section in -devel package like so:

%files devel
%defattr(-,root,root,-)
%{_includedir}/discid/
%{_libdir}/*.so
%{_libdir}/pkgconfig/*.pc
Comment 9 Nicolas Chauvet (kwizart) 2007-11-16 07:36:19 EST
---------------------------------------------------------------

    This package (libdiscid) is APPROVED by me

---------------------------------------------------------------
Comment 10 Alex Lancaster 2007-11-16 07:44:31 EST
New Package CVS Request
=======================
Package Name: libdiscid
Short Description: A library for creating MusicBrainz DiscIDs
Owners: alexlan
Branches: F-7 F-8
InitialCC:
Cvsextras Commits: yes
Comment 11 Kevin Fenzi 2007-11-16 12:15:24 EST
cvs done.
Comment 12 Alex Lancaster 2007-11-16 21:35:43 EST
Builds fine in devel/ pending updates in updates-testing for F-7 and F-8,
closing review.
Comment 13 Ismael Olea 2014-02-17 05:33:35 EST
Package Change Request
======================
Package Name: libdiscid
New Branches: epel-7
Owners: cicku

Requested at #1065583
Comment 14 Jon Ciesla 2014-02-17 07:57:20 EST
Any comments from the Fedora maintiners?
Comment 15 Ismael Olea 2014-02-17 08:04:14 EST
As comaintainer it's obviously fine with me.
Comment 16 Alex Lancaster 2014-02-20 00:24:18 EST
(In reply to Jon Ciesla from comment #14)
> Any comments from the Fedora maintiners?

All good with me!
Comment 17 Christopher Meng 2014-02-20 00:26:01 EST
Package Change Request
======================
Package Name: libdiscid
New Branches: epel7
Owners: cicku
Comment 18 Jon Ciesla 2014-02-20 08:43:17 EST
Git done (by process-git-requests).