[PATCH v2 0/4] meson: Fixes for dbus modules

[PATCH v2 0/4] meson: Fixes for dbus modules

[Akihiko Odaki]

I found it was failing to build dbus modules when --enable-dbus so here
are fixes.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Changes in v2:
- Updated MAINTAINERS.
- Link to v1: https://lore.kernel.org/r/20231215-dbus-v1-0-349e059ac5b2@daynix.com

---
Akihiko Odaki (4):
      Move dbus-display1 out of ui
      audio: Depend on dbus_display1_dep
      meson: Explicitly specify dbus-display1.h dependency
      tests/qtest: Depend on dbus_display1_dep

 MAINTAINERS                               |  1 +
 meson.build                               | 21 +++++++++++++++++++++
 ui/dbus.h                                 |  2 +-
 audio/dbusaudio.c                         |  2 +-
 tests/qtest/dbus-display-test.c           |  2 +-
 audio/meson.build                         |  3 ++-
 ui/dbus-display1.xml => dbus-display1.xml |  0
 tests/qtest/meson.build                   |  2 +-
 ui/meson.build                            | 18 ------------------
 9 files changed, 28 insertions(+), 23 deletions(-)
---
base-commit: 4705fc0c8511d073bee4751c3c974aab2b10a970
change-id: 20231215-dbus-86876ecb7b09

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>

[PATCH v2 1/4] Move dbus-display1 out of ui

[Akihiko Odaki]

Despite its name, dbus-display1 does not only provide DBus interfaces
for the display but also for the audio.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 MAINTAINERS                               |  1 +
 meson.build                               | 21 +++++++++++++++++++++
 ui/dbus.h                                 |  2 +-
 audio/dbusaudio.c                         |  2 +-
 tests/qtest/dbus-display-test.c           |  2 +-
 ui/dbus-display1.xml => dbus-display1.xml |  0
 ui/meson.build                            | 18 ------------------
 7 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 695e0bd34fbb..0c2630d0e264 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3389,6 +3389,7 @@ F: docs/sphinx/dbus*
 F: docs/sphinx/fakedbusdoc.py
 F: tests/qtest/dbus*
 F: scripts/xml-preprocess*
+F: dbus-display.xml
 
 Seccomp
 M: Daniel P. Berrange <berrange@redhat.com>
diff --git a/meson.build b/meson.build
index ec01f8b138aa..5e1b25a47184 100644
--- a/meson.build
+++ b/meson.build
@@ -2014,6 +2014,27 @@ dbus_display = get_option('dbus_display') \
            error_message: gdbus_codegen_error.format('-display dbus')) \
   .allowed()
 
+if dbus_display
+  env = environment()
+  env.set('TARGETOS', targetos)
+  xml = custom_target('dbus-display preprocess',
+                      input: 'dbus-display1.xml',
+                      output: 'dbus-display1.xml',
+                      env: env,
+                      command: [xml_pp, '@INPUT@', '@OUTPUT@'])
+  dbus_display1 = custom_target('dbus-display gdbus-codegen',
+                                output: ['dbus-display1.h', 'dbus-display1.c'],
+                                input: xml,
+                                command: [gdbus_codegen, '@INPUT@',
+                                          '--glib-min-required', '2.64',
+                                          '--output-directory', meson.current_build_dir(),
+                                          '--interface-prefix', 'org.qemu.',
+                                          '--c-namespace', 'QemuDBus',
+                                          '--generate-c-code', '@BASENAME@'])
+  dbus_display1_lib = static_library('dbus-display1', dbus_display1, dependencies: gio)
+  dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib, include_directories: include_directories('.'))
+endif
+
 have_virtfs = get_option('virtfs') \
     .require(targetos == 'linux' or targetos == 'darwin',
              error_message: 'virtio-9p (virtfs) requires Linux or macOS') \
diff --git a/ui/dbus.h b/ui/dbus.h
index 1e8c24a48e32..ba9bab9f84d9 100644
--- a/ui/dbus.h
+++ b/ui/dbus.h
@@ -31,7 +31,7 @@
 #include "ui/console.h"
 #include "ui/clipboard.h"
 
