rpmlint on SRPM gave me
W: libcdaudio invalid-license GPL2

* /usr/bin/libcdaudio-config needs a patch for option --libs, because
it contains a hardcoded  ${exec_prefix}/lib  path.

* RPM Group better is changed to:  System Environment/Libraries

God catch, thanks!

Spec URL: http://dl.atrpms.net/all/libcdaudio.spec
SRPM URL: http://dl.atrpms.net/all/libcdaudio-0.99.12p2-8.at.src.rpm

* Fri Dec 29 2006 Axel Thimm <Axel.Thimm> - 0.99.12p2-8
- Change Group tag.
- Fix libcdaudio-config for libdir != %%{_prefix}/lib.

* source files match upstream:
  - 15de3830b751818a54a42899bd3ae72c libcdaudio-0.99.12p2.tar.gz
  - 15de3830b751818a54a42899bd3ae72c libcdaudio-0.99.12p2-orig.tar.gz
* package meets naming and versioning guidelines
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
! BuildRoot is not correct, should be:
      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
* license field matches the actual license.
* license is open source-compatible.  GPL2, License text included in package.
  - rpmlint complains about GPL2, but rpmlint probably shouldn't...
* latest version is being packaged.
! BuildRequires are proper.
  - Technically, nothing wrong with gcc-c++, but its listed in the
    packaging guidelines as an exception that should be left out,
    since its always in the minimum build root.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (F7/x86_64).
* package installs properly
* debuginfo package looks complete.
* rpmlint is silent.
  - Only complaint is the Warning about GPL2, which I say we ignore.
* final provides and requires are sane:
    libcdaudio provides/requires:
    --
    libcdaudio.so.1()(64bit)  
    libcdaudio = 0.99.12p2-8.fc7
    --
    libcdaudio.so.1()(64bit)

    libcdaudio-devel provides/requires:
    --
    libcdaudio-devel = 0.99.12p2-8.fc7
    --
    libcdaudio = 0.99.12p2-8.fc7
    libcdaudio.so.1()(64bit)
    pkgconfig
* %check is n/a
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* 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.
* headers in -devel
  - cdaudio.h ought to be installed w/-p to preserve timestamps,
    but that looks to be upstream's fault
* pkgconfig files in -devel, properly requires pkgconfig
* no libtool .la droppings.
* not a GUI app.


I think the only MUSTFIX item is the BuildRoot, but I'd suggest also dropping
the BR: on gcc-c++. Whether or not to patch the Makefile to install the header
w/-p I'll leave up to you.

Minor correction after talking with some folks on the license issue: it should
be reduced to simply "GPL", since the included readme doesn't list an explicit
version of the GPL.

--
libcdaudio is distributed under the GNU Library General Public
License, included in this package under the top level source directory
in the file COPYING.
--

Thanks for the review!

On buildroots we recently voted against that buildroot and currently I'm trying
to get the mandatory insanity of the guidelines, so let's keep it open for now,
I hope for the best ;)

On the license: There is a COPYING file that explicitely states
>                     GNU GENERAL PUBLIC LICENSE
>                        Version 2, June 1991

And the README says

> libcdaudio is distributed under the GNU Library General Public
> License, included in this package under the top level source directory
> in the file COPYING.

(In reply to comment #7)
> Thanks for the review!

No problem, sorry it sat so long w/o any attention.

> On buildroots we recently voted against that buildroot and currently I'm trying
> to get the mandatory insanity of the guidelines, so let's keep it open for now,
> I hope for the best ;)

Hm... Talked to some FESCo folks, they say that the buildroot specified in the
guidelines was accepted by FESCo via a vote at FUDCon as the standard for now,
but with work that needs doing at the rpm level to properly address concerns. So
for the moment, I'm told that it does have to be as specified... Who voted
against it?

> On the license: There is a COPYING file that explicitely states
> >                     GNU GENERAL PUBLIC LICENSE
> >                        Version 2, June 1991
> 
> And the README says
> 
> > libcdaudio is distributed under the GNU Library General Public
> > License, included in this package under the top level source directory
> > in the file COPYING.

I saw that the actual license file says v2, but that's the case with almost
every GPL-licensed bits these days. My understanding from the FESCo folks I was
talking to is that unless the author says v2 is explicitly required, to just
keep the license field general, specifying just "GPL". Personally, I don't
really care either way, just trying to follow the letter of the law, so to
speak. (Though I like "GPLv2" better than "GPL2".)

The buildroot was voted on in the FPC and later ratified by fesco. Since the
vote in the FPC was under peculiar circumstances the issue was raised again the
next week and the buildroot changed (by vote) to one w/o id -un.

