[PATCH 0/3] build: some cleanups for common-user/ infrastructure

[PATCH 0/3] build: some cleanups for common-user/ infrastructure

[Paolo Bonzini]

A belated review of the patches that introduced common-user/.
Limiting the interface that it expects from bsd-user/ and linux-user/
to an include/ subdirectory, and simplifying the mesonry that builds
the new directory.

Paolo Bonzini (3):
  meson: reuse common_user_inc when building files specific to user-mode
    emulators
  user: move common-user includes to a subdirectory of {bsd,linux}-user/
  meson: cleanup common-user/ build

 bsd-user/{ => include}/special-errno.h            |  0
 bsd-user/meson.build                              |  2 +-
 common-user/meson.build                           |  2 +-
 .../{ => include}/host/aarch64/host-signal.h      |  0
 linux-user/{ => include}/host/alpha/host-signal.h |  0
 linux-user/{ => include}/host/arm/host-signal.h   |  0
 linux-user/{ => include}/host/i386/host-signal.h  |  0
 linux-user/{ => include}/host/mips/host-signal.h  |  0
 linux-user/{ => include}/host/ppc/host-signal.h   |  0
 linux-user/{ => include}/host/ppc64/host-signal.h |  0
 linux-user/{ => include}/host/riscv/host-signal.h |  0
 linux-user/{ => include}/host/s390/host-signal.h  |  0
 linux-user/{ => include}/host/s390x/host-signal.h |  0
 linux-user/{ => include}/host/sparc/host-signal.h |  0
 .../{ => include}/host/sparc64/host-signal.h      |  0
 linux-user/{ => include}/host/x32/host-signal.h   |  0
 .../{ => include}/host/x86_64/host-signal.h       |  0
 linux-user/{ => include}/special-errno.h          |  0
 linux-user/meson.build                            |  4 ++--
 meson.build                                       | 15 ++-------------
 20 files changed, 6 insertions(+), 17 deletions(-)
 rename bsd-user/{ => include}/special-errno.h (100%)
 rename linux-user/{ => include}/host/aarch64/host-signal.h (100%)
 rename linux-user/{ => include}/host/alpha/host-signal.h (100%)
 rename linux-user/{ => include}/host/arm/host-signal.h (100%)
 rename linux-user/{ => include}/host/i386/host-signal.h (100%)
 rename linux-user/{ => include}/host/mips/host-signal.h (100%)
 rename linux-user/{ => include}/host/ppc/host-signal.h (100%)
 rename linux-user/{ => include}/host/ppc64/host-signal.h (100%)
 rename linux-user/{ => include}/host/riscv/host-signal.h (100%)
 rename linux-user/{ => include}/host/s390/host-signal.h (100%)
 rename linux-user/{ => include}/host/s390x/host-signal.h (100%)
 rename linux-user/{ => include}/host/sparc/host-signal.h (100%)
 rename linux-user/{ => include}/host/sparc64/host-signal.h (100%)
 rename linux-user/{ => include}/host/x32/host-signal.h (100%)
 rename linux-user/{ => include}/host/x86_64/host-signal.h (100%)
 rename linux-user/{ => include}/special-errno.h (100%)

-- 
2.33.1

[PATCH 1/3] meson: reuse common_user_inc when building files specific to user-mode emulators

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index f45ecf31bd..b0af02b805 100644
--- a/meson.build
+++ b/meson.build
@@ -2897,6 +2897,7 @@ foreach target : target_dirs
   else
     abi = config_target['TARGET_ABI_DIR']
     target_type='user'
+    target_inc += common_user_inc
     qemu_target_name = 'qemu-' + target_name
     if target_base_arch in target_user_arch
       t = target_user_arch[target_base_arch].apply(config_target, strict: false)
@@ -2905,7 +2906,6 @@ foreach target : target_dirs
     endif
     if 'CONFIG_LINUX_USER' in config_target
       base_dir = 'linux-user'
-      target_inc += include_directories('linux-user/host/' / host_arch)
     endif
     if 'CONFIG_BSD_USER' in config_target
       base_dir = 'bsd-user'
