[libvirt] [PATCH 0/3] Fix examples/ under mingw

Eric Blake posted 3 patches 5 years, 2 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190108195641.27052-1-eblake@redhat.com
cfg.mk                              |  2 +-
examples/admin/client_info.c        |  6 +++++-
examples/admin/list_clients.c       |  6 +++++-
examples/domtop/domtop.c            | 13 ++++++-------
examples/object-events/event-test.c | 16 +++++++---------
5 files changed, 24 insertions(+), 19 deletions(-)
[libvirt] [PATCH 0/3] Fix examples/ under mingw
Posted by Eric Blake 5 years, 2 months ago
My recent patch to make all of the examples work independently of
gnulib worked fine on Linux, but failed on mingw:
https://travis-ci.org/libvirt/libvirt/jobs/476898635

The whole point of gnulib is to work around portability pitfalls so
we don't have to bend over backwards thinking about it in our code;
and it would be possible to fix this bug by linking the problematic
example binaries against gnulib after all.  But since the examples
are still small and self-contained enough that using manual
workarounds wasn't too daunting, that's the approach I took here.

I'm pushing this series as a build-breaker fix in a couple of hours,
or sooner if I get a review.

Eric Blake (3):
  examples: Work around mingw printf() weakness
  examples: Work around lack of mingw sigaction()
  examples: Work around lack of mingw localtime_r()

 cfg.mk                              |  2 +-
 examples/admin/client_info.c        |  6 +++++-
 examples/admin/list_clients.c       |  6 +++++-
 examples/domtop/domtop.c            | 13 ++++++-------
 examples/object-events/event-test.c | 16 +++++++---------
 5 files changed, 24 insertions(+), 19 deletions(-)

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Fix examples/ under mingw
Posted by Andrea Bolognani 5 years, 2 months ago
On Tue, 2019-01-08 at 13:56 -0600, Eric Blake wrote:
> My recent patch to make all of the examples work independently of
> gnulib worked fine on Linux, but failed on mingw:
> https://travis-ci.org/libvirt/libvirt/jobs/476898635
> 
> The whole point of gnulib is to work around portability pitfalls so
> we don't have to bend over backwards thinking about it in our code;
> and it would be possible to fix this bug by linking the problematic
> example binaries against gnulib after all.  But since the examples
> are still small and self-contained enough that using manual
> workarounds wasn't too daunting, that's the approach I took here.

It's kind of a done deal now, but I still wonder if it was the right
approach.

The way I see it, our examples are supposed to illustrate how to use
libvirt itself, not how to write C code that is portable to a
multitude of platforms: with that goal in mind, taking advantage of
gnulib makes perfect sense, as it allows us to put the focus on the
usage of libvirt rather than the surrounding compatibility gunk.

I would probably have a different opinion about this if the
workarounds were related to eg. using certain types as opposed to
others when passing data to libvirt functions, but as far as I can
tell that's not the case.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Fix examples/ under mingw
Posted by Daniel P. Berrangé 5 years, 2 months ago
On Wed, Jan 09, 2019 at 01:33:44PM +0100, Andrea Bolognani wrote:
> On Tue, 2019-01-08 at 13:56 -0600, Eric Blake wrote:
> > My recent patch to make all of the examples work independently of
> > gnulib worked fine on Linux, but failed on mingw:
> > https://travis-ci.org/libvirt/libvirt/jobs/476898635
> > 
> > The whole point of gnulib is to work around portability pitfalls so
> > we don't have to bend over backwards thinking about it in our code;
> > and it would be possible to fix this bug by linking the problematic
> > example binaries against gnulib after all.  But since the examples
> > are still small and self-contained enough that using manual
> > workarounds wasn't too daunting, that's the approach I took here.
> 
> It's kind of a done deal now, but I still wonder if it was the right
> approach.
> 
> The way I see it, our examples are supposed to illustrate how to use
> libvirt itself, not how to write C code that is portable to a
> multitude of platforms: with that goal in mind, taking advantage of
> gnulib makes perfect sense, as it allows us to put the focus on the
> usage of libvirt rather than the surrounding compatibility gunk.

