diff mbox series

[v5,04/48] meson: apply target config for picking files from libsystem and libuser

Message ID 20250505015223.3895275-5-pierrick.bouvier@linaro.org
State Superseded
Headers show
Series single-binary: compile target/arm twice | expand

Commit Message

Pierrick Bouvier May 5, 2025, 1:51 a.m. UTC
semihosting code needs to be included only if CONFIG_SEMIHOSTING is set.
However, this is a target configuration, so we need to apply it to the
libsystem libuser source sets.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 meson.build | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

Comments

Richard Henderson May 5, 2025, 4:59 p.m. UTC | #1
On 5/4/25 18:51, Pierrick Bouvier wrote:
> semihosting code needs to be included only if CONFIG_SEMIHOSTING is set.
> However, this is a target configuration, so we need to apply it to the
> libsystem libuser source sets.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   meson.build | 26 ++++++++++++++------------
>   1 file changed, 14 insertions(+), 12 deletions(-)

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

I'm not quite sure how this is supposed to work.  It appears this compiles everything in 
libsystem_ss, and then later selects whether the individual objects should be included in 
the link.  This is fine for internal CONFIG like SEMIHOSTING, but won't be fine for a 
CONFIG that implies external dependencies like VNC.

I guess we can think about externals later, if it becomes an issue.


r~
Pierrick Bouvier May 5, 2025, 5:41 p.m. UTC | #2
On 5/5/25 9:59 AM, Richard Henderson wrote:
> On 5/4/25 18:51, Pierrick Bouvier wrote:
>> semihosting code needs to be included only if CONFIG_SEMIHOSTING is set.
>> However, this is a target configuration, so we need to apply it to the
>> libsystem libuser source sets.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    meson.build | 26 ++++++++++++++------------
>>    1 file changed, 14 insertions(+), 12 deletions(-)
> 
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> 
> I'm not quite sure how this is supposed to work.  It appears this compiles everything in
> libsystem_ss, and then later selects whether the individual objects should be included in
> the link.  This is fine for internal CONFIG like SEMIHOSTING, but won't be fine for a
> CONFIG that implies external dependencies like VNC.
>

The trick used in our build system is that static libraries are never 
fully compiled (no archive is created), but everything is done by 
extracting objects matching sources available after config. It's a bit 
weird, but it works. I understand it was done this way to avoid creating 
specific static libraries per QEMU target.

Before this patch libsystem was including all object files by default 
(thus the link error with --disable-tcg in Philippe series), while now, 
it selects them based on target config, so it's a subset.

In short: Static libraries in QEMU build system are just virtual sets of 
files (sharing flags and dependencies), and only a subset is included in 
each binary based on target config.

> I guess we can think about externals later, if it becomes an issue.
> 

Most of our external dependencies are not set as required, so if no 
object files selected uses it, it should link fine without the 
dependency being present on linker command line.

> 
> r~
>
diff mbox series

Patch

diff --git a/meson.build b/meson.build
index b2c79a7a928..1b365dcae17 100644
--- a/meson.build
+++ b/meson.build
@@ -4049,27 +4049,19 @@  common_ss.add(qom, qemuutil)
 common_ss.add_all(when: 'CONFIG_SYSTEM_ONLY', if_true: [system_ss])
 common_ss.add_all(when: 'CONFIG_USER_ONLY', if_true: user_ss)
 
-libuser_ss = libuser_ss.apply({})
 libuser = static_library('user',
-                         libuser_ss.sources() + genh,
+                         libuser_ss.all_sources() + genh,
                          c_args: ['-DCONFIG_USER_ONLY',
                                   '-DCOMPILING_SYSTEM_VS_USER'],
-                         dependencies: libuser_ss.dependencies(),
+                         dependencies: libuser_ss.all_dependencies(),
                          build_by_default: false)
-libuser = declare_dependency(objects: libuser.extract_all_objects(recursive: false),
-                             dependencies: libuser_ss.dependencies())
-common_ss.add(when: 'CONFIG_USER_ONLY', if_true: libuser)
 
-libsystem_ss = libsystem_ss.apply({})
 libsystem = static_library('system',
-                           libsystem_ss.sources() + genh,
+                           libsystem_ss.all_sources() + genh,
                            c_args: ['-DCONFIG_SOFTMMU',
                                     '-DCOMPILING_SYSTEM_VS_USER'],
-                           dependencies: libsystem_ss.dependencies(),
+                           dependencies: libsystem_ss.all_dependencies(),
                            build_by_default: false)
-libsystem = declare_dependency(objects: libsystem.extract_all_objects(recursive: false),
-                               dependencies: libsystem_ss.dependencies())
-common_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: libsystem)
 
 # Note that this library is never used directly (only through extract_objects)
 # and is not built by default; therefore, source files not used by the build
@@ -4308,6 +4300,16 @@  foreach target : target_dirs
   target_common = common_ss.apply(config_target, strict: false)
   objects = [common_all.extract_objects(target_common.sources())]
   arch_deps += target_common.dependencies()
+  if target_type == 'system'
+    src = libsystem_ss.apply(config_target, strict: false)
+    objects += libsystem.extract_objects(src.sources())
+    arch_deps += src.dependencies()
+  endif
+  if target_type == 'user'
+    src = libuser_ss.apply(config_target, strict: false)
+    objects += libuser.extract_objects(src.sources())
+    arch_deps += src.dependencies()
+  endif
   if target_base_arch in target_common_arch_libs
     src = target_common_arch[target_base_arch].apply(config_target, strict: false)
     lib = target_common_arch_libs[target_base_arch]