Currently I'm trying to persuade the FPC to drop the mandatory part on
buildroots and simply allow any sane buildroot.

I'd like for someone from the FPC and/or FESCo to comment. People I'm finding on
IRC say it was agreed it should be changed to something without the id, call,
instead using a mktemp call, but that had unexpected negative side-effects, so
everything is currently still as it has been. So at the moment, the guidelines
still say that the mandatory buildroot is the one previously quoted.

Of course, I agree that anything sane should be fine. Above and beyond that, I
really don't think we should have to care what the buildroot is at all. If its
supposed to be the same in all specs, it should be specified by the build
system, not duplicated in every spec.

> I'd like for someone from the FPC and/or FESCo to comment.

I'm from FPC. Yes, we went for using mktemp -ud as the lesser evil. The expected
side-effects are that stepped rpmbuilds (using --short-circuit for example)
won't work anymore unless you supply a buildroot on the command line.

> Of course, I agree that anything sane should be fine. Above and beyond that, I
> really don't think we should have to care what the buildroot is at all. If its
> supposed to be the same in all specs, it should be specified by the build
> system, not duplicated in every spec.

+\infty

(In reply to comment #11)
> > I'd like for someone from the FPC and/or FESCo to comment.
> 
> I'm from FPC.

Ah, I was completely unaware of that. :)

It seems there has been a lack of communication between FPC and FESCo. At the
very least, some members of FESCo appear to be unaware of any changes made to
the packaging guidelines...

> Yes, we went for using mktemp -ud as the lesser evil. The expected
> side-effects are that stepped rpmbuilds (using --short-circuit for example)
> won't work anymore unless you supply a buildroot on the command line.

So where does this leave us with respect to what's currently spelled out as
being mandatory in the guidelines?

> > Of course, I agree that anything sane should be fine. Above and beyond that, I
> > really don't think we should have to care what the buildroot is at all. If its
> > supposed to be the same in all specs, it should be specified by the build
> > system, not duplicated in every spec.
> 
> +\infty

:)

So how about just putting in the current "Mandatory" BuildRoot value as listed
in the current packaging guidelines for now, and once the dust settles for The
Grand Future of BuildRoot, change it to whatever the new-and-improved version is?

http://fedoraproject.org/wiki/Packaging/Guidelines#head-f196e7b2477c2f5dd97ef64e8eacddfb517f1aa1
says 
The MANDATORY value for the BuildRoot tag is 
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)


its mandatory use it regardless of your personal beliefs the community that 
you are part of agreed to it.  if it changes at some point in the future you 
will be free to change it then.  until then please abide by the guidelines.

