Regressions in build process introduced since August

Regressions in build process introduced since August

[Stefan Weil]

Dear all,

yesterday I tried to build new QEMU installers for Windows and noticed 
two regressions which break my build process:

*** Change in handling of --extra-cflags

Running `configure [...] --extra-cflags="-I /xyz"` results in compiler 
flags `-I [...] /xyz`, so the `-I` and `/xyz` are separated by other 
compiler flags which obviously cannot work as expected. I could work 
around that by removing the space and using a pattern like `-I/xyz`.

This regression is not restricted to builds targeting Windows.

*** Setting INSTALLER no longer handled

meson.build sets a hard name for the Windows installer executable: 
installer = 'qemu-setup-' + meson.project_version() + '.exe'.

Previously the installer name could be changed by running `make 
installer INSTALLER=qemu-setup-something.exe`. This no longer works. Is 
there an alternative solution how the name of the installer executable 
can be set? Or how could I reimplement the lost functionality?

Kind regards

Stefan Weil

Re: Regressions in build process introduced since August

[Stefano Garzarella]

CCing Paolo and Marc-André who worked on meson integrations.

On Sun, Nov 15, 2020 at 11:57:25AM +0100, Stefan Weil wrote:
>Dear all,
>
>yesterday I tried to build new QEMU installers for Windows and noticed 
>two regressions which break my build process:
>
>*** Change in handling of --extra-cflags
>
>Running `configure [...] --extra-cflags="-I /xyz"` results in compiler 
>flags `-I [...] /xyz`, so the `-I` and `/xyz` are separated by other 
>compiler flags which obviously cannot work as expected. I could work 
>around that by removing the space and using a pattern like `-I/xyz`.
>
>This regression is not restricted to builds targeting Windows.
>
>*** Setting INSTALLER no longer handled
>
>meson.build sets a hard name for the Windows installer executable: 
>installer = 'qemu-setup-' + meson.project_version() + '.exe'.
>
>Previously the installer name could be changed by running `make 
>installer INSTALLER=qemu-setup-something.exe`. This no longer works. 
>Is there an alternative solution how the name of the installer 
>executable can be set? Or how could I reimplement the lost 
>functionality?
>
>Kind regards
>
>Stefan Weil
>
>
>
>

Re: Regressions in build process introduced since August

[Paolo Bonzini]

On 17/11/20 18:50, Stefano Garzarella wrote:
> Running `configure [...] --extra-cflags="-I /xyz"` results in
> compiler flags `-I [...] /xyz`, so the `-I` and `/xyz` are separated
> by other compiler flags which obviously cannot work as expected. I
> could work around that by removing the space and using a pattern like
> `-I/xyz`.
> 
> This regression is not restricted to builds targeting Windows.

I'll take a look at fixing this (in meson).

> meson.build sets a hard name for the Windows installer executable: 
> installer = 'qemu-setup-' + meson.project_version() + '.exe'.
> 
> Previously the installer name could be changed by running `make 
> installer INSTALLER=qemu-setup-something.exe`. This no longer works.
> Is there an alternative solution how the name of the installer
> executable can be set? Or how could I reimplement the lost
> functionality?

No, there's no way to do this apart from patching meson.build.

Thanks,

Paolo

Re: Regressions in build process introduced since August

[Stefan Weil]

Am 17.11.20 um 19:01 schrieb Paolo Bonzini:

> On 17/11/20 18:50, Stefano Garzarella wrote:
>> Running `configure [...] --extra-cflags="-I /xyz"` results in
>> compiler flags `-I [...] /xyz`, so the `-I` and `/xyz` are separated
>> by other compiler flags which obviously cannot work as expected. I
>> could work around that by removing the space and using a pattern like
>> `-I/xyz`.
>>
>> This regression is not restricted to builds targeting Windows.
>
> I'll take a look at fixing this (in meson).


Thanks. Here is another regression for builds targeting Windows:

Running `../configure --disable-guest-agent-msi [...]` fails with 
"../qga/meson.build:64:4: ERROR: Key QEMU_GA_VERSION is not in dict".

QEMU_GA_VERSION is only set with enabled guest-agent-msi, but currently 
used with enabled guest-agent even when guest-agent-msi is disabled.

Regards,

Stefan

Re: Regressions in build process introduced since August

[Stefan Weil]