-- 
2.33.1

Re: [PATCH 1/3] meson: reuse common_user_inc when building files specific to user-mode emulators

[Richard Henderson]

On 12/21/21 8:32 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   meson.build | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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

r~

[PATCH 2/3] user: move common-user includes to a subdirectory of {bsd, linux}-user/

[Paolo Bonzini]

Avoid polluting the compilation of common-user/ with local include files;
making an include file available to common-user/ should be a deliberate
decision in order to keep a clear interface that can be used by both
bsd-user/ and linux-user/.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 bsd-user/{ => include}/special-errno.h              | 0
 bsd-user/meson.build                                | 2 +-
 linux-user/{ => include}/host/aarch64/host-signal.h | 0
 linux-user/{ => include}/host/alpha/host-signal.h   | 0
 linux-user/{ => include}/host/arm/host-signal.h     | 0
 linux-user/{ => include}/host/i386/host-signal.h    | 0
 linux-user/{ => include}/host/mips/host-signal.h    | 0
 linux-user/{ => include}/host/ppc/host-signal.h     | 0
 linux-user/{ => include}/host/ppc64/host-signal.h   | 0
 linux-user/{ => include}/host/riscv/host-signal.h   | 0
 linux-user/{ => include}/host/s390/host-signal.h    | 0
 linux-user/{ => include}/host/s390x/host-signal.h   | 0
 linux-user/{ => include}/host/sparc/host-signal.h   | 0
 linux-user/{ => include}/host/sparc64/host-signal.h | 0
 linux-user/{ => include}/host/x32/host-signal.h     | 0
 linux-user/{ => include}/host/x86_64/host-signal.h  | 0
 linux-user/{ => include}/special-errno.h            | 0
 linux-user/meson.build                              | 4 ++--
 18 files changed, 3 insertions(+), 3 deletions(-)
 rename bsd-user/{ => include}/special-errno.h (100%)
 rename linux-user/{ => include}/host/aarch64/host-signal.h (100%)
 rename linux-user/{ => include}/host/alpha/host-signal.h (100%)
 rename linux-user/{ => include}/host/arm/host-signal.h (100%)
 rename linux-user/{ => include}/host/i386/host-signal.h (100%)
 rename linux-user/{ => include}/host/mips/host-signal.h (100%)
 rename linux-user/{ => include}/host/ppc/host-signal.h (100%)
 rename linux-user/{ => include}/host/ppc64/host-signal.h (100%)
 rename linux-user/{ => include}/host/riscv/host-signal.h (100%)
 rename linux-user/{ => include}/host/s390/host-signal.h (100%)
 rename linux-user/{ => include}/host/s390x/host-signal.h (100%)
 rename linux-user/{ => include}/host/sparc/host-signal.h (100%)
 rename linux-user/{ => include}/host/sparc64/host-signal.h (100%)
 rename linux-user/{ => include}/host/x32/host-signal.h (100%)
 rename linux-user/{ => include}/host/x86_64/host-signal.h (100%)
 rename linux-user/{ => include}/special-errno.h (100%)

diff --git a/bsd-user/special-errno.h b/bsd-user/include/special-errno.h
similarity index 100%
rename from bsd-user/special-errno.h
rename to bsd-user/include/special-errno.h
diff --git a/bsd-user/meson.build b/bsd-user/meson.build
index 9fcb80c3fa..8380fa44c2 100644
--- a/bsd-user/meson.build
+++ b/bsd-user/meson.build
@@ -4,7 +4,7 @@ endif
 
 bsd_user_ss = ss.source_set()
 