(In reply to comment #11)
> I'm from FPC. Yes, we went for using mktemp -ud as the lesser evil. The expected
> side-effects are that stepped rpmbuilds (using --short-circuit for example)
> won't work anymore unless you supply a buildroot on the command line.

Could you post a reproducer for this to fedora-packaging?  -bi --short-circuit
builds work just fine for me with a mktemp -ud BuildRoot when it's specified in
a specfile.  OTOH it doesn't work for whatever reason if I set it in
~/.rpmmacros (gets evaluated twice in that case), but perhaps that's what you meant?

(In reply to comment #12)
> (In reply to comment #11)
> It seems there has been a lack of communication between FPC and FESCo. At the
> very least, some members of FESCo appear to be unaware of any changes made to
> the packaging guidelines...

Well, fesco and fpc overlap by > 50% or more, so at least that amount of fesco
should be aware of fpc topics automatically and have perfect communication. :)

(In reply to comment #13)
> its mandatory use it regardless of your personal beliefs the community that 
> you are part of agreed to it.  if it changes at some point in the future

Ahem, the "future" was a week ago. FPC reevaluated this decision and dropped
that buildroot in favour of the mktemp one, the voting was unanimously. But I
don't know whether it was persented to fesco on Thursday and if, whether it was
ratified.

> you will be free to change it then.  until then please abide by the
> guidelines.

Please check fedora-packaging

(In reply to comment #14)
> (In reply to comment #11)
> > I'm from FPC. Yes, we went for using mktemp -ud as the lesser evil. The expected
> > side-effects are that stepped rpmbuilds (using --short-circuit for example)
> > won't work anymore unless you supply a buildroot on the command line.
> 
> Could you post a reproducer for this to fedora-packaging?  -bi --short-circuit
> builds work just fine for me with a mktemp -ud BuildRoot when it's specified in
> a specfile.  OTOH it doesn't work for whatever reason if I set it in
> ~/.rpmmacros (gets evaluated twice in that case), but perhaps that's what you
meant?

Sorry, Ville, I think you are 100% correct and this is a Red Herring. I was just
quoting racor on this w/o thinking too much on my own.

Indeed, since it's not quite sane to use the buildroot outside of %install and
%clean, all that is slightly broken is separating building and cleaning, but
IMHO we can live with that. Of course you will find some people that claim that
%buildroot should be available during %build, too, and I remember such a
discussion on fedora-packaging, perhaps racor was even one of them. We should
better create a guideline against using %buildroot in %prep and %build. :)

So even less evil with the mktemp variant.

> -bi --short-circuit builds work just fine for me with a
> mktemp -ud BuildRoot when it's specified in a specfile.

Define "work just fine". You do get a new buildroot with every
invocation of rpmbuild, don't you? Effectively, these newly created
buildroots pile up to a gigantic pile of crap. Subsequent invocations
of rpmbuild do not remove the old buildroots automatically as it
is done with a predictable buildroot filename. In situations when
--short-circuit is really helpful, the pile of buildroots gets
inconvenient to navigate in and requires work-arounds. Hence a
mandatory buildroot that uses mktemp would turn against packagers
and would be a bad decision.

(In reply to comment #16)
> You do get a new buildroot with every invocation of rpmbuild, don't you?

No, only on rpmbuilds that go into %install. That's due to using the -u switch
of mktemp. Quite clever and credits go to Ville for that.

> In situations when --short-circuit is really helpful, the pile of buildroots
> gets inconvenient

Forget I mentioned --short-circuit, Ville's trick deals fine with that, and I
simply trusted some mails I had read w/o checking, so --short-circuit is not an
issue.

The only remaining bit is running %install w/o %clean. This will indeed leave
buildroots behind.

I did refer to --short-circuit commands in my entire comment.
Please do try the proposed buildroot with --short-circuit and
prove that it doesn't do what I've written in comment 16. Option
-u does not prevent mktemp from returning a new path with
every invocation.

> The only remaining bit is running %install w/o %clean.

?? But --short-circuit -i does execute %install, so that is
where the crap starts.

(In reply to comment #16)
> You do get a new buildroot with every invocation of rpmbuild, don't you?
> Effectively, these newly created buildroots pile up to a gigantic pile of
> crap.

(In reply to comment #18)
> Please do try the proposed buildroot with --short-circuit and
> prove that it doesn't do what I've written in comment 16. Option
> -u does not prevent mktemp from returning a new path with
> every invocation.

Prove? It's true that it *returns* a new buildroot on every invocation, but it
does not *create* it, no "gigantic pile of crap" is generated by merely invoking
rpmbuild as you wrote. E.g. rpmbuild -bp/-bc are not creating anything. 

And that's why --short-circuit has nothing to do with this here, unless you
would had been suggesting packages that use %buildroot in %prep and %build,
which (almost) all agree to be A Bad Thing.

Anyway whoever is smart enough to make diligent use of --short-circuit will be
able to use his favourite external %buildroot setting as well.

But for the normal non-short-circuit case there is still a small bitter pill
left, that seperated %install and %clean of rpmbuilds will leave buildroots behind.

Ceterum censeo clausulum definendam buildrootium coactum esse esse delendam.

Ceterum censeo 

I still only refer to --short-circuit commands. Get back to
the quote that started this:

> -bi --short-circuit builds work just fine for me with a
> mktemp -ud BuildRoot when it's specified in a specfile.

Do you see? -bi --short-circuit

> Anyway whoever is smart enough to make diligent use of
> --short-circuit will be able to use his favourite external
> %buildroot setting as well.

So, I must specify a BuildRoot in the spec only to override
it in ~/.rpmmacros because it is over-engineered crap? Sad.

Please take this discussion to the fedora-packaging mailing list.

Last week the buildroot proposal

http://fedoraproject.org/wiki/Packaging/Guidelines?action=recall&rev=92#head-b4fdd45fa76cbf54c885ef0836361319ab962473

was ratified by both fpc and fesco and I just edited the guidelines. :)

Yep, saw that and was meaning to dig this bug back up and throw an APPROVED
stamp on it accordingly.

Package APPROVED, import at will.

Whoops, wrong blocker bug... Will set the review + flag too...

Thanks!

New Package CVS Request
=======================
Package Name: libcdaudio
Short Description: Control operation of a CD-ROM when playing audio CDs
Owners: Axel.Thimm
Branches: FC-5 FC-6
InitialCC: 

branched

...and built. Can this bug be closed now?

Thanks again for the review!