[PATCH 1/2] system: Restrict libSDL CPPFLAGS to vl.c

Philippe Mathieu-Daudé posted 2 patches 5 months ago
[PATCH 1/2] system: Restrict libSDL CPPFLAGS to vl.c
Posted by Philippe Mathieu-Daudé 5 months ago
Only vl.c includes libSDL headers.
No need to pass them to all system_ss[] files.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 system/meson.build | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/system/meson.build b/system/meson.build
index 4952f4b2c7d..f7e2c8b826f 100644
--- a/system/meson.build
+++ b/system/meson.build
@@ -23,8 +23,11 @@ system_ss.add(files(
   'runstate-hmp-cmds.c',
   'runstate.c',
   'tpm-hmp-cmds.c',
+), libpmem, libdaxctl)
+
+system_ss.add(files(
   'vl.c',
-), sdl, libpmem, libdaxctl)
+), sdl)
 
 if have_tpm
   system_ss.add(files('tpm.c'))
-- 
2.45.2


Re: [PATCH 1/2] system: Restrict libSDL CPPFLAGS to vl.c
Posted by Richard Henderson 5 months ago
On 12/12/24 03:26, Philippe Mathieu-Daudé wrote:
> Only vl.c includes libSDL headers.
> No need to pass them to all system_ss[] files.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   system/meson.build | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/system/meson.build b/system/meson.build
> index 4952f4b2c7d..f7e2c8b826f 100644
> --- a/system/meson.build
> +++ b/system/meson.build
> @@ -23,8 +23,11 @@ system_ss.add(files(
>     'runstate-hmp-cmds.c',
>     'runstate.c',
>     'tpm-hmp-cmds.c',
> +), libpmem, libdaxctl)
> +
> +system_ss.add(files(
>     'vl.c',
> -), sdl, libpmem, libdaxctl)
> +), sdl)
>   
>   if have_tpm
>     system_ss.add(files('tpm.c'))

I'm sure Paolo will correct me, but I don't think this does what you think it does.  I 
believe this has no change at all.

The presence of sdl within a *particular* source_set.add() call is immaterial.  If the 
condition is true (and here, because the condition is missing it is true), all of the 
files and dependencies get lumped together.  In the end, everything gets copied into 
common_ss.

Both of these patches only alters the order of the items in the link ordering.


r~



Re: [PATCH 1/2] system: Restrict libSDL CPPFLAGS to vl.c
Posted by Paolo Bonzini 5 months ago
On 12/12/24 14:18, Richard Henderson wrote:
> On 12/12/24 03:26, Philippe Mathieu-Daudé wrote:
>> Only vl.c includes libSDL headers.
>> No need to pass them to all system_ss[] files.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   system/meson.build | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/system/meson.build b/system/meson.build
>> index 4952f4b2c7d..f7e2c8b826f 100644
>> --- a/system/meson.build
>> +++ b/system/meson.build
>> @@ -23,8 +23,11 @@ system_ss.add(files(
>>     'runstate-hmp-cmds.c',
>>     'runstate.c',
>>     'tpm-hmp-cmds.c',
>> +), libpmem, libdaxctl)
>> +
>> +system_ss.add(files(
>>     'vl.c',
>> -), sdl, libpmem, libdaxctl)
>> +), sdl)
>>   if have_tpm
>>     system_ss.add(files('tpm.c'))
> 
> I'm sure Paolo will correct me, but I don't think this does what you 
> think it does.  I believe this has no change at all.

No need to correct anything! :)

> The presence of sdl within a *particular* source_set.add() call is 
> immaterial.  If the condition is true (and here, because the condition 
> is missing it is true), all of the files and dependencies get lumped 
> together.  In the end, everything gets copied into common_ss.

Yep.  You can have different flags, including library CFLAGS/CPPFLAGS, 
for specific vs system (the former ends up in the libqemu-*.a.p 
directory, vs libcommon.a.p for system), or for those sourcesets that 
are used also by user-mode emulation (IIRC libhwcore.a.p) or by tests 
(the various directories libio.a.p, libmigration.a.p).  However, here 
there is no effect at all.

If you want to do this as a cleanup to keep the libraries close to their 
users, I have no objection, but please adjust the commit message and 
squash the two patches together.

Paolo

> Both of these patches only alters the order of the items in the link 
> ordering.
> 
> 
> r~
> 
> 
>