diff mbox series

meson: use config_base_arch for target libraries

Message ID 20250602233801.2699961-1-pierrick.bouvier@linaro.org
State New
Headers show
Series meson: use config_base_arch for target libraries | expand

Commit Message

Pierrick Bouvier June 2, 2025, 11:38 p.m. UTC
Fixed commit introduced common dependencies for target libraries. Alas,
it wrongly reused the 'target' variable, which was previously set from
another loop.

Thus, some dependencies were missing depending on order of target list,
as found here [1].

The fix is to use the correct config_base_arch instead.
Kudos to Thomas Huth who had this right, before I reimplement it, and
introduce this bug.

[1] https://lore.kernel.org/qemu-devel/c54469ce-0385-4aea-b345-47711e9e61de@linaro.org/

Fixes: 4fb54de823e9 (meson: build target libraries with common dependencies)
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 meson.build | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Pierrick Bouvier June 2, 2025, 11:41 p.m. UTC | #1
On 6/2/25 4:38 PM, Pierrick Bouvier wrote:
> Fixed commit introduced common dependencies for target libraries. Alas,
> it wrongly reused the 'target' variable, which was previously set from
> another loop.
> 
> Thus, some dependencies were missing depending on order of target list,
> as found here [1].
> 
> The fix is to use the correct config_base_arch instead.
> Kudos to Thomas Huth who had this right, before I reimplement it, and
> introduce this bug.
> 
> [1] https://lore.kernel.org/qemu-devel/c54469ce-0385-4aea-b345-47711e9e61de@linaro.org/
> 
> Fixes: 4fb54de823e9 (meson: build target libraries with common dependencies)
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   meson.build | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 2df89006f8b..ad9cef99ed9 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -4142,13 +4142,12 @@ common_all = static_library('common',
>   target_common_arch_libs = {}
>   target_common_system_arch_libs = {}
>   foreach target_base_arch, config_base_arch : config_base_arch_mak
> -  config_target = config_target_mak[target]
>     target_inc = [include_directories('target' / target_base_arch)]
>     inc = [common_user_inc + target_inc]
>   
> -  target_common = common_ss.apply(config_target, strict: false)
> -  target_system = system_ss.apply(config_target, strict: false)
> -  target_user = user_ss.apply(config_target, strict: false)
> +  target_common = common_ss.apply(config_base_arch, strict: false)
> +  target_system = system_ss.apply(config_base_arch, strict: false)
> +  target_user = user_ss.apply(config_base_arch, strict: false)
>     common_deps = []
>     system_deps = []
>     user_deps = []

As arm targets can be impacted by the build bug fixed here, it would be 
nice if this could be merged upstream quickly.

Thanks,
Pierrick
Thomas Huth June 3, 2025, 6:05 a.m. UTC | #2
On 03/06/2025 01.38, Pierrick Bouvier wrote:
> Fixed commit introduced common dependencies for target libraries. Alas,
> it wrongly reused the 'target' variable, which was previously set from
> another loop.
> 
> Thus, some dependencies were missing depending on order of target list,
> as found here [1].
> 
> The fix is to use the correct config_base_arch instead.
> Kudos to Thomas Huth who had this right, before I reimplement it, and

s/Thomas Huth/Paolo Bonzini/ ... it was his patch indeed, I just was the one 
who sent it as a proper patch to the mailing list.

> introduce this bug.
> 
> [1] https://lore.kernel.org/qemu-devel/c54469ce-0385-4aea-b345-47711e9e61de@linaro.org/
> 
> Fixes: 4fb54de823e9 (meson: build target libraries with common dependencies)
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   meson.build | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 2df89006f8b..ad9cef99ed9 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -4142,13 +4142,12 @@ common_all = static_library('common',
>   target_common_arch_libs = {}
>   target_common_system_arch_libs = {}
>   foreach target_base_arch, config_base_arch : config_base_arch_mak
> -  config_target = config_target_mak[target]
>     target_inc = [include_directories('target' / target_base_arch)]
>     inc = [common_user_inc + target_inc]
>   
> -  target_common = common_ss.apply(config_target, strict: false)
> -  target_system = system_ss.apply(config_target, strict: false)
> -  target_user = user_ss.apply(config_target, strict: false)
> +  target_common = common_ss.apply(config_base_arch, strict: false)
> +  target_system = system_ss.apply(config_base_arch, strict: false)
> +  target_user = user_ss.apply(config_base_arch, strict: false)
>     common_deps = []
>     system_deps = []
>     user_deps = []

With the patch description updated:
Reviewed-by: Thomas Huth <thuth@redhat.com>
Cédric Le Goater June 3, 2025, 7:12 a.m. UTC | #3
On 6/3/25 01:38, Pierrick Bouvier wrote:
> Fixed commit introduced common dependencies for target libraries. Alas,
> it wrongly reused the 'target' variable, which was previously set from
> another loop.
> 
> Thus, some dependencies were missing depending on order of target list,
> as found here [1].
> 
> The fix is to use the correct config_base_arch instead.
> Kudos to Thomas Huth who had this right, before I reimplement it, and
> introduce this bug.
> 
> [1] https://lore.kernel.org/qemu-devel/c54469ce-0385-4aea-b345-47711e9e61de@linaro.org/
> 
> Fixes: 4fb54de823e9 (meson: build target libraries with common dependencies)
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>


Tested-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   meson.build | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 2df89006f8b..ad9cef99ed9 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -4142,13 +4142,12 @@ common_all = static_library('common',
>   target_common_arch_libs = {}
>   target_common_system_arch_libs = {}
>   foreach target_base_arch, config_base_arch : config_base_arch_mak
> -  config_target = config_target_mak[target]
>     target_inc = [include_directories('target' / target_base_arch)]
>     inc = [common_user_inc + target_inc]
>   
> -  target_common = common_ss.apply(config_target, strict: false)
> -  target_system = system_ss.apply(config_target, strict: false)
> -  target_user = user_ss.apply(config_target, strict: false)
> +  target_common = common_ss.apply(config_base_arch, strict: false)
> +  target_system = system_ss.apply(config_base_arch, strict: false)
> +  target_user = user_ss.apply(config_base_arch, strict: false)
>     common_deps = []
>     system_deps = []
>     user_deps = []
Paolo Bonzini June 3, 2025, 2:51 p.m. UTC | #4
Queued, thanks.

Paolo
diff mbox series

Patch

diff --git a/meson.build b/meson.build
index 2df89006f8b..ad9cef99ed9 100644
--- a/meson.build
+++ b/meson.build
@@ -4142,13 +4142,12 @@  common_all = static_library('common',
 target_common_arch_libs = {}
 target_common_system_arch_libs = {}
 foreach target_base_arch, config_base_arch : config_base_arch_mak
-  config_target = config_target_mak[target]
   target_inc = [include_directories('target' / target_base_arch)]
   inc = [common_user_inc + target_inc]
 
-  target_common = common_ss.apply(config_target, strict: false)
-  target_system = system_ss.apply(config_target, strict: false)
-  target_user = user_ss.apply(config_target, strict: false)
+  target_common = common_ss.apply(config_base_arch, strict: false)
+  target_system = system_ss.apply(config_base_arch, strict: false)
+  target_user = user_ss.apply(config_base_arch, strict: false)
   common_deps = []
   system_deps = []
   user_deps = []