Bug 322061 - Review Request: asunder - A graphical Audio CD ripper and encoder
Review Request: asunder - A graphical Audio CD ripper and encoder
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michael Schwendt
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-07 08:43 EDT by Marcin Zajaczkowski
Modified: 2007-11-30 17:12 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-11-12 17:20:15 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
bugs.michael: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Marcin Zajaczkowski 2007-10-07 08:43:35 EDT
Spec URL: http://timeoff.wsisiz.edu.pl/rpms/asunder/asunder.spec
SRPM URL: http://timeoff.wsisiz.edu.pl/rpms/asunder/asunder-0.9-1.src.rpm
Description: Allows to save tracks from an Audio CD as WAV, MP3, OGG, and/or FLAC.
Comment 1 Marcin Zajaczkowski 2007-10-07 09:45:35 EDT
And it's a desktop environment neutral.
Comment 2 Michael Schwendt 2007-11-08 15:52:30 EST
> rpmlint -i asunder-0.9-1.src.rpm 
> asunder.src: W: non-coherent-filename asunder-0.9-1.src.rpm
> The file which contains the package should be named
> <NAME>-<VERSION>-<RELEASE>.<ARCH>.rpm.

Unimportant, but: rpm -qp --qf %{r}\\n asunder-0.9-1.src.rpm 
1.fc5


> %configure --prefix=%{_prefix}

That arg is the default.


> Requires:       libcddb >= 0.9.5
> BuildRequires:  libcddb-devel >= 0.9.5

Questionable.

Why? First of all, libcddb < 0.9.6 is more than three years old.
There have been several [much] newer versions since then. Secondly,
when building against 1.3.0 in Fedora 7, for instance, it is unsafe
to assume that >= 0.9.5 would be sufficient. That old version is
not ABI compatible.

Suggestion: Keep the version in the BuildRequires if you insist on
doing that (although Fedora's libcddb is latest). Drop the version
in the Requires and rely on rpmbuild's SONAME dependencies.


> #is already installed by desktop-file-install
> rm %{buildroot}%{_datadir}/applications/asunder.desktop

You can tell desktop-file-install to  --delete-original  and
give this file as input.

[...]

At run-time, I cannot get it to rip anything, not even as WAV.
It opens an error dialog:

  There was an error creating 15 files

or (when I select only one file):

  There was an error creating 1 files

$ asunder
Unable to delete WAV file "/home/qa/Desktop/Various Artists - Back In Time
II/Various Artists - Martin Galway/Galway Is God 2000 (CD mix by Jogeir
Liljedahl).wav": No such file or directory

It has only created an empty directory. The most I can get out of
it is an .m3u file if I enable that option.
Comment 3 Andrew Smith 2007-11-09 05:18:01 EST
Michael, can you post here or email me details about how to reproduce this bug?
If it's a pretty standard Fedora install, what version?

What format do you have set for the album directory and the music file (in the
preferences)? Do you have 'single artist' checked?

For that particular CD, could you tell me what the album artist, album title,
and track name are (as displayed by Asunder)? I'm a bit confused why there seems
to be two album names ('Various Artists - Back In Time II' and 'Various Artists
- Martin Galway').

I will fix this as soon as I figure out what's wrong, but I suspect it's an
unusual problem (this is the only report I got) so I wouldn't block it from..
whatever you were doing Marcin :)
Comment 4 Marcin Zajaczkowski 2007-11-09 15:56:44 EST
(In reply to comment #2)
> Unimportant, but: rpm -qp --qf %{r}\\n asunder-0.9-1.src.rpm 
> 1.fc5

Probably I have too old rpmlint (0.79), because I haven't got that message.
I know about .fc5 (I have to remove SRPM name everytime). The only solution
would be remove %dist macro from .rpmmacros when building srpm for upload and
later turn it on to build binary RPM with valid %dist (binary RPMs are also
available on my website).

> > %configure --prefix=%{_prefix}
> 
> That arg is the default.

Yup, fixed.

> > Requires:       libcddb >= 0.9.5
> > BuildRequires:  libcddb-devel >= 0.9.5
> 
> Questionable.

I took it from the program website. I didn't test it with an older versions. In
FC5 there is 1.2.2.
Andrew, what as the author you think about backward compatibility?

> You can tell desktop-file-install to  --delete-original  and
> give this file as input.

Useful option.

> At run-time, I cannot get it to rip anything, not even as WAV.

I was able to rip CD on my FC5. I will check also on F7. Hopefully you resolve
that problem with Andrew.
Comment 5 Andrew Smith 2007-11-09 16:18:27 EST
> Andrew, what as the author you think about backward compatibility?
I feel strongly that backward-compatibility must exist unless there's a really
good reason otherwise.

But I don't think that applies to binary packages. If a package is made for a
distro that has a certain version of libcddb, I don't see why the binary can't
depend on that version or newer. In other words - I don't think it's a problem
to change it to 1.2.2

One day 'll learn something about binary compatibility, but not today :)
Comment 6 Michael Schwendt 2007-11-09 17:10:19 EST
This application doesn't replace slashes in the character
strings it creates file names from:

Album artist: Various Artists
Album title: Back in Time II
Track 2 Title: Martin Galway/Galway Is God 2000 (CD mix by Jogeir Liljedahl)

Notice the '/' in the track title, which is seen as a directory
delimiter by the file-system. Unless there is a document which
defines that a slash must not be used in the track titles (and in
that case the CD would be wrong), not substituting those slashes
is an application bug.

[...]

I've given a different Audio CD a try, and there the application works,
so:

APPROVED (with the things from comment 2 fixed)
Comment 7 Marcin Zajaczkowski 2007-11-10 06:37:50 EST
Ok, I removed a version from Requires. Your example with build on F7 is very
reasonable. I left version for BuildRequires, but there is already EOL for
systems with so old version of libcddb bundled, so it's probably unnecessary.

Changelog summarize:

* Fri Nov 09 2007 Marcin Zajaczkowski <mszpak ATT wp DOTT pl> - 0.9-2
- applied Michael Schwendt suggestions (all below)
- removed redundant --prefix argument
- used --delete-original to delete .desktop file
- removed minimal required version of libcddb from Requires

Current version:
http://timeoff.wsisiz.edu.pl/rpms/asunder/asunder.spec
http://timeoff.wsisiz.edu.pl/rpms/asunder/asunder-0.9-2.src.rpm
Comment 8 Marcin Zajaczkowski 2007-11-10 06:40:07 EST
New Package CVS Request
=======================
Package Name: asunder
Short Description: A graphical Audio CD ripper and encoder
Owners: szpak
Branches: F-7 F-8
InitialCC: 
Cvsextras Commits: yes
Comment 9 Kevin Fenzi 2007-11-10 15:29:38 EST
cvs done.
Comment 10 Marcin Zajaczkowski 2007-11-12 17:20:15 EST
Commited, built and pushed for testing.

Thanks for your help, guys.

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