Bug 168339

NEEDSWORK:

* BuildRequires:  libstdc++-devel
  ^^^^ This is probably superfluous.

* BuildRequires:  info
  ^^^^ This is probably wrong.
info is an *.info reader. *.infos normally are generated by makeinfo (from the
texinfo packages). Packages complying to the GNU Standards ship prebuilt *.infos
(Which AFAIS applies, here), so neither info nor texinfo should be required for
building.

* %{_includedir}/*.h
I hate packages installing their headers to /usr/include, because they pollute
the system include path. Instead, I recommend to install them to a subdirectory
of /usr/include, e.g.
%configure --includedir=%{_include}/binio
...
%{_includedir}/binio
...

* Your %post script is non functional.
You can't use -p /sbin/ldconfig there because you are trying to install infos at
the same time. You'll have to use something along these lines:

Requires(post): /sbin/ldconfig
Requires(post): /sbin/install-info

%post
/sbin/ldconfig
/sbin/install-info .....

Thanks for the splendid review Ralf (as always).

New source package is created:
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio-1.3-2.src.rpm

The BuildRequires: info was for the /sbin/install-info script that is used
in the build process, I have clarified this by changing it to
BuildRequires: /sbin/install-info. The rest is fixed according to your
comments.

Modifying the path where the headers are installed in a package which doesn't  
have a foo-config script or a pkgconfig file (which would adjust to correspond  
with the path where the headers are actually installed in the package) causes  
annoying extra work for packagers and others using such a -devel package,  
needs special treatment in all dependent packages, and should be thus strongly  
discouraged IMO.  If there's a foo-config or a *.pc file that would reflect  
the change, it'd be ok. 
 
On a related note, could add "|| :" at the end of those install-info commands 
in order to support --excludedocs installs. 

(In reply to comment #3)
> Modifying the path where the headers are installed in a package which doesn't  
> have a foo-config script or a pkgconfig file (which would adjust to correspond  
> with the path where the headers are actually installed in the package) causes  
> annoying extra work for packagers and others using such a -devel package,  
What is so difficult to use
gcc -I/usr/include/binio .... ?!?

> needs special treatment in all dependent packages, and should be thus strongly  
> discouraged IMO.
I could not disagree more. IMO, all packages which install header files directly
to /usr/include are badly designed considered to be rejected.

(In reply to comment #4) 
> What is so difficult to use 
> gcc -I/usr/include/binio .... ?!? 
 
Well, it wouldn't be difficult to install the libs (or eg. "gcc", or whatever) 
into a custom path and to ask everyone to deal with it either.  That doesn't 
make it the right thing to do. 
 
See comment 3.  When done _by a packager on "IMO" basis_, it's (most likely, 
unchecked in this particular case) completely unnecessary, bloats dependent 
packages, possible maintenance burden, results in projects needing distro 
specific adaptations, not what upstream intended, and unexpected in general. 
 
I didn't say that dropping everything into /usr/include is optimal either.  
But non-transparent changes like this should not be made unilaterally by 
packagers, but rather reported/fixed upstream (preferably fixing it with a 
foo-config and/or a pkgconfig file) if something causes real world problems, 
or if someone cares deeply enough about it otherwise and can convince 
upstream. 

OK then, two mentors say different things and I get to decide. Ville wrote
large parts of the Maximum RPM book so his karma is better, and I therefore
restore the .h files to the %{_includedir}.

New source package is created:
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio-1.3-3.src.rpm

These magic "|| :" things - if I am correctly informed it is just a
way of using bash operators to avoid stop of the execution flow.
Isn't there something less magic than this "|| :" for example an
environment variable that can be IF:ed like this:

if [ $excludedocs = 1 ]; then
/sbin/install-info ....
fi

I have however added them so you can safely install with --excludedocs.
Am I approved then...

(In reply to comment #5)
> (In reply to comment #4) 
> > What is so difficult to use 
> > gcc -I/usr/include/binio .... ?!? 
>  
> Well, it wouldn't be difficult to install the libs (or eg. "gcc", or whatever) 
> into a custom path and to ask everyone to deal with it either. 
GCC already does do this automatically ;)

> See comment 3.  When done _by a packager on "IMO" basis_, it's (most likely, 
> unchecked in this particular case) completely unnecessary, bloats dependent 
> packages, possible maintenance burden, results in projects needing distro 
> specific adaptations, not what upstream intended, and unexpected in general. 
BS - All packaging is based technical requirements AND on personal views.
Also, it's packagers who package set the per-package conventions for each
package. And it's the package users who have to adopt these per-package conventions.

> I didn't say that dropping everything into /usr/include is optimal either.
> But non-transparent changes like this should not be made unilaterally by 
> packagers, but rather reported/fixed upstream (preferably fixing it with a 
> foo-config and/or a pkgconfig file) if something causes real world problems, 

Any file in /usr/include causes real problems, because any file in /usr/include
* is likely to clash with standards (POSIX etc.)
* is likely to clash with other packages (other packages shipping headers with
the same name)
* prevents parallel installation.
* /usr/include is a privileged directory (system include directory) and is
treated special by compilers (secondary include path).

> or if someone cares deeply enough about it otherwise and can convince 
> upstream. 
If something clashes, it's too late for upstream to change.

Also, the main reason, why packages install headers into /usr/include is the
developers typically installing packages into private directories or into per
package directories, but not systemwide.

I will not approve any package which installs headers directly to /usr/include.

OK so how are these disputes normally resolved?

I have no problem with doing what Ralf says, because the package that
I am creating to use libbinio do not complain about having its headers
in /include/binio so that is OK with me.

Ralf says he will veto against this package unless I do so, so Ville,
will you also veto against it in case I do so that it is a catch22
situation, or can you live with that I follow Ralfs demand?

Will it be brought up to the steering committe othwerwise?

Sorry Ralf if I was misreading the diplomatic tone of considering vetos...
Anyway, I propose the following solution:

1. There is a new package,
   http://www.df.lth.se/~triad/krad/fc/libbinio-1.3-4.src.rpm
   which restores the relocation to %{_includedir}/binio

2. I will only build this for devel and FC-4 due to problems with GCC
   3.x (it does not descend into subdirs for finding headers, which means
   it would need lots of #include <foo.h> to be patched to 
   #include <binio/foo.h>). When upstream fixes this problem, we will
   consider packages for FC-3 and below to be built.

3. I will notify upstream that we want this to be default behaviour,
   if it is easy enough, I will also prepare a patch.

4. Ralf approves the package for devel and FC-4.

Is everybody happy with this solution?

Upstream has been notified on the issue, see:
http://sourceforge.net/tracker/index.php?func=detail&aid=1293200&group_id=69728&atid=525584

I don't understand point 2.  First of all, any program which uses libbinio
ought to use '#include "binio/foo.h"' or '#include <binio/foo.h>"' anyway,
emphasising that these are non-standard headers. Else the fun starts when
the headers are installed into %_includedir/binio on one distribution and
%_includedir/libbinio on another one.
Secondly, for any version of GCC to find the headers in %_includedir/binio,
you need to add %_includedir/binio to the compiler's search path list, i.e.
-I/usr/include/binio for any programs which do '#include <binfile.h>',
pretending that it would be a standard header or a header in standard
search path.

As a another voice in this matter, I consider any program which cannot
be configured easily to extend the search path list for headers as broken.
You should always be able to build a program with libbinio installed into
/home/foo/libbinio/include (and /home/foo/libbinio/lib), for instance,
without having to do apply a lot of patch-work.

I'm not going to veto this change, but my opinion has not changed, and in any 
case, I suggest waiting for upstream comments before approving/pushing the 
package..
 
Re comment 6: 
 
- s/large parts/some bits here and there/ 
 
- "|| :" like in this case is just a cheap, generic way to ensure that the 
%post* etc scriptlet does not fail.  I'm not aware of a way to specifically 
detect an --excludedocs install.  One could check if the %{_infodir}/foo.info* 
files exist before invoking install-info on them though, but IMO "|| :" is 
very much sufficient in this case. 
 
Re comment 7: 
 
Forcing end users to adapt to per-package decisions instead of making things 
Just Work as expected out of the box significantly decreases the value of 
packaged software. 

The problem with this package has been fixed, due to the much responsive
and understanding upstream author named Simon Peter. libbinio now installs
headers in %{_includedir}/libbinio/ (not binio - there is according to Simon
already an (much outdated) GNU package with this name).

The new upstream release also adds a pkgconfig libbinio.pc file to the
%{_libdir}/pkgconfig directory.

New package and SPEC:
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio-1.4-1.src.rpm

Another quality fix suggested by the Fedora project! Everybody should 
be happy now. (Atleast for the time being.)

(In reply to comment #13)
> New package and SPEC:
> Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio.spec

%{_includedir}/%{name}/*.h
^^^
this should be 
%{_includedir}/%{name}

Uh um I don't quite get it but does that syntax mean "own the directory and
everything in it" then I broke it down to the more obvious:

%dir %{_includedir}/%{name}
%{_includedir}/%{name}/*.h

Is this OK?
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio-1.4-2.src.rpm

> %dir %{_includedir}/%{name}
> %{_includedir}/%{name}/*.h

Right.

Where you want to include the directory and its contents recursively,
the following one is a good recommendation:

%{_datadir}/%{name}/

The slash at the end makes it more clear that it is a directory and
not just a file.

Could this be approved then?

All NEEDSWORK on this package has, to the best of my knowledge, been
completed. Would someone please approve it?

Would someone like to FE-ACCEPT (Approve) this package now?

(In reply to comment #19)
> Would someone like to FE-ACCEPT (Approve) this package now?
First of all, you can't push me or any body else around here - Most of us are 
volunteers and probably only very few are being paid, so .. patience please ;)

Anyway, further comments:

There still are blocking issues:

1. the /usr/share/info/dir entry being produced by the *.texi is broken. 
(Try "info libbinio" and try to enter the libbinio node).

A fix to the sources would be this patch and to regenerate the info files:

diff -ur libbinio-1.4.orig/doc/libbinio.texi libbinio-1.4/doc/libbinio.texi
--- libbinio-1.4.orig/doc/libbinio.texi 2004-08-18 21:49:11.000000000 +0200
+++ libbinio-1.4/doc/libbinio.texi      2005-09-27 14:43:23.000000000 +0200
@@ -27,7 +27,7 @@

 @dircategory Software Libraries
 @direntry
-* libbinio: (libbinio)          Binary I/O stream class library @value{VERSION}
+* libbinio: (libbinio).         Binary I/O stream class library @value{VERSION}
 @end direntry

 @titlepage

2. The info files are developer docs, so they should better be packaged into the
*-devel package.

3. The pkgconfig file is non-functional:
# pkg-config --libs libbinio
Unknown keyword 'URL' in '/usr/lib/pkgconfig/libbinio.pc'


Non blocking issues:

* IMO, this is questionable design:
# pkg-config --cflags libbinio
-I/usr/include/libbinio

Is this really what upstream wants?

* Has this code been tested on big endian targets?

Well, yeah sorry Ralf, patience is not my strong side. Curiously
enough it worked this time ;-)

I had this idea that once I have been submitted my first error-free 
package I would take on reviewing packages myself so more things get 
done. I know you're all volunteers. I try to be polite but whenever I 
fail feel free to correct me (as you did).

Anyway, I filed bugs upstream with your comments for (1) and (3) so let's 
hope it gets fixed.
http://sourceforge.net/tracker/index.php?func=detail&aid=1305865&group_id=69728&atid=525584
http://sourceforge.net/tracker/index.php?func=detail&aid=1305853&group_id=69728&atid=525584

The (2) issue is fixed but I'll wait for upstream and 
(hopefully) fix them all in next package.

Comment on (3) from upstream:

>Comment By: Simon Peter (dynamite)
Date: 2005-09-28 12:54

Message:
Logged In: YES
user_id=31101

The pkgconfig file works here:
# pkg-config --libs libbinio
-L/home/simon/lib -lbinio

I'm using version 0.19 of pkg-config. I got the URL keyword
directly from the example in the manpage of pkg-config.

For the second point, I made cflags point to
/usr/include/libbinio for backwards compatibility. If i
would make it point to /usr/include and use #include
<libbinio/binio.h> instead, i would have to change all my
other software for the new include path. I don't know if
this is what Ralf meant.

(In reply to comment #22)

> The pkgconfig file works here:
> # pkg-config --libs libbinio
> -L/home/simon/lib -lbinio
> 
> I'm using version 0.19 of pkg-config.
FC4 ships pkgconfig-0.15.0-6

=> Either upstream should not use "URL:"
or Linus should patch libbinio.pc
or libbinio-devel should Require: pkgconfig >= 0.19 (I.e. the package could not
be shipped for FC4)

> For the second point, I made cflags point to
> /usr/include/libbinio for backwards compatibility. If i
> would make it point to /usr/include and use #include
> <libbinio/binio.h> instead, i would have to change all my
> other software for the new include path. I don't know if
> this is what Ralf meant.
Yes, this is one point, I was referring to, but this is your (upstream's) decision.

I will make a patch to remove the URL directive and once imported 
into the Extras I will consider reenabling the URL: directive for 
the devel version if and when it ships with a sufficiently new 
pkg-config. As for now it will be turned off.

A new package with all fixes is due soon.

Here is a fixed spec and SRPM package:

Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio-1.4-3.src.rpm

(1) Texinfo file patched until upstream is fixed
(2) Texinfo files moved to devel package, along with the texinfo scriptlets
(3) Pkgconfig-file is patched to not used URL token for the time being

You are patching the *.texinfo. Therefore you should 
BR: makeinfo
otherwise the *infos will not be regenerated.

OK Ralf sorry for that real obvious thing. Here is a fixed package:

Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/libbinio-1.4-4.src.rpm

I am still having doubts if this package works on big-endian targets, but the
packaging seems to be fine, now.

APPROVED

OK imported and builds. Thanks Ralf, and the others.