1. Patchew
  2. QEMU
  3. View series

[PATCH v6] util: Move dbus_display1 to util

Akihiko Odaki posted 1 patch 9 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240214-dbus-v6-1-411af2b9289e@daynix.com
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
MAINTAINERS                     |  2 +-
docs/interop/dbus-display.rst   |  6 +++---
ui/dbus.h                       |  2 +-
audio/dbusaudio.c               |  2 +-
tests/qtest/dbus-display-test.c |  2 +-
tests/qtest/meson.build         |  2 +-
ui/meson.build                  | 20 +-------------------
{ui => util}/dbus-display1.xml  |  0
util/meson.build                | 21 +++++++++++++++++++++
9 files changed, 30 insertions(+), 27 deletions(-)
[PATCH v6] util: Move dbus_display1 to util
Posted by Akihiko Odaki 9 months, 2 weeks ago 
Move dbus_display1 from ui to util as it's used not only by ui/dbus but
also dbus-display-test.

dbus_display1 is now added to util_ss accordingly. It ensures that the
source will be linked with audio/dbus, and also avoids recompilation
when linking with dbus-display-test.

dbus-display.h is also added to genh to ensure it is generated before
compiling ui/dbus, audio/dbus, and dbus-display-test.

Both changes combined, it is no longer necessary for ui/dbus,
audio/dbus, and dbus-display-test to explicitly state the dependency on
dbus_display1.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
I found it was failing to build dbus modules when --enable-dbus so here
are fixes.
---
Changes in v6:
- Dropped patch "audio: Do not include ui/dbus.h" (Marc-André Lureau).
- Rebased.
- Link to v5: https://lore.kernel.org/r/20231217-dbus-v5-0-8122e822a392@daynix.com

Changes in v5:
- Fixed docs/interop/dbus-display.rst.
- Link to v4: https://lore.kernel.org/r/20231217-dbus-v4-0-4fd5410bf04f@daynix.com

Changes in v4:
- Moved dbus_display1 to util.
- Link to v3: https://lore.kernel.org/r/20231216-dbus-v3-0-b4bcbed7374a@daynix.com

Changes in v3:
- Merged dbus_display1_lib into libqemuutil.
- Added patch "audio: Do not include ui/dbus.h".
- Link to v2: https://lore.kernel.org/r/20231215-dbus-v2-0-1e2e6aa02115@daynix.com

Changes in v2:
- Updated MAINTAINERS.
- Link to v1: https://lore.kernel.org/r/20231215-dbus-v1-0-349e059ac5b2@daynix.com
---
 MAINTAINERS                     |  2 +-
 docs/interop/dbus-display.rst   |  6 +++---
 ui/dbus.h                       |  2 +-
 audio/dbusaudio.c               |  2 +-
 tests/qtest/dbus-display-test.c |  2 +-
 tests/qtest/meson.build         |  2 +-
 ui/meson.build                  | 20 +-------------------
 {ui => util}/dbus-display1.xml  |  0
 util/meson.build                | 21 +++++++++++++++++++++
 9 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f80db6a96a3f..0a81159e33d1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3413,7 +3413,7 @@ S: Maintained
 F: backends/dbus-vmstate.c
 F: ui/dbus*
 F: audio/dbus*
-F: util/dbus.c
+F: util/dbus*
 F: include/ui/dbus*
 F: include/qemu/dbus.h
 F: docs/interop/dbus*
diff --git a/docs/interop/dbus-display.rst b/docs/interop/dbus-display.rst
index 8c6e8e0f5a82..efec89723a34 100644
--- a/docs/interop/dbus-display.rst
+++ b/docs/interop/dbus-display.rst
@@ -18,14 +18,14 @@ QEMU also implements the standard interfaces, such as
 
 .. only:: sphinx4
 
-   .. dbus-doc:: ui/dbus-display1.xml
+   .. dbus-doc:: util/dbus-display1.xml
 
 .. only:: not sphinx4
 
    .. warning::
       Sphinx 4 is required to build D-Bus documentation.
 
-      This is the content of ``ui/dbus-display1.xml``:
+      This is the content of ``util/dbus-display1.xml``:
 
-   .. literalinclude:: ../../ui/dbus-display1.xml
+   .. literalinclude:: ../../util/dbus-display1.xml
       :language: xml