Hi,

I just noticed another regression:

A cross build targeting Windows no longer detects VNC JPEG support. 
Using pkg-config like in the patch below would fix that (and also works 
for native builds on Debian GNU Linux).

Maybe the current find_library ignores --extra-cflags and 
--extra-ldflags? That would explain why it fails to find the required 
header and library files. Other checks would fail then, too.

Regards,

Stefan


--- a/meson.build
+++ b/meson.build
@@ -649,9 +649,8 @@ if get_option('vnc').enabled()
    vnc = declare_dependency() # dummy dependency
    png = dependency('libpng', required: get_option('vnc_png'),
                     method: 'pkg-config', static: enable_static)
-  jpeg = cc.find_library('jpeg', has_headers: ['jpeglib.h'],
-                         required: get_option('vnc_jpeg'),
-                         static: enable_static)
+  jpeg = dependency('libpng', required: get_option('vnc_jpeg'),
+                   method: 'pkg-config', static: enable_static)
    sasl = cc.find_library('sasl2', has_headers: ['sasl/sasl.h'],
                           required: get_option('vnc_sasl'),
                           static: enable_static)

Re: Regressions in build process introduced since August

[Stefan Weil]

Am 21.11.20 um 11:25 schrieb Stefan Weil:

> Hi,
>
> I just noticed another regression:
>
> A cross build targeting Windows no longer detects VNC JPEG support. 
> Using pkg-config like in the patch below would fix that (and also 
> works for native builds on Debian GNU Linux).
>
> Maybe the current find_library ignores --extra-cflags and 
> --extra-ldflags? That would explain why it fails to find the required 
> header and library files. Other checks would fail then, too.
>
> Regards,
>
> Stefan
>
>
> --- a/meson.build
> +++ b/meson.build
> @@ -649,9 +649,8 @@ if get_option('vnc').enabled()
>    vnc = declare_dependency() # dummy dependency
>    png = dependency('libpng', required: get_option('vnc_png'),
>                     method: 'pkg-config', static: enable_static)
> -  jpeg = cc.find_library('jpeg', has_headers: ['jpeglib.h'],
> -                         required: get_option('vnc_jpeg'),
> -                         static: enable_static)
> +  jpeg = dependency('libpng', required: get_option('vnc_jpeg'),

Copy+paste error:

s/libpng/libjpeg/

> + method: 'pkg-config', static: enable_static)
>    sasl = cc.find_library('sasl2', has_headers: ['sasl/sasl.h'],
>                           required: get_option('vnc_sasl'),
>                           static: enable_static)

Re: Regressions in build process introduced since August

[Stefan Weil]

One more regression: a lot of build targets are no longer supported with 
the new meson based build.

Personally I am missing `make install-doc` and found no good replacement 
up to now.

Regards,
Stefan

Re: Regressions in build process introduced since August

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 359 bytes --]

Il sab 21 nov 2020, 12:26 Stefan Weil <sw@weilnetz.de> ha scritto:

> One more regression: a lot of build targets are no longer supported with
> the new meson based build.
>
> Personally I am missing `make install-doc` and found no good replacement
> up to now.
>

Just use DESTDIR to install in a staging area, and copy the contents of the
docdir.

Paolo

>