-#include "ui/dbus-display1.h"
+#include "dbus-display1.h"
 
 typedef struct DBusClipboardRequest {
     GDBusMethodInvocation *invocation;
diff --git a/audio/dbusaudio.c b/audio/dbusaudio.c
index 60fcf643ecf8..5222b3c68804 100644
--- a/audio/dbusaudio.c
+++ b/audio/dbusaudio.c
@@ -34,7 +34,7 @@
 #endif
 
 #include "ui/dbus.h"
-#include "ui/dbus-display1.h"
+#include "dbus-display1.h"
 
 #define AUDIO_CAP "dbus"
 #include "audio.h"
diff --git a/tests/qtest/dbus-display-test.c b/tests/qtest/dbus-display-test.c
index 21edaa1e321f..a15e9c377b08 100644
--- a/tests/qtest/dbus-display-test.c
+++ b/tests/qtest/dbus-display-test.c
@@ -5,7 +5,7 @@
 #include <gio/gio.h>
 #include <gio/gunixfdlist.h>
 #include "libqtest.h"
-#include "ui/dbus-display1.h"
+#include "dbus-display1.h"
 
 static GDBusConnection*
 test_dbus_p2p_from_fd(int fd)
diff --git a/ui/dbus-display1.xml b/dbus-display1.xml
similarity index 100%
rename from ui/dbus-display1.xml
rename to dbus-display1.xml
diff --git a/ui/meson.build b/ui/meson.build
index 0ccb3387ee6a..bbb7c5242d55 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -75,24 +75,6 @@ endif
 
 if dbus_display
   dbus_ss = ss.source_set()
-  env = environment()
-  env.set('TARGETOS', targetos)
-  xml = custom_target('dbus-display preprocess',
-                      input: 'dbus-display1.xml',
-                      output: 'dbus-display1.xml',
-                      env: env,
-                      command: [xml_pp, '@INPUT@', '@OUTPUT@'])
-  dbus_display1 = custom_target('dbus-display gdbus-codegen',
-                                output: ['dbus-display1.h', 'dbus-display1.c'],
-                                input: xml,
-                                command: [gdbus_codegen, '@INPUT@',
-                                          '--glib-min-required', '2.64',
-                                          '--output-directory', meson.current_build_dir(),
-                                          '--interface-prefix', 'org.qemu.',
-                                          '--c-namespace', 'QemuDBus',
-                                          '--generate-c-code', '@BASENAME@'])
-  dbus_display1_lib = static_library('dbus-display1', dbus_display1, dependencies: gio)
-  dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib, include_directories: include_directories('.'))
   dbus_ss.add(when: [gio, dbus_display1_dep],
               if_true: [files(
                 'dbus-chardev.c',

-- 
2.43.0

Re: [PATCH v2 1/4] Move dbus-display1 out of ui

[Marc-André Lureau]

Hi

On Fri, Dec 15, 2023 at 3:29 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> Despite its name, dbus-display1 does not only provide DBus interfaces
> for the display but also for the audio.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

It can actually do a lot more, since you can arbitrarily redirect
chardev devices.. USB, smartcard, QMP and what's not are possible.

Yet, this is a -display module, so we should leave it under ui/ for now imho

> ---
>  MAINTAINERS                               |  1 +
>  meson.build                               | 21 +++++++++++++++++++++
>  ui/dbus.h                                 |  2 +-
>  audio/dbusaudio.c                         |  2 +-
>  tests/qtest/dbus-display-test.c           |  2 +-
>  ui/dbus-display1.xml => dbus-display1.xml |  0
>  ui/meson.build                            | 18 ------------------
>  7 files changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 695e0bd34fbb..0c2630d0e264 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3389,6 +3389,7 @@ F: docs/sphinx/dbus*
>  F: docs/sphinx/fakedbusdoc.py
>  F: tests/qtest/dbus*
>  F: scripts/xml-preprocess*
> +F: dbus-display.xml
>
>  Seccomp
>  M: Daniel P. Berrange <berrange@redhat.com>
> diff --git a/meson.build b/meson.build
> index ec01f8b138aa..5e1b25a47184 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2014,6 +2014,27 @@ dbus_display = get_option('dbus_display') \
>             error_message: gdbus_codegen_error.format('-display dbus')) \
>    .allowed()
>
> +if dbus_display
> +  env = environment()
> +  env.set('TARGETOS', targetos)
> +  xml = custom_target('dbus-display preprocess',
> +                      input: 'dbus-display1.xml',
> +                      output: 'dbus-display1.xml',
> +                      env: env,
> +                      command: [xml_pp, '@INPUT@', '@OUTPUT@'])
> +  dbus_display1 = custom_target('dbus-display gdbus-codegen',
> +                                output: ['dbus-display1.h', 'dbus-display1.c'],
> +                                input: xml,
> +                                command: [gdbus_codegen, '@INPUT@',
> +                                          '--glib-min-required', '2.64',
> +                                          '--output-directory', meson.current_build_dir(),
> +                                          '--interface-prefix', 'org.qemu.',
> +                                          '--c-namespace', 'QemuDBus',
> +                                          '--generate-c-code', '@BASENAME@'])
> +  dbus_display1_lib = static_library('dbus-display1', dbus_display1, dependencies: gio)
> +  dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib, include_directories: include_directories('.'))
> +endif
> +
>  have_virtfs = get_option('virtfs') \
>      .require(targetos == 'linux' or targetos == 'darwin',
>               error_message: 'virtio-9p (virtfs) requires Linux or macOS') \
> diff --git a/ui/dbus.h b/ui/dbus.h
> index 1e8c24a48e32..ba9bab9f84d9 100644
> --- a/ui/dbus.h
> +++ b/ui/dbus.h
> @@ -31,7 +31,7 @@
>  #include "ui/console.h"
>  #include "ui/clipboard.h"
>
> -#include "ui/dbus-display1.h"
> +#include "dbus-display1.h"
>
>  typedef struct DBusClipboardRequest {
>      GDBusMethodInvocation *invocation;
> diff --git a/audio/dbusaudio.c b/audio/dbusaudio.c
> index 60fcf643ecf8..5222b3c68804 100644
> --- a/audio/dbusaudio.c
> +++ b/audio/dbusaudio.c
> @@ -34,7 +34,7 @@
>  #endif
>
>  #include "ui/dbus.h"
> -#include "ui/dbus-display1.h"
> +#include "dbus-display1.h"
>
>  #define AUDIO_CAP "dbus"
>  #include "audio.h"
> diff --git a/tests/qtest/dbus-display-test.c b/tests/qtest/dbus-display-test.c
> index 21edaa1e321f..a15e9c377b08 100644
> --- a/tests/qtest/dbus-display-test.c
> +++ b/tests/qtest/dbus-display-test.c
> @@ -5,7 +5,7 @@
>  #include <gio/gio.h>
>  #include <gio/gunixfdlist.h>
>  #include "libqtest.h"
> -#include "ui/dbus-display1.h"
> +#include "dbus-display1.h"
>
>  static GDBusConnection*
>  test_dbus_p2p_from_fd(int fd)
> diff --git a/ui/dbus-display1.xml b/dbus-display1.xml
> similarity index 100%
> rename from ui/dbus-display1.xml
> rename to dbus-display1.xml
> diff --git a/ui/meson.build b/ui/meson.build
> index 0ccb3387ee6a..bbb7c5242d55 100644
> --- a/ui/meson.build
> +++ b/ui/meson.build
> @@ -75,24 +75,6 @@ endif
>
>  if dbus_display
>    dbus_ss = ss.source_set()
> -  env = environment()
> -  env.set('TARGETOS', targetos)
> -  xml = custom_target('dbus-display preprocess',
> -                      input: 'dbus-display1.xml',
> -                      output: 'dbus-display1.xml',
> -                      env: env,
> -                      command: [xml_pp, '@INPUT@', '@OUTPUT@'])
> -  dbus_display1 = custom_target('dbus-display gdbus-codegen',
> -                                output: ['dbus-display1.h', 'dbus-display1.c'],
> -                                input: xml,
> -                                command: [gdbus_codegen, '@INPUT@',
> -                                          '--glib-min-required', '2.64',
> -                                          '--output-directory', meson.current_build_dir(),
> -                                          '--interface-prefix', 'org.qemu.',
> -                                          '--c-namespace', 'QemuDBus',
> -                                          '--generate-c-code', '@BASENAME@'])
> -  dbus_display1_lib = static_library('dbus-display1', dbus_display1, dependencies: gio)
> -  dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib, include_directories: include_directories('.'))
>    dbus_ss.add(when: [gio, dbus_display1_dep],
>                if_true: [files(
>                  'dbus-chardev.c',
>
> --
> 2.43.0
>
>


-- 
Marc-André Lureau

Re: [PATCH v2 1/4] Move dbus-display1 out of ui

[Akihiko Odaki]

On 2023/12/15 21:00, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Dec 15, 2023 at 3:29 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>
>> Despite its name, dbus-display1 does not only provide DBus interfaces
>> for the display but also for the audio.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> 
> It can actually do a lot more, since you can arbitrarily redirect
> chardev devices.. USB, smartcard, QMP and what's not are possible.
> 
> Yet, this is a -display module, so we should leave it under ui/ for now imho

I'm making this change specifically for audio. audio/dbusaudio.c has no 
dependency on the display despite it's part of the dbus-display1 interface.

By the way, I noticed audio/dbusaudio.c includes ui/dbus.h for no reason 
so I removed the inclusion in v3. It clarifies audio/dbus is independent 
of ui/dbus.

Regards,
Akihiko Odaki

[PATCH v2 2/4] audio: Depend on dbus_display1_dep

[Akihiko Odaki]

dbusaudio needs dbus_display1_dep.

Fixes: 739362d4205c ("audio: add "dbus" audio backend")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 audio/meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/audio/meson.build b/audio/meson.build
index c8f658611f42..608f35e6af78 100644
--- a/audio/meson.build
+++ b/audio/meson.build
@@ -30,7 +30,8 @@ endforeach
 
 if dbus_display
     module_ss = ss.source_set()
-    module_ss.add(when: [gio, pixman], if_true: files('dbusaudio.c'))
+    module_ss.add(when: [gio, dbus_display1_dep, pixman],
+                  if_true: files('dbusaudio.c'))
     audio_modules += {'dbus': module_ss}
 endif
 

-- 
2.43.0

Re: [PATCH v2 2/4] audio: Depend on dbus_display1_dep

[Marc-André Lureau]

Hi

On Fri, Dec 15, 2023 at 3:29 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> dbusaudio needs dbus_display1_dep.
>
> Fixes: 739362d4205c ("audio: add "dbus" audio backend")

The build didn't fail, did it? You don't need to backport this, thus I
don't think "Fixes" apply here.

(furthermore, dbus_display1_dep was added later, in commit 1222070e7
"meson: ensure dbus-display generated code is built before other
units")

> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

otherwise, lgtm
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  audio/meson.build | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/audio/meson.build b/audio/meson.build
> index c8f658611f42..608f35e6af78 100644
> --- a/audio/meson.build
> +++ b/audio/meson.build
> @@ -30,7 +30,8 @@ endforeach
>
>  if dbus_display
>      module_ss = ss.source_set()
> -    module_ss.add(when: [gio, pixman], if_true: files('dbusaudio.c'))
> +    module_ss.add(when: [gio, dbus_display1_dep, pixman],
> +                  if_true: files('dbusaudio.c'))
>      audio_modules += {'dbus': module_ss}
>  endif
>
>
> --
> 2.43.0
>
>


-- 
Marc-André Lureau

[PATCH v2 3/4] meson: Explicitly specify dbus-display1.h dependency

[Akihiko Odaki]

Explicitly specify dbus-display1.h as a dependency so that files
depending on it will not get compiled too early.

Fixes: 1222070e7728 ("meson: ensure dbus-display generated code is built before other units")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 5e1b25a47184..c31168e11cfb 100644
--- a/meson.build
+++ b/meson.build
@@ -2032,7 +2032,7 @@ if dbus_display
                                           '--c-namespace', 'QemuDBus',
                                           '--generate-c-code', '@BASENAME@'])
   dbus_display1_lib = static_library('dbus-display1', dbus_display1, dependencies: gio)
-  dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib, include_directories: include_directories('.'))
+  dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib, sources: dbus_display1[0])
 endif
 
 have_virtfs = get_option('virtfs') \

-- 
2.43.0

Re: [PATCH v2 3/4] meson: Explicitly specify dbus-display1.h dependency

[Marc-André Lureau]

Hi

On Fri, Dec 15, 2023 at 3:29 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> Explicitly specify dbus-display1.h as a dependency so that files
> depending on it will not get compiled too early.
>
> Fixes: 1222070e7728 ("meson: ensure dbus-display generated code is built before other units")
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

ok, hopefully this finally fixes the problem.. I have the feeling the
solution applied in this commit isn't the right one..

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index 5e1b25a47184..c31168e11cfb 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2032,7 +2032,7 @@ if dbus_display
>                                            '--c-namespace', 'QemuDBus',
>                                            '--generate-c-code', '@BASENAME@'])
>    dbus_display1_lib = static_library('dbus-display1', dbus_display1, dependencies: gio)
> -  dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib, include_directories: include_directories('.'))
> +  dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib, sources: dbus_display1[0])
>  endif
>
>  have_virtfs = get_option('virtfs') \
>
> --
> 2.43.0
>
>


