Bug 177865

Summary: Review Request: adplay
Product: [Fedora] Fedora Reporter: Linus Walleij <triad>
Component: Package ReviewAssignee: Jason Tibbitts <j>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: imlinux
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-06-27 09:27:48 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 177818    
Bug Blocks: 163779    

Description Linus Walleij 2006-01-15 20:59:59 UTC
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/adplay.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/adplay-1.4-1.src.rpm
Description:
AdPlay is a command-line player for AdLib (OPL2) music utilizing
the AdPlug library.

Comment 1 Linus Walleij 2006-01-24 16:09:26 UTC
New package:
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/adplay.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/adplay-1.4-2.src.rpm

Comment 2 Mike McGrath 2006-01-24 17:18:06 UTC
Source should be http://dl.sourceforge.net/%{name}/%{name}-%{version}.tar.gz

Comment 3 Mike McGrath 2006-01-24 19:28:31 UTC
Actually, the package is called adplug, not adplay.  So the sourceforge project
adplay does not exist, adplug does though.

Comment 4 Linus Walleij 2006-01-24 19:56:17 UTC
Well yeah the project is called adplug, and the package is called adplay.
So:

wget http://download.sourceforge.net/adplug/adplay-1.4.tar.gz

works, thus I change the first name and it's fixed... I guess its the same
for xmms-adplug (a related package) so I will fix that too.

Comment 5 Linus Walleij 2006-01-24 22:04:12 UTC
New package:
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/adplay.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/adplay-1.4-3.src.rpm

Comment 6 Mike McGrath 2006-01-25 01:11:00 UTC
Sorry, I should have emphasized that the Source should be http://dl.sourceforge.net/

Comment 7 Linus Walleij 2006-01-25 07:43:23 UTC
Why? Has sourceforge stated that they will remove the DNS name 
http://dowload.sourceforge.net?

Well, I think it's like:
http://dl.sourceforge.net/adplug/adplay-1.4.tar.gz

will take the latest version directly from sourceforge, whereas
http://download.sourceforge.net/adplug/adplay-1.4.tar.gz

picks a mirror. Why must we not use a mirror? (Right now the mirror
is broken, yeah... perhaps that is reason enough :-)

Comment 8 Paul Howarth 2006-01-25 08:06:48 UTC
(In reply to comment #7)
> Why? Has sourceforge stated that they will remove the DNS name 
> http://dowload.sourceforge.net?
> 
> Well, I think it's like:
> http://dl.sourceforge.net/adplug/adplay-1.4.tar.gz
> 
> will take the latest version directly from sourceforge, whereas
> http://download.sourceforge.net/adplug/adplay-1.4.tar.gz
> 
> picks a mirror. Why must we not use a mirror? (Right now the mirror
> is broken, yeah... perhaps that is reason enough :-)

Both work fine for me; no need to change it.

The usual problem with sourceforge-hosted packages is that people specify
"prdownloads.." URLs, which don't work because they pull up the mirror selection
page rather than the actual tarball.


Comment 9 Michael Schwendt 2006-01-31 11:43:13 UTC
Use

  export CXXFLAGS="${RPM_OPT_FLAGS} -I%{_includedir}/libbinio"

instead of just

  export CXXFLAGS=-I%{_includedir}/libbinio

so you build with Fedora default compiler flags.


BR libstdc++-devel is redundant, as gcc-c++ is an implicit build
requirement and depends on libstdc++-devel.

Comment 10 Linus Walleij 2006-02-01 20:26:34 UTC
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/adplay.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/adplay-1.4-4.src.rpm

Fixes for the stuff pointed out by Michael.

Comment 11 Linus Walleij 2006-05-06 19:52:10 UTC
New release from upstream:

Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/adplay.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/adplay-1.5-1.src.rpm

Comment 12 Jason Tibbitts 2006-06-24 04:41:58 UTC
Builds fine in mock (x86_64, development) and rpmlint is silent.  Not much to
say; it's a tiny package.

* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* source files match upstream:
   18e1ac84b6f07d0388902a083f400da7  adplay-1.5.tar.bz2
* latest version is being packaged.
* BuildRequires are proper.
* package builds in mock (x86_64, development).
* rpmlint is silent.
* final provides and requires are sane:
   adplay = 1.5-1.fc6
  =
   libadplug-2.0.so.0()(64bit)
   libasound.so.2()(64bit)
   libasound.so.2(ALSA_0.9)(64bit)
   libasound.so.2(ALSA_0.9.0rc4)(64bit)
   libaudiofile.so.0()(64bit)
   libbinio.so.1()(64bit)
   libesd.so.0()(64bit)
* no shared libraries are present.
* package is not relocatable.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* %clean is present.
* %check is not present; no test suite upstream.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app.

APPROVED

Comment 13 Linus Walleij 2006-06-27 09:27:48 UTC
OK imported and built, will be released in next sweep.

THANK YOU Jason for picking this up, your help and attention
to detail is much appreciated.