[-- Attachment #2: Type: text/html, Size: 905 bytes --]

Re: Regressions in build process introduced since August

[Paolo Bonzini]

On 21/11/20 12:09, Stefan Weil wrote:
> --- a/meson.build
> +++ b/meson.build
> @@ -649,9 +649,8 @@ if get_option('vnc').enabled()
>     vnc = declare_dependency() # dummy dependency
>     png = dependency('libpng', required: get_option('vnc_png'),
>                      method: 'pkg-config', static: enable_static)
> -  jpeg = cc.find_library('jpeg', has_headers: ['jpeglib.h'],
> -                         required: get_option('vnc_jpeg'),
> -                         static: enable_static)

Does it work if you just remove "static: enable_static"?  That asks 
Meson to look explicitly for a ".a" file instead of just adding a 
"-ljpeg" flag.  However it is not what configure used to do so it 
shouldn't be necessary.

Paolo

Re: Regressions in build process introduced since August

[Stefan Weil]

Am 23.11.20 um 14:55 schrieb Paolo Bonzini:

> On 21/11/20 12:09, Stefan Weil wrote:
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -649,9 +649,8 @@ if get_option('vnc').enabled()
>>     vnc = declare_dependency() # dummy dependency
>>     png = dependency('libpng', required: get_option('vnc_png'),
>>                      method: 'pkg-config', static: enable_static)
>> -  jpeg = cc.find_library('jpeg', has_headers: ['jpeglib.h'],
>> -                         required: get_option('vnc_jpeg'),
>> -                         static: enable_static)
>
> Does it work if you just remove "static: enable_static"?  That asks 
> Meson to look explicitly for a ".a" file instead of just adding a 
> "-ljpeg" flag.  However it is not what configure used to do so it 
> shouldn't be necessary.


No, that does not help. Neither header file nor library will be found in 
my cross environment without using the provided --extra-cflags and 
--extra-ldflags, because all those files are installed below 
/usr/i686-w64-mingw32/sys-root/mingw or 
/usr/x86_64-w64-mingw32/sys-root/mingw which is not searched by the 
default compiler settings. The Meson checks obviously don't use those flags.

pkg-config provides the right paths and works therefore.

Of course I can manually fix my cross environment and move or link all 
header and library files to standard directories, so they will be found. 
But then I'd suggest to remove --extra-cflags and --extra-ldflags 
because they promise something which they don't provide. I think that 
full support for both configure options would be better.

Stefan

Re: Regressions in build process introduced since August

[Paolo Bonzini]

On 23/11/20 15:40, Stefan Weil wrote:
> The Meson checks obviously don't use those flags.

Indeed they don't.  For 0.56 however something like this should be doable:

diff --git a/meson.build b/meson.build
index 5062407c70..25940f8de7 100644
--- a/meson.build
+++ b/meson.build
@@ -107,6 +107,22 @@ 
add_project_arguments(config_host['QEMU_CXXFLAGS'].split(),
  add_project_link_arguments(config_host['QEMU_LDFLAGS'].split(),
                             native: false, language: ['c', 'cpp', 'objc'])

+include_args = []
+library_dirs = []
+foreach arg : config_host['QEMU_CFLAGS'].split() + 
config_host['QEMU_LDFLAGS'].split()
+  if arg.startswith('-I')
+    include_args += arg
+  endif
+  if arg.startswith('-L')
+    library_dirs = arg.substring(2)
+  endif
+endif
+find_library_args = {
+  'static': enable_static,
+  'header_args': include_args,
+  'dirs': library_dirs
+}
+
  if targetos == 'linux'
    add_project_arguments('-isystem', meson.current_source_dir() / 
'linux-headers',
                          '-isystem', 'linux-headers',
  @@ -656,10 +672,10 @@ if get_option('vnc').enabled()
                     method: 'pkg-config', static: enable_static)
    jpeg = cc.find_library('jpeg', has_headers: ['jpeglib.h'],
                           required: get_option('vnc_jpeg'),
-                         static: enable_static)
+                         kwargs: find_library_args)
    sasl = cc.find_library('sasl2', has_headers: ['sasl/sasl.h'],
                           required: get_option('vnc_sasl'),
-                         static: enable_static)
+                         kwargs: find_library_args)
    if sasl.found()
      sasl = declare_dependency(dependencies: sasl,
                                compile_args: '-DSTRUCT_IOVEC_DEFINED')

Paolo

Re: Regressions in build process introduced since August

[Daniel P. Berrangé]

On Mon, Nov 23, 2020 at 03:40:48PM +0100, Stefan Weil wrote:
> Am 23.11.20 um 14:55 schrieb Paolo Bonzini:
> 
> > On 21/11/20 12:09, Stefan Weil wrote:
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -649,9 +649,8 @@ if get_option('vnc').enabled()
> > >     vnc = declare_dependency() # dummy dependency
> > >     png = dependency('libpng', required: get_option('vnc_png'),
> > >                      method: 'pkg-config', static: enable_static)
> > > -  jpeg = cc.find_library('jpeg', has_headers: ['jpeglib.h'],
> > > -                         required: get_option('vnc_jpeg'),
> > > -                         static: enable_static)
> > 
> > Does it work if you just remove "static: enable_static"?  That asks
> > Meson to look explicitly for a ".a" file instead of just adding a
> > "-ljpeg" flag.  However it is not what configure used to do so it
> > shouldn't be necessary.
> 
> 
> No, that does not help. Neither header file nor library will be found in my
> cross environment without using the provided --extra-cflags and
> --extra-ldflags, because all those files are installed below
> /usr/i686-w64-mingw32/sys-root/mingw or
> /usr/x86_64-w64-mingw32/sys-root/mingw which is not searched by the default
> compiler settings. The Meson checks obviously don't use those flags.
> 
> pkg-config provides the right paths and works therefore.