Using gnulib is not an easy thing if you are not familiar with
it, largely because of the painful autotools integration it
imposes. It is important that the examples are both simple to
read, and simple to build standalone. We've always considered that
the examples should be possible to build using nothing more than
the C compiler and pkg-config. eg 

  $CC  `pkg-config --cflags --libs libvirt` -o foo foo.c

The benefits of gnulib are not compelling enough to be worth the
complexity that it brings in for the examples. 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Fix examples/ under mingw
Posted by Andrea Bolognani 5 years, 2 months ago
On Wed, 2019-01-09 at 12:53 +0000, Daniel P. Berrangé wrote:
> On Wed, Jan 09, 2019 at 01:33:44PM +0100, Andrea Bolognani wrote:
> > The way I see it, our examples are supposed to illustrate how to use
> > libvirt itself, not how to write C code that is portable to a
> > multitude of platforms: with that goal in mind, taking advantage of
> > gnulib makes perfect sense, as it allows us to put the focus on the
> > usage of libvirt rather than the surrounding compatibility gunk.
> 
> Using gnulib is not an easy thing if you are not familiar with
> it, largely because of the painful autotools integration it
> imposes. It is important that the examples are both simple to
> read, and simple to build standalone. We've always considered that
> the examples should be possible to build using nothing more than
> the C compiler and pkg-config. eg 
> 
>   $CC  `pkg-config --cflags --libs libvirt` -o foo foo.c
> 
> The benefits of gnulib are not compelling enough to be worth the
> complexity that it brings in for the examples. 

I would see a point in making them buildable in a standalone
fashion if

  1) we installed them, eg. under /usr/share/doc/libvirt/examples
  2) we also installed, for each of them, a plain Makefile that
     calls $CC and pkg-config as seen above

Until that's the case, the gnulib dependency makes complete sense to
me, because you're not going to install libvirt using your package
manager, then clone the git repository and figure out yourself how
to compile the examples, just to poke around a bit.


Anyway, I've noticed that the apparmor/ directory contains files
that we end up installing on the system in non-documentation
directories, which feels all kinds of wrong. Am I missing something,
or should we move that stuff somewhere else?

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Fix examples/ under mingw
Posted by Daniel P. Berrangé 5 years, 2 months ago
On Wed, Jan 09, 2019 at 03:46:19PM +0100, Andrea Bolognani wrote:
> On Wed, 2019-01-09 at 12:53 +0000, Daniel P. Berrangé wrote:
> > On Wed, Jan 09, 2019 at 01:33:44PM +0100, Andrea Bolognani wrote:
> > > The way I see it, our examples are supposed to illustrate how to use
> > > libvirt itself, not how to write C code that is portable to a
> > > multitude of platforms: with that goal in mind, taking advantage of
> > > gnulib makes perfect sense, as it allows us to put the focus on the
> > > usage of libvirt rather than the surrounding compatibility gunk.
> > 
> > Using gnulib is not an easy thing if you are not familiar with
> > it, largely because of the painful autotools integration it
> > imposes. It is important that the examples are both simple to
> > read, and simple to build standalone. We've always considered that
> > the examples should be possible to build using nothing more than
> > the C compiler and pkg-config. eg 
> > 
> >   $CC  `pkg-config --cflags --libs libvirt` -o foo foo.c
> > 
> > The benefits of gnulib are not compelling enough to be worth the
> > complexity that it brings in for the examples. 
> 
> I would see a point in making them buildable in a standalone
> fashion if
> 
>   1) we installed them, eg. under /usr/share/doc/libvirt/examples
>   2) we also installed, for each of them, a plain Makefile that
>      calls $CC and pkg-config as seen above
> 
> Until that's the case, the gnulib dependency makes complete sense to
> me, because you're not going to install libvirt using your package
> manager, then clone the git repository and figure out yourself how
> to compile the examples, just to poke around a bit.

I don't really agree. There's no need to clone the git repo to use
the examples. I've given people direct links to the gitweb viewer
for example programs and just told them to build using the $CC
arg above. Sure we could install them & provide a plain Makefile,
but that's tangential to use of gnulib IMHO.

> Anyway, I've noticed that the apparmor/ directory contains files
> that we end up installing on the system in non-documentation
> directories, which feels all kinds of wrong. Am I missing something,
> or should we move that stuff somewhere else?

