qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: Stefan Weil <sw@weilnetz.de>, Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel <qemu-devel@nongnu.org>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"open list:Block layer core" <qemu-block@nongnu.org>,
	"Kevin Wolf" <kwolf@redhat.com>, "Peter Wu" <peter@lekensteyn.nl>,
	"Julio Faracco" <jcfaracco@gmail.com>,
	"Eli Schwartz" <eschwartz93@gmail.com>
Subject: Re: [PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from <lzfse.h>
Date: Tue, 28 Mar 2023 15:12:27 +0200	[thread overview]
Message-ID: <84824486-6c67-3e78-fd93-91741d9fccff@linaro.org> (raw)
In-Reply-To: <ac4ac6ae-9147-bdb2-5adb-3322182cba4f@weilnetz.de>

On 28/3/23 08:29, Stefan Weil wrote:
> Am 27.03.23 um 23:09 schrieb Paolo Bonzini:
> 
>> Il lun 27 mar 2023, 20:58 Philippe Mathieu-Daudé <philmd@linaro.org> 
>> ha scritto:
>>
>>     > The warning can also be suppressed if the build uses `-isystem
>>     > /opt/homebrew/include` instead of `-I/opt/homebrew/include` as I
>>     just
>>     > have tested.
>>
>> Is that option added by QEMU's configure or meson.build? Or is it 
>> added by homebrew? The fact that /opt/homebrew/include it isn't 
>> considered a system seems to be a homebrew decision.
>>
>>     IIUC by design meson only allows including *relative* directories,
>>     and manage the system ones:
>>     https://mesonbuild.com/Include-directories.html
>>
>> That's for includes that are part of QEMU.
>>
>> Meson has as_system for dependency objects 
>> (https://mesonbuild.com/Reference-manual_returned_dep.html) but lzfse 
>> doesn't have a .pc file, its detection has to be done by hand.
>>
>> Paolo
>>
>>     > If we can find a solution how to implement that I thing it would
>>     look
>>     > nicer. Technically the patch looks good.
>>     >
>>     > Reviewed-by: Stefan Weil <sw@weilnetz.de>
>>
>>     Thanks!
>>
> 
> Typically I configure the build on macOS with `./configure 
> --extra-cflags=-I/opt/homebrew/include 
> --extra-ldflags=-L/opt/homebrew/lib --disable-werror`. With that 
> configuration I get the two warnings for lzfse.h.
> 
> When I use `./configure '--extra-cflags=-isystem /opt/homebrew/include' 
> --extra-ldflags=-L/opt/homebrew/lib --disable-werror` instead, I get no 
> compiler warnings (and `--disable-werror` could be ommitted).
> 
> So at least for macOS with Homebrew in /opt/homebrew (M1 / M2 Macs) the 
> patch is not needed when the right configure options (`--extra-cflags`) 
> were used.

What I learned:

- If lzfse were well packaged (as noted Paolo), we could use
   dependency(..., include_type: 'system')
   https://github.com/mesonbuild/meson/issues/963#issuecomment-1277851401

- I agree with Eli Schwartz, a Meson maintainer:

   > More bugs caused by and only affecting people who misuse and
   > abuse -isystem as "not system, but as a side effect please
   > don't emit warnings for this messy dependency"...

   https://github.com/mesonbuild/meson/issues/8755#issuecomment-836913759

I wasted few hours on this, and am now giving up in favor of this simple
patch.


  reply	other threads:[~2023-03-28 13:12 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-27 15:13 [PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from <lzfse.h> Philippe Mathieu-Daudé
2023-03-27 17:08 ` Stefan Weil via
2023-03-27 18:58   ` Philippe Mathieu-Daudé
2023-03-27 21:09     ` Paolo Bonzini
2023-03-28  6:29       ` Stefan Weil via
2023-03-28 13:12         ` Philippe Mathieu-Daudé [this message]
2023-03-30 14:26 ` Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=84824486-6c67-3e78-fd93-91741d9fccff@linaro.org \
    --to=philmd@linaro.org \
    --cc=eschwartz93@gmail.com \
    --cc=hreitz@redhat.com \
    --cc=jcfaracco@gmail.com \
    --cc=kwolf@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter@lekensteyn.nl \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=sw@weilnetz.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).