Relying on pkg-config should be our strong preference. I expect we're doing
the old fashioned library check just for historical reasons, but even RHEL-7
supports pkg-config for libjpeg. So I think we're safe to just unconditionally
rely on pkg-config these days and thus have it "just work" for cross compiles
too.


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 :|

Re: Regressions in build process introduced since August

[Daniel P. Berrangé]

On Mon, Nov 23, 2020 at 03:28:39PM +0000, Daniel P. Berrangé wrote:
> On Mon, Nov 23, 2020 at 03:40:48PM +0100, Stefan Weil wrote:
> > Am 23.11.20 um 14:55 schrieb Paolo Bonzini:
> > 
> > > On 21/11/20 12:09, Stefan Weil wrote:
> > > > --- a/meson.build
> > > > +++ b/meson.build
> > > > @@ -649,9 +649,8 @@ if get_option('vnc').enabled()
> > > >     vnc = declare_dependency() # dummy dependency
> > > >     png = dependency('libpng', required: get_option('vnc_png'),
> > > >                      method: 'pkg-config', static: enable_static)
> > > > -  jpeg = cc.find_library('jpeg', has_headers: ['jpeglib.h'],
> > > > -                         required: get_option('vnc_jpeg'),
> > > > -                         static: enable_static)
> > > 
> > > Does it work if you just remove "static: enable_static"?  That asks
> > > Meson to look explicitly for a ".a" file instead of just adding a
> > > "-ljpeg" flag.  However it is not what configure used to do so it
> > > shouldn't be necessary.
> > 
> > 
> > No, that does not help. Neither header file nor library will be found in my
> > cross environment without using the provided --extra-cflags and
> > --extra-ldflags, because all those files are installed below
> > /usr/i686-w64-mingw32/sys-root/mingw or
> > /usr/x86_64-w64-mingw32/sys-root/mingw which is not searched by the default
> > compiler settings. The Meson checks obviously don't use those flags.
> > 
> > pkg-config provides the right paths and works therefore.
> 
> Relying on pkg-config should be our strong preference. I expect we're doing
> the old fashioned library check just for historical reasons, but even RHEL-7
> supports pkg-config for libjpeg. So I think we're safe to just unconditionally
> rely on pkg-config these days and thus have it "just work" for cross compiles
> too.

For that matter I think we can also rely on  pkg-config for sasl too, as
again all our supported platforms have a new enough cyrus-sasl to have
a pkg-config file present these days.

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 :|

Re: Regressions in build process introduced since August

[Paolo Bonzini]

On 23/11/20 16:41, Daniel P. Berrangé wrote:
> On Mon, Nov 23, 2020 at 03:28:39PM +0000, Daniel P. Berrangé wrote:
>> On Mon, Nov 23, 2020 at 03:40:48PM +0100, Stefan Weil wrote:
>>> Am 23.11.20 um 14:55 schrieb Paolo Bonzini:
>>>
>>>> On 21/11/20 12:09, Stefan Weil wrote:
>>>>> --- a/meson.build
>>>>> +++ b/meson.build
>>>>> @@ -649,9 +649,8 @@ if get_option('vnc').enabled()
>>>>>      vnc = declare_dependency() # dummy dependency
>>>>>      png = dependency('libpng', required: get_option('vnc_png'),
>>>>>                       method: 'pkg-config', static: enable_static)
>>>>> -  jpeg = cc.find_library('jpeg', has_headers: ['jpeglib.h'],
>>>>> -                         required: get_option('vnc_jpeg'),
>>>>> -                         static: enable_static)
>>>>
>>>> Does it work if you just remove "static: enable_static"?  That asks
>>>> Meson to look explicitly for a ".a" file instead of just adding a
>>>> "-ljpeg" flag.  However it is not what configure used to do so it
>>>> shouldn't be necessary.
>>>
>>>
>>> No, that does not help. Neither header file nor library will be found in my
>>> cross environment without using the provided --extra-cflags and
>>> --extra-ldflags, because all those files are installed below
>>> /usr/i686-w64-mingw32/sys-root/mingw or
>>> /usr/x86_64-w64-mingw32/sys-root/mingw which is not searched by the default
>>> compiler settings. The Meson checks obviously don't use those flags.
>>>
>>> pkg-config provides the right paths and works therefore.
>>
>> Relying on pkg-config should be our strong preference. I expect we're doing
>> the old fashioned library check just for historical reasons, but even RHEL-7
>> supports pkg-config for libjpeg. So I think we're safe to just unconditionally
>> rely on pkg-config these days and thus have it "just work" for cross compiles
>> too.
> 
> For that matter I think we can also rely on  pkg-config for sasl too, as
> again all our supported platforms have a new enough cyrus-sasl to have
> a pkg-config file present these days.