-- 
Marc-André Lureau

[PATCH v2 4/4] tests/qtest: Depend on dbus_display1_dep

[Akihiko Odaki]

It ensures dbus-display1.c will not be recompiled.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 tests/qtest/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 47dabf91d048..3a1a79d7c72e 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -332,7 +332,7 @@ if vnc.found()
 endif
 
 if dbus_display
-  qtests += {'dbus-display-test': [dbus_display1, gio]}
+  qtests += {'dbus-display-test': [dbus_display1_dep, gio]}
 endif
 
 qtest_executables = {}

-- 
2.43.0

Re: [PATCH v2 4/4] tests/qtest: Depend on dbus_display1_dep

[Marc-André Lureau]

On Fri, Dec 15, 2023 at 3:29 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> It ensures dbus-display1.c will not be recompiled.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  tests/qtest/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 47dabf91d048..3a1a79d7c72e 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -332,7 +332,7 @@ if vnc.found()
>  endif
>
>  if dbus_display
> -  qtests += {'dbus-display-test': [dbus_display1, gio]}
> +  qtests += {'dbus-display-test': [dbus_display1_dep, gio]}
>  endif
>
>  qtest_executables = {}
>
> --
> 2.43.0
>
>


-- 
Marc-André Lureau

Re: [PATCH v2 0/4] meson: Fixes for dbus modules