Yes, those look like they should be in src/security instead.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Fix examples/ under mingw
Posted by Andrea Bolognani 5 years, 2 months ago
On Wed, 2019-01-09 at 14:54 +0000, Daniel P. Berrangé wrote:
[...]
> I don't really agree. There's no need to clone the git repo to use
> the examples. I've given people direct links to the gitweb viewer
> for example programs and just told them to build using the $CC
> arg above. Sure we could install them & provide a plain Makefile,
> but that's tangential to use of gnulib IMHO.

Okay, fair enough.

> > Anyway, I've noticed that the apparmor/ directory contains files
> > that we end up installing on the system in non-documentation
> > directories, which feels all kinds of wrong. Am I missing something,
> > or should we move that stuff somewhere else?
> 
> Yes, those look like they should be in src/security instead.

I'm cooking a patch, I'll post it in a while.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Fix examples/ under mingw
Posted by John Ferlan 5 years, 2 months ago

On 1/9/19 7:33 AM, Andrea Bolognani wrote:
> On Tue, 2019-01-08 at 13:56 -0600, Eric Blake wrote:
>> My recent patch to make all of the examples work independently of
>> gnulib worked fine on Linux, but failed on mingw:
>> https://travis-ci.org/libvirt/libvirt/jobs/476898635
>>
>> The whole point of gnulib is to work around portability pitfalls so
>> we don't have to bend over backwards thinking about it in our code;
>> and it would be possible to fix this bug by linking the problematic
>> example binaries against gnulib after all.  But since the examples
>> are still small and self-contained enough that using manual
>> workarounds wasn't too daunting, that's the approach I took here.
> 
> It's kind of a done deal now, but I still wonder if it was the right
> approach.
> 
> The way I see it, our examples are supposed to illustrate how to use
> libvirt itself, not how to write C code that is portable to a
> multitude of platforms: with that goal in mind, taking advantage of
> gnulib makes perfect sense, as it allows us to put the focus on the
> usage of libvirt rather than the surrounding compatibility gunk.

Not everyone that would write to libvirt API's would have installed
gnulib. Asked differently, is having gnulib installed a prerequisite for
having libvirt installed? What about libvirt-devel?

While altering these examples to adhere to some least common denominator
architecture is perhaps less optimal - it does show one can generate
architecture agnostic examples as long as they understand issues
surrounding portability.

An alternative one could generate patches for would be #ifdef'd code or
perhaps even simply comments indicating the alternative methodology that
is less portable. For those without git history it's perhaps harder to
figure out.

John

> 
> I would probably have a different opinion about this if the
> workarounds were related to eg. using certain types as opposed to
> others when passing data to libvirt functions, but as far as I can
> tell that's not the case.
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Fix examples/ under mingw
Posted by John Ferlan 5 years, 2 months ago

On 1/8/19 2:56 PM, Eric Blake wrote:
> My recent patch to make all of the examples work independently of
> gnulib worked fine on Linux, but failed on mingw:
> https://travis-ci.org/libvirt/libvirt/jobs/476898635
> 
> The whole point of gnulib is to work around portability pitfalls so
> we don't have to bend over backwards thinking about it in our code;
> and it would be possible to fix this bug by linking the problematic
> example binaries against gnulib after all.  But since the examples
> are still small and self-contained enough that using manual
> workarounds wasn't too daunting, that's the approach I took here.
> 
> I'm pushing this series as a build-breaker fix in a couple of hours,
> or sooner if I get a review.
> 
> Eric Blake (3):
>   examples: Work around mingw printf() weakness
>   examples: Work around lack of mingw sigaction()
>   examples: Work around lack of mingw localtime_r()
> 
>  cfg.mk                              |  2 +-
>  examples/admin/client_info.c        |  6 +++++-
>  examples/admin/list_clients.c       |  6 +++++-
>  examples/domtop/domtop.c            | 13 ++++++-------
>  examples/object-events/event-test.c | 16 +++++++---------
>  5 files changed, 24 insertions(+), 19 deletions(-)
> 

Everything seems reasonable to me...

Reviewed-by: John Ferlan <jferlan@redhat.com>


John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list