diff --git a/ui/dbus.h b/ui/dbus.h
index 1e8c24a48e32..a847bee98876 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 "util/dbus-display1.h"
 
 typedef struct DBusClipboardRequest {
     GDBusMethodInvocation *invocation;
diff --git a/audio/dbusaudio.c b/audio/dbusaudio.c
index 60fcf643ecf8..2aacdac6715b 100644
--- a/audio/dbusaudio.c
+++ b/audio/dbusaudio.c
@@ -34,7 +34,7 @@
 #endif
 
 #include "ui/dbus.h"
-#include "ui/dbus-display1.h"
+#include "util/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..d4871e2fd80f 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 "util/dbus-display1.h"
 
 static GDBusConnection*
 test_dbus_p2p_from_fd(int fd)
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 39557d5ecbb0..627ff8fbe1c7 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -344,7 +344,7 @@ if vnc.found()
 endif
 
 if dbus_display
-  qtests += {'dbus-display-test': [dbus_display1, gio]}
+  qtests += {'dbus-display-test': [gio]}
 endif
 
 qtest_executables = {}
diff --git a/ui/meson.build b/ui/meson.build
index 376e0d771ba9..74e2a79b8c1e 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -74,25 +74,7 @@ endif
 
 if dbus_display
   dbus_ss = ss.source_set()
-  env = environment()
-  env.set('HOST_OS', host_os)
-  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],
+  dbus_ss.add(when: [gio],
               if_true: [files(
                 'dbus-chardev.c',
                 'dbus-clipboard.c',
diff --git a/ui/dbus-display1.xml b/util/dbus-display1.xml
similarity index 100%
rename from ui/dbus-display1.xml
rename to util/dbus-display1.xml
diff --git a/util/meson.build b/util/meson.build
index 0ef9886be048..d23c8382fa37 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -128,3 +128,24 @@ elif cpu == 'loongarch64'
 elif cpu in ['ppc', 'ppc64']
   util_ss.add(files('cpuinfo-ppc.c'))
 endif
+
+if dbus_display
+  env = environment()
+  env.set('HOST_OS', host_os)
+  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@'])
+  util_ss.add(dbus_display1)
+  genh += dbus_display1[0]
+endif

---
base-commit: 5005aed8a7e728d028efb40e243ecfc2b4f3df3a
change-id: 20231215-dbus-86876ecb7b09

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>
Re: [PATCH v6] util: Move dbus_display1 to util
Posted by Marc-André Lureau 9 months, 2 weeks ago 
Hi Akihiko

On Wed, Feb 14, 2024 at 9:39 AM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> Move dbus_display1 from ui to util as it's used not only by ui/dbus but
> also dbus-display-test.

It doesn't seem like the right place either. So let's focus on what
this is fixing.

> dbus_display1 is now added to util_ss accordingly. It ensures that the
> source will be linked with audio/dbus, and also avoids recompilation
> when linking with dbus-display-test.

- "It ensures that the source will be linked with audio/dbus"
Right, this could be achieved with: module_ss.add(when: [gio, pixman],
if_true: [files('dbusaudio.c'), dbus_display1_dep])

- "avoids recompilation when linking with dbus-display-test"
Similarly: qtests += {'dbus-display-test': [dbus_display1_dep, gio]}

>
> dbus-display.h is also added to genh to ensure it is generated before
> compiling ui/dbus, audio/dbus, and dbus-display-test.
>

meson should take care of those dependencies if they are expressed correctly.

> Both changes combined, it is no longer necessary for ui/dbus,
> audio/dbus, and dbus-display-test to explicitly state the dependency on
> dbus_display1.

This is eventually nice for things that are really common, but in this
case I don't think it's a win.

>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> I found it was failing to build dbus modules when --enable-dbus so here
> are fixes.
> ---
> Changes in v6:
> - Dropped patch "audio: Do not include ui/dbus.h" (Marc-André Lureau).
> - Rebased.
> - Link to v5: https://lore.kernel.org/r/20231217-dbus-v5-0-8122e822a392@daynix.com
>
> Changes in v5:
> - Fixed docs/interop/dbus-display.rst.
> - Link to v4: https://lore.kernel.org/r/20231217-dbus-v4-0-4fd5410bf04f@daynix.com
>
> Changes in v4:
> - Moved dbus_display1 to util.
> - Link to v3: https://lore.kernel.org/r/20231216-dbus-v3-0-b4bcbed7374a@daynix.com
>
> Changes in v3:
> - Merged dbus_display1_lib into libqemuutil.
> - Added patch "audio: Do not include ui/dbus.h".
> - Link to v2: https://lore.kernel.org/r/20231215-dbus-v2-0-1e2e6aa02115@daynix.com
>
> Changes in v2:
> - Updated MAINTAINERS.
> - Link to v1: https://lore.kernel.org/r/20231215-dbus-v1-0-349e059ac5b2@daynix.com
> ---
>  MAINTAINERS                     |  2 +-
>  docs/interop/dbus-display.rst   |  6 +++---
>  ui/dbus.h                       |  2 +-
>  audio/dbusaudio.c               |  2 +-
>  tests/qtest/dbus-display-test.c |  2 +-
>  tests/qtest/meson.build         |  2 +-
>  ui/meson.build                  | 20 +-------------------
>  {ui => util}/dbus-display1.xml  |  0
>  util/meson.build                | 21 +++++++++++++++++++++
>  9 files changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f80db6a96a3f..0a81159e33d1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3413,7 +3413,7 @@ S: Maintained
>  F: backends/dbus-vmstate.c
>  F: ui/dbus*
>  F: audio/dbus*
> -F: util/dbus.c
> +F: util/dbus*
>  F: include/ui/dbus*
>  F: include/qemu/dbus.h
>  F: docs/interop/dbus*
> diff --git a/docs/interop/dbus-display.rst b/docs/interop/dbus-display.rst
> index 8c6e8e0f5a82..efec89723a34 100644
> --- a/docs/interop/dbus-display.rst
> +++ b/docs/interop/dbus-display.rst
> @@ -18,14 +18,14 @@ QEMU also implements the standard interfaces, such as
>
>  .. only:: sphinx4
>
> -   .. dbus-doc:: ui/dbus-display1.xml
> +   .. dbus-doc:: util/dbus-display1.xml
>
>  .. only:: not sphinx4
>
>     .. warning::
>        Sphinx 4 is required to build D-Bus documentation.
>
> -      This is the content of ``ui/dbus-display1.xml``:
> +      This is the content of ``util/dbus-display1.xml``:
>
> -   .. literalinclude:: ../../ui/dbus-display1.xml
> +   .. literalinclude:: ../../util/dbus-display1.xml
>        :language: xml
> diff --git a/ui/dbus.h b/ui/dbus.h
> index 1e8c24a48e32..a847bee98876 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 "util/dbus-display1.h"
>
>  typedef struct DBusClipboardRequest {
>      GDBusMethodInvocation *invocation;
> diff --git a/audio/dbusaudio.c b/audio/dbusaudio.c
> index 60fcf643ecf8..2aacdac6715b 100644
> --- a/audio/dbusaudio.c
> +++ b/audio/dbusaudio.c
> @@ -34,7 +34,7 @@
>  #endif
>
>  #include "ui/dbus.h"
> -#include "ui/dbus-display1.h"
> +#include "util/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..d4871e2fd80f 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 "util/dbus-display1.h"
>
>  static GDBusConnection*
>  test_dbus_p2p_from_fd(int fd)
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 39557d5ecbb0..627ff8fbe1c7 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -344,7 +344,7 @@ if vnc.found()
>  endif
>
>  if dbus_display
> -  qtests += {'dbus-display-test': [dbus_display1, gio]}
> +  qtests += {'dbus-display-test': [gio]}
>  endif
>
>  qtest_executables = {}
> diff --git a/ui/meson.build b/ui/meson.build
> index 376e0d771ba9..74e2a79b8c1e 100644
> --- a/ui/meson.build
> +++ b/ui/meson.build
> @@ -74,25 +74,7 @@ endif
>
>  if dbus_display
>    dbus_ss = ss.source_set()
> -  env = environment()
> -  env.set('HOST_OS', host_os)
> -  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],
> +  dbus_ss.add(when: [gio],
>                if_true: [files(
>                  'dbus-chardev.c',
>                  'dbus-clipboard.c',
> diff --git a/ui/dbus-display1.xml b/util/dbus-display1.xml
> similarity index 100%
> rename from ui/dbus-display1.xml
> rename to util/dbus-display1.xml
> diff --git a/util/meson.build b/util/meson.build
> index 0ef9886be048..d23c8382fa37 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -128,3 +128,24 @@ elif cpu == 'loongarch64'
>  elif cpu in ['ppc', 'ppc64']
>    util_ss.add(files('cpuinfo-ppc.c'))
>  endif
> +
> +if dbus_display
> +  env = environment()
> +  env.set('HOST_OS', host_os)
> +  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@'])
> +  util_ss.add(dbus_display1)
> +  genh += dbus_display1[0]
> +endif
>
> ---
> base-commit: 5005aed8a7e728d028efb40e243ecfc2b4f3df3a
> change-id: 20231215-dbus-86876ecb7b09
>
> Best regards,
> --
> Akihiko Odaki <akihiko.odaki@daynix.com>
>
>


-- 
Marc-André Lureau
Re: [PATCH v6] util: Move dbus_display1 to util
Posted by Akihiko Odaki 9 months, 2 weeks ago 
On 2024/02/14 16:05, Marc-André Lureau wrote:
> Hi Akihiko
> 
> On Wed, Feb 14, 2024 at 9:39 AM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>
>> Move dbus_display1 from ui to util as it's used not only by ui/dbus but
>> also dbus-display-test.
> 
> It doesn't seem like the right place either. So let's focus on what
> this is fixing.

Ok. Then please have a look at v2, which doesn't use util:
https://lore.kernel.org/all/20231215-dbus-v2-0-1e2e6aa02115@daynix.com/

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.