-common_user_inc += include_directories('.')
+common_user_inc += include_directories('include')
 
 bsd_user_ss.add(files(
   'bsdload.c',
diff --git a/linux-user/host/aarch64/host-signal.h b/linux-user/include/host/aarch64/host-signal.h
similarity index 100%
rename from linux-user/host/aarch64/host-signal.h
rename to linux-user/include/host/aarch64/host-signal.h
diff --git a/linux-user/host/alpha/host-signal.h b/linux-user/include/host/alpha/host-signal.h
similarity index 100%
rename from linux-user/host/alpha/host-signal.h
rename to linux-user/include/host/alpha/host-signal.h
diff --git a/linux-user/host/arm/host-signal.h b/linux-user/include/host/arm/host-signal.h
similarity index 100%
rename from linux-user/host/arm/host-signal.h
rename to linux-user/include/host/arm/host-signal.h
diff --git a/linux-user/host/i386/host-signal.h b/linux-user/include/host/i386/host-signal.h
similarity index 100%
rename from linux-user/host/i386/host-signal.h
rename to linux-user/include/host/i386/host-signal.h
diff --git a/linux-user/host/mips/host-signal.h b/linux-user/include/host/mips/host-signal.h
similarity index 100%
rename from linux-user/host/mips/host-signal.h
rename to linux-user/include/host/mips/host-signal.h
diff --git a/linux-user/host/ppc/host-signal.h b/linux-user/include/host/ppc/host-signal.h
similarity index 100%
rename from linux-user/host/ppc/host-signal.h
rename to linux-user/include/host/ppc/host-signal.h
diff --git a/linux-user/host/ppc64/host-signal.h b/linux-user/include/host/ppc64/host-signal.h
similarity index 100%
rename from linux-user/host/ppc64/host-signal.h
rename to linux-user/include/host/ppc64/host-signal.h
diff --git a/linux-user/host/riscv/host-signal.h b/linux-user/include/host/riscv/host-signal.h
similarity index 100%
rename from linux-user/host/riscv/host-signal.h
rename to linux-user/include/host/riscv/host-signal.h
diff --git a/linux-user/host/s390/host-signal.h b/linux-user/include/host/s390/host-signal.h
similarity index 100%
rename from linux-user/host/s390/host-signal.h
rename to linux-user/include/host/s390/host-signal.h
diff --git a/linux-user/host/s390x/host-signal.h b/linux-user/include/host/s390x/host-signal.h
similarity index 100%
rename from linux-user/host/s390x/host-signal.h
rename to linux-user/include/host/s390x/host-signal.h
diff --git a/linux-user/host/sparc/host-signal.h b/linux-user/include/host/sparc/host-signal.h
similarity index 100%
rename from linux-user/host/sparc/host-signal.h
rename to linux-user/include/host/sparc/host-signal.h
diff --git a/linux-user/host/sparc64/host-signal.h b/linux-user/include/host/sparc64/host-signal.h
similarity index 100%
rename from linux-user/host/sparc64/host-signal.h
rename to linux-user/include/host/sparc64/host-signal.h
diff --git a/linux-user/host/x32/host-signal.h b/linux-user/include/host/x32/host-signal.h
similarity index 100%
rename from linux-user/host/x32/host-signal.h
rename to linux-user/include/host/x32/host-signal.h
diff --git a/linux-user/host/x86_64/host-signal.h b/linux-user/include/host/x86_64/host-signal.h
similarity index 100%
rename from linux-user/host/x86_64/host-signal.h
rename to linux-user/include/host/x86_64/host-signal.h
diff --git a/linux-user/special-errno.h b/linux-user/include/special-errno.h
similarity index 100%
rename from linux-user/special-errno.h
rename to linux-user/include/special-errno.h
diff --git a/linux-user/meson.build b/linux-user/meson.build
index b2f4afd5e7..de4320af05 100644
--- a/linux-user/meson.build
+++ b/linux-user/meson.build
@@ -4,8 +4,8 @@ endif
 
 linux_user_ss = ss.source_set()
 
-common_user_inc += include_directories('host/' / host_arch)
-common_user_inc += include_directories('.')
+common_user_inc += include_directories('include/host/' / host_arch)
+common_user_inc += include_directories('include')
 
 linux_user_ss.add(files(
   'elfload.c',
-- 
2.33.1

Re: [PATCH 2/3] user: move common-user includes to a subdirectory of {bsd,linux}-user/

[Richard Henderson]

On 12/21/21 8:32 AM, Paolo Bonzini wrote:
> Avoid polluting the compilation of common-user/ with local include files;
> making an include file available to common-user/ should be a deliberate
> decision in order to keep a clear interface that can be used by both
> bsd-user/ and linux-user/.

The reason that I did not do this before is that very shortly we're going to have 
bsd-user/host/arch/host-signal.h too.

If we combine them into a top-level include like this, then we have to add host-specific 
ifdefs.  IMO it is cleaner to leave them separate.

If you really really want to move them out of <os>-user/include/host, then the only other 
thing I can suggest is include/host/<os>/<arch>/.


r~

Re: [PATCH 2/3] user: move common-user includes to a subdirectory of {bsd,linux}-user/

[Richard Henderson]

On 12/21/21 11:56 AM, Richard Henderson wrote:
> On 12/21/21 8:32 AM, Paolo Bonzini wrote:
>> Avoid polluting the compilation of common-user/ with local include files;
>> making an include file available to common-user/ should be a deliberate
>> decision in order to keep a clear interface that can be used by both
>> bsd-user/ and linux-user/.
> 
> The reason that I did not do this before is that very shortly we're going to have 
> bsd-user/host/arch/host-signal.h too.
> 
> If we combine them into a top-level include like this, then we have to add host-specific 
> ifdefs.  IMO it is cleaner to leave them separate.
> 
> If you really really want to move them out of <os>-user/include/host, then the only other 
> thing I can suggest is include/host/<os>/<arch>/.

Bah. Nevermind, I now see it's not top-level.


r~

Re: [PATCH 2/3] user: move common-user includes to a subdirectory of {bsd,linux}-user/

[Richard Henderson]

On 12/21/21 8:32 AM, Paolo Bonzini wrote:
> Avoid polluting the compilation of common-user/ with local include files;
> making an include file available to common-user/ should be a deliberate
> decision in order to keep a clear interface that can be used by both
> bsd-user/ and linux-user/.
> 
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   bsd-user/{ => include}/special-errno.h              | 0
>   bsd-user/meson.build                                | 2 +-
>   linux-user/{ => include}/host/aarch64/host-signal.h | 0
>   linux-user/{ => include}/host/alpha/host-signal.h   | 0
>   linux-user/{ => include}/host/arm/host-signal.h     | 0
>   linux-user/{ => include}/host/i386/host-signal.h    | 0
>   linux-user/{ => include}/host/mips/host-signal.h    | 0
>   linux-user/{ => include}/host/ppc/host-signal.h     | 0
>   linux-user/{ => include}/host/ppc64/host-signal.h   | 0
>   linux-user/{ => include}/host/riscv/host-signal.h   | 0
>   linux-user/{ => include}/host/s390/host-signal.h    | 0
>   linux-user/{ => include}/host/s390x/host-signal.h   | 0
>   linux-user/{ => include}/host/sparc/host-signal.h   | 0
>   linux-user/{ => include}/host/sparc64/host-signal.h | 0
>   linux-user/{ => include}/host/x32/host-signal.h     | 0
>   linux-user/{ => include}/host/x86_64/host-signal.h  | 0
>   linux-user/{ => include}/special-errno.h            | 0
>   linux-user/meson.build                              | 4 ++--
>   18 files changed, 3 insertions(+), 3 deletions(-)
>   rename bsd-user/{ => include}/special-errno.h (100%)
>   rename linux-user/{ => include}/host/aarch64/host-signal.h (100%)
>   rename linux-user/{ => include}/host/alpha/host-signal.h (100%)
>   rename linux-user/{ => include}/host/arm/host-signal.h (100%)
>   rename linux-user/{ => include}/host/i386/host-signal.h (100%)
>   rename linux-user/{ => include}/host/mips/host-signal.h (100%)
>   rename linux-user/{ => include}/host/ppc/host-signal.h (100%)
>   rename linux-user/{ => include}/host/ppc64/host-signal.h (100%)
>   rename linux-user/{ => include}/host/riscv/host-signal.h (100%)
>   rename linux-user/{ => include}/host/s390/host-signal.h (100%)
>   rename linux-user/{ => include}/host/s390x/host-signal.h (100%)
>   rename linux-user/{ => include}/host/sparc/host-signal.h (100%)
>   rename linux-user/{ => include}/host/sparc64/host-signal.h (100%)
>   rename linux-user/{ => include}/host/x32/host-signal.h (100%)
>   rename linux-user/{ => include}/host/x86_64/host-signal.h (100%)
>   rename linux-user/{ => include}/special-errno.h (100%)

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

r~

[PATCH 3/3] meson: cleanup common-user/ build

[Paolo Bonzini]

Do not go through a static_library, which is only necessary in order to reuse
some source files between emulators and tests.  Instead just place common files
for all user-mode emulators in common_ss, similar to what is already done for
softmmu_ss in full system emulators.

The only disadvantage is that the include_directories under bsd-user/include/
and linux-user/include/ are now enabled for all targets rather than only
user mode emulators.  This however is not different from how include/sysemu/
is available when building user mode emulators.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 common-user/meson.build |  2 +-
 meson.build             | 13 +------------
 2 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/common-user/meson.build b/common-user/meson.build
index 5cb42bc664..26212dda5c 100644
--- a/common-user/meson.build
+++ b/common-user/meson.build
@@ -1,6 +1,6 @@
 common_user_inc += include_directories('host/' / host_arch)
 
-common_user_ss.add(files(
+user_ss.add(files(
   'safe-syscall.S',
   'safe-syscall-error.c',
 ))
diff --git a/meson.build b/meson.build
index b0af02b805..879628ab68 100644
--- a/meson.build
+++ b/meson.build
@@ -2377,7 +2377,6 @@ blockdev_ss = ss.source_set()
 block_ss = ss.source_set()
 chardev_ss = ss.source_set()
 common_ss = ss.source_set()
-common_user_ss = ss.source_set()
 crypto_ss = ss.source_set()
 hwcore_ss = ss.source_set()
 io_ss = ss.source_set()
@@ -2629,17 +2628,6 @@ subdir('common-user')
 subdir('bsd-user')
 subdir('linux-user')
 
-common_user_ss = common_user_ss.apply(config_all, strict: false)
-common_user = static_library('common-user',
-                             sources: common_user_ss.sources(),
-                             dependencies: common_user_ss.dependencies(),
-                             include_directories: common_user_inc,
-                             name_suffix: 'fa',
-                             build_by_default: false)
-common_user = declare_dependency(link_with: common_user)
-
-user_ss.add(common_user)
-
 # needed for fuzzing binaries
 subdir('tests/qtest/libqos')
 subdir('tests/qtest/fuzz')
@@ -2857,6 +2845,7 @@ common_all = common_ss.apply(config_all, strict: false)
 common_all = static_library('common',
                             build_by_default: false,
                             sources: common_all.sources() + genh,
+                            include_directories: common_user_inc,
                             implicit_include_directories: false,
                             dependencies: common_all.dependencies(),
                             name_suffix: 'fa')
-- 
2.33.1

Re: [PATCH 3/3] meson: cleanup common-user/ build

[Richard Henderson]

On 12/21/21 8:33 AM, Paolo Bonzini wrote:
> Do not go through a static_library, which is only necessary in order to reuse
> some source files between emulators and tests.

That's not true, is it.  Anyway, you're moving the file to a different static_library.  I 
though I had tried this myself, but I don't remember the details now.

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


r~

Re: [PATCH 3/3] meson: cleanup common-user/ build

[Paolo Bonzini]

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

Il mar 21 dic 2021, 21:32 Richard Henderson <richard.henderson@linaro.org>
ha scritto:

> On 12/21/21 8:33 AM, Paolo Bonzini wrote:
> > Do not go through a static_library, which is only necessary in order to
> reuse
> > some source files between emulators and tests.
>
> That's not true, is it.  Anyway, you're moving the file to a different
> static_library.  I
> though I had tried this myself, but I don't remember the details now.
>

Right, I will change the commit message to "a static_library just for
common_user files, since the one that covers all of common_ss is enough".

Paolo


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

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