Libguestfs - Nov 2015 - Re: [PATCH] launch: add missing headers on Darwin

On 4 November 2015 at 12:37, Pino Toscano <ptoscano@redhat.com> wrote:
[...]
> NACK, these OS-specific blocks are hard to maintain, and it makes no
> sense to enclose POSIX headers in them.
>
> Can you please try the attached patch? I see Rich already pushed your
> patch, but IMHO that should be reverted and this one should go...

Happy to. However, these same header files have been removed in
3b17152ec399acb35b23331d2c3b4f424a68824a:

commit 3b17152ec399acb35b23331d2c3b4f424a68824a
Author: Richard W.M. Jones <rjones@redhat.com>
Date:   Tue Sep 29 21:58:41 2015 +0100

    lib: Remove unused header files.

    I used ESR's deheader program to look for unused includes.  I then
    examined each instance by hand, and also test-compiled (on Linux).

...which makes me suspect it will happen again at some point unless there
is some indication that it's a non-Linux include.


-- 
Margaret

On Wed, Nov 04, 2015 at 12:43:31PM +0000, Margaret Lewicka
wrote:
> On 4 November 2015 at 12:37, Pino Toscano <ptoscano@redhat.com>
wrote:
> [...]
> 
> > NACK, these OS-specific blocks are hard to maintain, and it makes no
> > sense to enclose POSIX headers in them.
> >
> > Can you please try the attached patch? I see Rich already pushed your
> > patch, but IMHO that should be reverted and this one should go...
> 
> 
> Happy to. However, these same header files have been removed in
> 3b17152ec399acb35b23331d2c3b4f424a68824a:
> 
> commit 3b17152ec399acb35b23331d2c3b4f424a68824a
> Author: Richard W.M. Jones <rjones@redhat.com>
> Date:   Tue Sep 29 21:58:41 2015 +0100
> 
>     lib: Remove unused header files.
> 
>     I used ESR's deheader program to look for unused includes.  I then
>     examined each instance by hand, and also test-compiled (on Linux).
> 
> ...which makes me suspect it will happen again at some point unless there
> is some indication that it's a non-Linux include.
Yup, sorry about that one.

Perhaps a comment, like:

#include <sys/un.h> /* Required for Mac OS X */

?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On 4 November 2015 at 13:07, Richard W.M. Jones <rjones@redhat.com> wrote:
[...]
> > Happy to. However, these same header files have been removed in
> > 3b17152ec399acb35b23331d2c3b4f424a68824a:
> >
> > commit 3b17152ec399acb35b23331d2c3b4f424a68824a
> > Author: Richard W.M. Jones <rjones@redhat.com>
> > Date:   Tue Sep 29 21:58:41 2015 +0100
> >
> >     lib: Remove unused header files.
> >
> >     I used ESR's deheader program to look for unused includes.  I
then
> >     examined each instance by hand, and also test-compiled (on Linux).
> >
> > ...which makes me suspect it will happen again at some point unless
there
> > is some indication that it's a non-Linux include.
>
> Yup, sorry about that one.
>
> Perhaps a comment, like:
>
> #include <sys/un.h> /* Required for Mac OS X */
>
> ?

You're the boss(es) -- I'll go with any solution that a) keeps the
headers
in, b) satisfies your requirements. If a comment is enough to preserve
these includes during the next header purge, I'm happy to commit a new
patch.


-- 
Margaret

On Wednesday 04 November 2015 12:43:31 Margaret Lewicka
wrote:
> On 4 November 2015 at 12:37, Pino Toscano <ptoscano@redhat.com>
wrote:
> [...]
> 
> > NACK, these OS-specific blocks are hard to maintain, and it makes no
> > sense to enclose POSIX headers in them.
> >
> > Can you please try the attached patch? I see Rich already pushed your
> > patch, but IMHO that should be reverted and this one should go...
> 
> 
> Happy to. However, these same header files have been removed in
> 3b17152ec399acb35b23331d2c3b4f424a68824a:
> 
> commit 3b17152ec399acb35b23331d2c3b4f424a68824a
> Author: Richard W.M. Jones <rjones@redhat.com>
> Date:   Tue Sep 29 21:58:41 2015 +0100
> 
>     lib: Remove unused header files.
> 
>     I used ESR's deheader program to look for unused includes.  I then
>     examined each instance by hand, and also test-compiled (on Linux).
> 
> ...which makes me suspect it will happen again at some point unless there
> is some indication that it's a non-Linux include.
Well, it was because something else implicitly pulled it. Note that
sys/un.h is a POSIX header, and it's needed for sockaddr_un [1].

[1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_un.h.html

-- 
Pino Toscano