[Marc-André Lureau]

Hi Akihiko

On Fri, Dec 15, 2023 at 3:29 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> I found it was failing to build dbus modules when --enable-dbus so here
> are fixes.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> Changes in v2:
> - Updated MAINTAINERS.
> - Link to v1: https://lore.kernel.org/r/20231215-dbus-v1-0-349e059ac5b2@daynix.com
>
> ---
> Akihiko Odaki (4):
>       Move dbus-display1 out of ui
>       audio: Depend on dbus_display1_dep
>       meson: Explicitly specify dbus-display1.h dependency
>       tests/qtest: Depend on dbus_display1_dep

Can you send a v4 which doesn't move the dbus-display.xml out of
display/ but has the rest of the dependency fixes and r-b?

thanks

>
>  MAINTAINERS                               |  1 +
>  meson.build                               | 21 +++++++++++++++++++++
>  ui/dbus.h                                 |  2 +-
>  audio/dbusaudio.c                         |  2 +-
>  tests/qtest/dbus-display-test.c           |  2 +-
>  audio/meson.build                         |  3 ++-
>  ui/dbus-display1.xml => dbus-display1.xml |  0
>  tests/qtest/meson.build                   |  2 +-
>  ui/meson.build                            | 18 ------------------
>  9 files changed, 28 insertions(+), 23 deletions(-)
> ---
> base-commit: 4705fc0c8511d073bee4751c3c974aab2b10a970
> change-id: 20231215-dbus-86876ecb7b09
>
> Best regards,
> --
> Akihiko Odaki <akihiko.odaki@daynix.com>
>
>


-- 
Marc-André Lureau