[PATCH for-7.0] meson.build: Fix dependency of page-vary-common.c to config-poison.h

Thomas Huth posted 1 patch 3 years, 10 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220330114808.942933-1-thuth@redhat.com
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH for-7.0] meson.build: Fix dependency of page-vary-common.c to config-poison.h
Posted by Thomas Huth 3 years, 10 months ago
Before compiling page-vary-common.c, we have to make sure that
config-poison.h has been generated (which is in the "genh" list).

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/948
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index aef724ad3c..04ce33fef1 100644
--- a/meson.build
+++ b/meson.build
@@ -2881,7 +2881,7 @@ if get_option('b_lto')
   if get_option('cfi')
     pagevary_flags += '-fno-sanitize=cfi-icall'
   endif
-  pagevary = static_library('page-vary-common', sources: pagevary,
+  pagevary = static_library('page-vary-common', sources: pagevary + genh,
                             c_args: pagevary_flags)
   pagevary = declare_dependency(link_with: pagevary)
 endif
-- 
2.27.0
Re: [PATCH for-7.0] meson.build: Fix dependency of page-vary-common.c to config-poison.h
Posted by Philippe Mathieu-Daudé 3 years, 10 months ago
Hi Thomas,

On 30/3/22 13:48, Thomas Huth wrote:
> Before compiling page-vary-common.c, we have to make sure that
> config-poison.h has been generated (which is in the "genh" list).

I am a bit confused, "config-poison.h" is include by "exec/poison.h"
which is included by "qemu/osdep.h" for all non-softmmu code (tools,
common and -user).

Why is pagevary specific? Shouldn't add genh to all common_ss[]?

Maybe the problem is how common_all[] is created?

> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/948
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   meson.build | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index aef724ad3c..04ce33fef1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2881,7 +2881,7 @@ if get_option('b_lto')
>     if get_option('cfi')
>       pagevary_flags += '-fno-sanitize=cfi-icall'
>     endif
> -  pagevary = static_library('page-vary-common', sources: pagevary,
> +  pagevary = static_library('page-vary-common', sources: pagevary + genh,
>                               c_args: pagevary_flags)
>     pagevary = declare_dependency(link_with: pagevary)
>   endif
Re: [PATCH for-7.0] meson.build: Fix dependency of page-vary-common.c to config-poison.h
Posted by Paolo Bonzini 3 years, 10 months ago
On 3/30/22 14:05, Philippe Mathieu-Daudé wrote:
> Shouldn't add genh to all common_ss[]?
> 
> Maybe the problem is how common_all[] is created?

Generated headers have to be added to all sourcesets, or to all targets.

common_ss is only used to build the object files in libcommon.fa.p, but 
libcommon.fa also includes many other .fa build targets (via 
"dependencies:").  All those build targets also need to have genh in 
their build target as well, otherwise their object files might be built 
before the generated headers.

(The other question then is why we have libauthz, libcrypto, 
libmigration, etc. instead of just using the sourcesets.  The answer for 
that is that in Meson the static_library is the tool for object file 
reuse.  If you include the same source file in many different executable 
it is recompiled.  Therefore, if you used sourcesets, you would build 
the object files once for each testsuite program that uses them).

Paolo

Re: [PATCH for-7.0] meson.build: Fix dependency of page-vary-common.c to config-poison.h
Posted by Thomas Huth 3 years, 10 months ago
On 30/03/2022 14.05, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 30/3/22 13:48, Thomas Huth wrote:
>> Before compiling page-vary-common.c, we have to make sure that
>> config-poison.h has been generated (which is in the "genh" list).
> 
> I am a bit confused, "config-poison.h" is include by "exec/poison.h"
> which is included by "qemu/osdep.h" for all non-softmmu code (tools,
> common and -user).
> 
> Why is pagevary specific?

pagevary is certainly not specific here, we're doing this
all over the place in meson.build:

$ grep -r static_library.*genh meson.build
   libmodulecommon = static_library('module-common', files('module-common.c') + genh, pic: true, c_args: '-DBUILD_DSO')
   pagevary = static_library('page-vary-common', sources: pagevary + genh,
       sl = static_library(d + '-' + m, [genh, module_ss.sources()],
libqom = static_library('qom', qom_ss.sources() + genh,
libauthz = static_library('authz', authz_ss.sources() + genh,
libcrypto = static_library('crypto', crypto_ss.sources() + genh,
libio = static_library('io', io_ss.sources() + genh,
libmigration = static_library('migration', sources: migration_files + genh,
libblock = static_library('block', block_ss.sources() + genh,
libblockdev = static_library('blockdev', blockdev_ss.sources() + genh,
libqmp = static_library('qmp', qmp_ss.sources() + genh,
libchardev = static_library('chardev', chardev_ss.sources() + genh,
libhwcore = static_library('hwcore', sources: hwcore_ss.sources() + genh,

> Shouldn't add genh to all common_ss[]?
> 
> Maybe the problem is how common_all[] is created?

Maybe there is a better way to handle these dependencies ...
but that's rather a bigger change and thus something for
the 7.1 cycle, I think.

  Thomas


>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/948
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   meson.build | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/meson.build b/meson.build
>> index aef724ad3c..04ce33fef1 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -2881,7 +2881,7 @@ if get_option('b_lto')
>>     if get_option('cfi')
>>       pagevary_flags += '-fno-sanitize=cfi-icall'
>>     endif
>> -  pagevary = static_library('page-vary-common', sources: pagevary,
>> +  pagevary = static_library('page-vary-common', sources: pagevary + genh,
>>                               c_args: pagevary_flags)
>>     pagevary = declare_dependency(link_with: pagevary)
>>   endif
> 


Re: [PATCH for-7.0] meson.build: Fix dependency of page-vary-common.c to config-poison.h
Posted by Richard Henderson 3 years, 10 months ago
On 3/30/22 05:48, Thomas Huth wrote:
> Before compiling page-vary-common.c, we have to make sure that
> config-poison.h has been generated (which is in the "genh" list).
> 
> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/948
> Signed-off-by: Thomas Huth<thuth@redhat.com>
> ---
>   meson.build | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~