Bug 328771

For the url, you should use, such that it is easier to check 
your source:
http://fedoraproject.org/wiki/Packaging/SourceURL#head-615f6271efb394ab340a93a6cf030f2d08cf0d49

and not use a inexistent source url.

Do you intend to add this package to another branch 
than devel?

I propose the following to ship the html docs:

rm -rf __package_docs
mkdir -p __package_docs
cp -pr docs __package_docs
mv __package_docs/docs __package_docs/html
rm -rf __package_docs/html/tools __package_docs/html/.svn
__package_docs/html/graphics/.svn
rm __package_docs/html/Makefile* __package_docs/html/graphics/Makefile*

and add 
%doc __package_docs/html

You could use
make DESTDIR=%{buildroot} install INSTALL='install -p'
to keep timestamps

Suggestion: use
rm %{buildroot}%{_libdir}/*.la
to be noticed when .la files aren't shipped anymore.

(In reply to comment #1)
> For the url, you should use, such that it is easier to check 
> your source:
>
http://fedoraproject.org/wiki/Packaging/SourceURL#head-615f6271efb394ab340a93a6cf030f2d08cf0d49
> 
> and not use a inexistent source url.
>
Fixed 
> Do you intend to add this package to another branch 
> than devel?
>
Maybe F-7. I already got a request to update gnome-scan there to the latest.
 
> I propose the following to ship the html docs:
> 
I did this in a slightly different way.
Thanks.
Spec URL: ftp://czar.eas.yorku.ca/pub/babl/bal.spec
SRPM URL: ftp://czar.eas.yorku.ca/pub/babl/babl-0.0.15-0.2.20071011svn.fc8.src.rpm

There is a dot missing at the end of the devel %description.

Also the timestamps are still not kept. As above, I propose:
make DESTDIR=%{buildroot} install INSTALL='install -p'

The changelog entry is not very clear in my opinion:
- Remove the use of inexistent source (from package reviews)
I would suggest something along:
- clarify how the source may be generated
and don't mention the fact that the sources were inexistent, since
the source were not really inexistent the url was. It is only
a suggestion and not a big deal.

As for shipping babl svn snapshot in F-7 I think that it 
should be avoided, unless there are severe regression/bugs or 
there are wonderful new functionalities.

(In reply to comment #3)
> There is a dot missing at the end of the devel %description.
>
Will add it.
 
> Also the timestamps are still not kept. As above, I propose:
> make DESTDIR=%{buildroot} install INSTALL='install -p'
>
I don't see why its necessary, the packaging guildeline say to do it, when the
copy command is used. There's no mention of doing it for make install.
  
> The changelog entry is not very clear in my opinion:
> - Remove the use of inexistent source (from package reviews)
A typo, should have been 'inexistent source url'.  Will fix.

> 
> As for shipping babl svn snapshot in F-7 I think that it 
> should be avoided, unless there are severe regression/bugs or 
> there are wonderful new functionalities.
Actually, there are 'wonderful new functionalities' in latest gnome-scan (at
least according to the author), and it depends the svn version of babl and gegl.

(In reply to comment #4)

> > Also the timestamps are still not kept. As above, I propose:
> > make DESTDIR=%{buildroot} install INSTALL='install -p'
> >
> I don't see why its necessary, the packaging guildeline say to do it, when the
> copy command is used. There's no mention of doing it for make install.

It is an obvious deficiency of the guidelines. It is 
obviously better to keep timestamps for all the files 
that are not generated (and sometimes it is also better
to have relevant timestamps on generated files, in case
of multiarch packages sharing those files).

> > The changelog entry is not very clear in my opinion:
> > - Remove the use of inexistent source (from package reviews)
> A typo, should have been 'inexistent source url'.  Will fix.

Ok.
 
> > As for shipping babl svn snapshot in F-7 I think that it 
> > should be avoided, unless there are severe regression/bugs or 
> > there are wonderful new functionalities.
> Actually, there are 'wonderful new functionalities' in latest gnome-scan (at
> least according to the author), and it depends the svn version of babl and gegl.

Ok.

With fixes from comment #3,
Spec URL: ftp://czar.eas.yorku.ca/pub/babl/bal.spec
SRPM URL: ftp://czar.eas.yorku.ca/pub/babl/babl-0.0.15-0.2.20071011svn.fc8.src.rpm

There is a typo in the %changelog, as rpmlint shows:
babl.i386: W: incoherent-version-in-changelog 0.0.15-0.3.svn20071011
0.0.15-0.3.20071011svn.fc8

What is exactly the use of 
/usr/lib/babl-0.0/CIE-Lab.so
/usr/lib/babl-0.0/gegl-fixups.so
/usr/lib/babl-0.0/gggl-lies.so
/usr/lib/babl-0.0/gggl.so
/usr/lib/babl-0.0/naive-CMYK.so

They don't look like library to link against, but rather like
dlopened extensions. Is it the case? In that case they should be
in the main package, unless they are examples that shouldn't be 
shipped. And since gggl stuff seems to be GPLv2+, the version 
should be updated.

What about adding:

%check
make check

(In reply to comment #7)

> What is exactly the use of 
> /usr/lib/babl-0.0/CIE-Lab.so
> /usr/lib/babl-0.0/gegl-fixups.so
> /usr/lib/babl-0.0/gggl-lies.so
> /usr/lib/babl-0.0/gggl.so
> /usr/lib/babl-0.0/naive-CMYK.so
> 
You're correct, they are extension. I've put them in the main package.
 
> They don't look like library to link against, but rather like
> dlopened extensions. Is it the case? In that case they should be
> in the main package, unless they are examples that shouldn't be 
> shipped. And since gggl stuff seems to be GPLv2+, the version 
> should be updated.

I dont understand what you mean by the last sentence.

Spec URL: ftp://czar.eas.yorku.ca/pub/babl/bal.spec
SRPM URL: ftp://czar.eas.yorku.ca/pub/babl/babl-0.0.15-0.4.20071011svn.fc8.src.rpm

(In reply to comment #8)

> > shipped. And since gggl stuff seems to be GPLv2+, the version 
> > should be updated.
> 
> I dont understand what you mean by the last sentence.

The gggl code, that makes /usr/lib/babl-0.0/gggl.so,
for example, is covered by the GPLv2+. If the babl library dlopens 
gggl.so, then the whole is covered by the GPLv2+. So license
should be GPLv2+. If the extension is not used in the default
case, maybe the best would be to have
License: LGPLv2+ and GPLv2+
I haven't seen in the doc something clear about enabling
or disabling the extensions, though.

In general %check is after %install. Just a suggestion.

(In reply to comment #8)
> >  And since gggl stuff seems to be GPLv2+, the version 
> > should be updated.
> 
> I dont understand what you mean by the last sentence.

Rereading myself, I just used version instead of license!
Should have been:

And since gggl stuff seems to be GPLv2+, the License
should be updated.

Thanks, I've updated the spec file.

Spec URL: ftp://czar.eas.yorku.ca/pub/babl/bal.spec
SRPM URL: ftp://czar.eas.yorku.ca/pub/babl/babl-0.0.15-0.5.20071011svn.fc8.src.rpm

* rpmlint is silent
* follow packaging guidelines
* free software, one of the license included. 
* source match upstream except for some dates in .svn/entries 
* %files section right
* library packaged rightly

Please add a comment explaining what files are covered by the 
GPL.

APPROVED

New Package CVS Request
=======================
Package Name: babl
Short Description: A dynamic, any to any, pixel format conversion library
Owners: deji
Branches: F-7 F-8
InitialCC: 
Cvsextras Commits: yes

CVS done.

Thanks to Patrice and Jason. I'll add the comment about while file are under the
GPL later in CVS.