Yes, I agree.  However we have at least -lmpathpersist that does not 
have a pkg-config file.  In the meanwhile I'll integrate Stefan's patch. 
  Stefan, is libjpeg the only one that is affected in your build 
environment?

Paolo

Re: Regressions in build process introduced since August

[Stefan Weil]

Am 23.11.20 um 17:20 schrieb Paolo Bonzini:

> On 23/11/20 16:41, Daniel P. Berrangé wrote:
>>> Relying on pkg-config should be our strong preference. I expect 
>>> we're doing
>>> the old fashioned library check just for historical reasons, but 
>>> even RHEL-7
>>> supports pkg-config for libjpeg. So I think we're safe to just 
>>> unconditionally
>>> rely on pkg-config these days and thus have it "just work" for cross 
>>> compiles
>>> too.
>>
>> For that matter I think we can also rely on  pkg-config for sasl too, as
>> again all our supported platforms have a new enough cyrus-sasl to have
>> a pkg-config file present these days.
>
> Yes, I agree.  However we have at least -lmpathpersist that does not 
> have a pkg-config file.  In the meanwhile I'll integrate Stefan's 
> patch.  Stefan, is libjpeg the only one that is affected in your build 
> environment?
>
> Paolo
>

Yes, as far as I see currently only libjpeg is affected.

Thanks,
Stefan

Re: Regressions in build process introduced since August

[Daniel P. Berrangé]

On Sun, Nov 15, 2020 at 11:57:25AM +0100, Stefan Weil wrote:
> *** Setting INSTALLER no longer handled
> 
> meson.build sets a hard name for the Windows installer executable: installer
> = 'qemu-setup-' + meson.project_version() + '.exe'.
> 
> Previously the installer name could be changed by running `make installer
> INSTALLER=qemu-setup-something.exe`. This no longer works. Is there an
> alternative solution how the name of the installer executable can be set? Or
> how could I reimplement the lost functionality?

Why were you needing to override the name ?  Was this so that you can
give distinct names to the Win32 vs Win64 installer exes ?

If so, would it be better if QEMU used a distinct filename for the
Win32 vs Win64 installers automatically, to avoid need to override
at all ?


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 :|

Re: Regressions in build process introduced since August

[Stefan Weil]

Am 18.11.20 um 16:34 schrieb Daniel P. Berrangé:

> On Sun, Nov 15, 2020 at 11:57:25AM +0100, Stefan Weil wrote:
>> *** Setting INSTALLER no longer handled
>>
>> meson.build sets a hard name for the Windows installer executable: installer
>> = 'qemu-setup-' + meson.project_version() + '.exe'.
>>
>> Previously the installer name could be changed by running `make installer
>> INSTALLER=qemu-setup-something.exe`. This no longer works. Is there an
>> alternative solution how the name of the installer executable can be set? Or
>> how could I reimplement the lost functionality?
> Why were you needing to override the name ?  Was this so that you can
> give distinct names to the Win32 vs Win64 installer exes ?
>
> If so, would it be better if QEMU used a distinct filename for the
> Win32 vs Win64 installers automatically, to avoid need to override
> at all ?


I now use installer names like qemu-w64-setup-20201118.exe since about 6 
years, so yes, my names include w32 or w64 to show whether they contain 
32 or 64 bit binaries, and they also contain a date instead of the QEMU 
version.

For my build process I now create the default name and rename it later. 
That solves the problem for me.

Stefan