meson: tools: depend on keycode generated sources

[PATCH] meson: tools: depend on keycode generated sources

Posted by Roman Bogorodskiy 3 years, 1 month ago

Tools depend on keycode generated sources, so declare that as an
explicit dependency, otherwise it might fail with:

../tools/virsh-completer-domain.c:35:10: fatal error: 'virkeynametable_linux.h' file not found
         ^~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
---
I noticed that build failure on FreeBSD 11.x. For some reason, it
doesn't show up on newer versions. This is strange, but as the fix appears
to be straight-forward, I didn't spend much time figuring out the
reason.

In case you're interested, a full failing build log is here:

 https://people.freebsd.org/~novel/misc/libvirt-7.1.0.log

It doesn't seem to even try to generate these files. When I inspected
filesystem state, these files were missing.


 src/util/meson.build | 2 ++
 tools/meson.build    | 1 +
 2 files changed, 3 insertions(+)

diff --git a/src/util/meson.build b/src/util/meson.build
index 0080825bd0..cd3fe18524 100644
--- a/src/util/meson.build
+++ b/src/util/meson.build
@@ -168,6 +168,8 @@ foreach name : keyname_list
   )
 endforeach
 
+keycode_dep = declare_dependency(sources: keycode_gen_sources)
+
 io_helper_sources = [
   'iohelper.c',
 ]
diff --git a/tools/meson.build b/tools/meson.build
index b8c6802f0a..42dc609439 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -186,6 +186,7 @@ executable(
     tools_dep,
     readline_dep,
     thread_dep,
+    keycode_dep,
   ],
   link_args: [
     coverage_flags,
-- 
2.30.0

Re: [PATCH] meson: tools: depend on keycode generated sources

Posted by Ján Tomko 3 years, 1 month ago

On a Wednesday in 2021, Roman Bogorodskiy wrote:
>Tools depend on keycode generated sources, so declare that as an
>explicit dependency, otherwise it might fail with:
>
>../tools/virsh-completer-domain.c:35:10: fatal error: 'virkeynametable_linux.h' file not found
>         ^~~~~~~~~~~~~~~~~~~~~~~~~
>
>Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>

Fixes: b0f4cf25a6c17907d16523e0fa3c10e253f81f8b

>---
>I noticed that build failure on FreeBSD 11.x. For some reason, it
>doesn't show up on newer versions. This is strange, but as the fix appears
>to be straight-forward, I didn't spend much time figuring out the
>reason.
>
>In case you're interested, a full failing build log is here:
>
> https://people.freebsd.org/~novel/misc/libvirt-7.1.0.log
>
>It doesn't seem to even try to generate these files. When I inspected
>filesystem state, these files were missing.
>
>
> src/util/meson.build | 2 ++
> tools/meson.build    | 1 +
> 2 files changed, 3 insertions(+)
>
>diff --git a/src/util/meson.build b/src/util/meson.build
>index 0080825bd0..cd3fe18524 100644
>--- a/src/util/meson.build
>+++ b/src/util/meson.build
>@@ -168,6 +168,8 @@ foreach name : keyname_list
>   )
> endforeach
>
>+keycode_dep = declare_dependency(sources: keycode_gen_sources)
>+

Please format this as:

keycode_dep = declare_dependency(
   sources: keycode_gen_sources
)

to match the prevailing style.

> io_helper_sources = [
>   'iohelper.c',
> ]
>diff --git a/tools/meson.build b/tools/meson.build
>index b8c6802f0a..42dc609439 100644
>--- a/tools/meson.build
>+++ b/tools/meson.build
>@@ -186,6 +186,7 @@ executable(
>     tools_dep,
>     readline_dep,
>     thread_dep,
>+    keycode_dep,
>   ],
>   link_args: [
>     coverage_flags,

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano

Re: [PATCH] meson: tools: depend on keycode generated sources

Posted by Andrea Bolognani 3 years, 1 month ago

On Thu, 2021-03-04 at 17:47 +0100, Ján Tomko wrote:
> On a Wednesday in 2021, Roman Bogorodskiy wrote:
> > +keycode_dep = declare_dependency(sources: keycode_gen_sources)
> 
> Please format this as:
> 
> keycode_dep = declare_dependency(
>    sources: keycode_gen_sources
> )
> 
> to match the prevailing style.

Small correction: it should be

  keycode_dep = declare_dependency(
    sources: keycode_gen_sources,
  )

Note the additional comma, which allows us to have cleaner diffs when
making further changes, and the indentation being only two spaces
instead of three.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH] meson: tools: depend on keycode generated sources

Posted by Ján Tomko 3 years, 1 month ago

On a Friday in 2021, Andrea Bolognani wrote:
>On Thu, 2021-03-04 at 17:47 +0100, Ján Tomko wrote:
>> On a Wednesday in 2021, Roman Bogorodskiy wrote:
>> > +keycode_dep = declare_dependency(sources: keycode_gen_sources)
>>
>> Please format this as:
>>
>> keycode_dep = declare_dependency(
>>    sources: keycode_gen_sources
>> )
>>
>> to match the prevailing style.
>
>Small correction: it should be
>
>  keycode_dep = declare_dependency(
>    sources: keycode_gen_sources,
>  )
>
>Note the additional comma, which allows us to have cleaner diffs when
>making further changes, and the indentation being only two spaces
>instead of three.

The three spaces come from your MUA misquoting me. I see two spaces in
my version of the e-mail, as well as the list archive:
https://listman.redhat.com/archives/libvir-list/2021-March/msg00252.html

(Not that my MUA is any better in that regard - the indentation in my
quoting of Roman's patch is wrong too)

Jano

>
>-- 
>Andrea Bolognani / Red Hat / Virtualization
>

Re: [PATCH] meson: tools: depend on keycode generated sources

Posted by Andrea Bolognani 3 years, 1 month ago

On Fri, 2021-03-05 at 13:43 +0100, Ján Tomko wrote:
> On a Friday in 2021, Andrea Bolognani wrote:
> > On Thu, 2021-03-04 at 17:47 +0100, Ján Tomko wrote:
> > > On a Wednesday in 2021, Roman Bogorodskiy wrote:
> > > > +keycode_dep = declare_dependency(sources: keycode_gen_sources)
> > > 
> > > Please format this as:
> > > 
> > > keycode_dep = declare_dependency(
> > >    sources: keycode_gen_sources
> > > )
> > > 
> > > to match the prevailing style.
> > 
> > Small correction: it should be
> > 
> >  keycode_dep = declare_dependency(
> >    sources: keycode_gen_sources,
> >  )
> > 
> > Note the additional comma, which allows us to have cleaner diffs when
> > making further changes, and the indentation being only two spaces
> > instead of three.
> 
> The three spaces come from your MUA misquoting me. I see two spaces in
> my version of the e-mail, as well as the list archive:
> https://listman.redhat.com/archives/libvir-list/2021-March/msg00252.html
> 
> (Not that my MUA is any better in that regard - the indentation in my
> quoting of Roman's patch is wrong too)

That's interesting: if I look at the HTML version you linked above or
copy and paste the snippet from it, the indentation is indeed two
spaces; however, if I look at the copy in my local mailbox or at the
full mbox taken from

  https://listman.redhat.com/archives/libvir-list/2021-March.txt.gz

there are three spaces.

Looking at the headers for your message, I see

  Content-Type: multipart/signed; micalg=pgp-sha256;
      protocol="application/pgp-signature"; boundary="kmvAAwZj779MjF+K"

followed by

  Content-Type: text/plain; charset=iso-8859-1; format=flowed
  Content-Disposition: inline
  Content-Transfer-Encoding: quoted-printable

and the body contains stuff like

  keycode_dep =3D declare_dependency(

  Reviewed-by: J=E1n Tomko

so I think perhaps your MUA's configuration might be to blame for the
weirdness we're seeing? Honestly, I just don't understand email well
enough to be able to tell :)

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH] meson: tools: depend on keycode generated sources

Posted by Roman Bogorodskiy 3 years, 1 month ago

  Andrea Bolognani wrote:

> On Fri, 2021-03-05 at 13:43 +0100, Ján Tomko wrote:
> > On a Friday in 2021, Andrea Bolognani wrote:
> > > On Thu, 2021-03-04 at 17:47 +0100, Ján Tomko wrote:
> > > > On a Wednesday in 2021, Roman Bogorodskiy wrote:
> > > > > +keycode_dep = declare_dependency(sources: keycode_gen_sources)
> > > > 
> > > > Please format this as:
> > > > 
> > > > keycode_dep = declare_dependency(
> > > >    sources: keycode_gen_sources
> > > > )
> > > > 
> > > > to match the prevailing style.
> > > 
> > > Small correction: it should be
> > > 
> > >  keycode_dep = declare_dependency(
> > >    sources: keycode_gen_sources,
> > >  )
> > > 
> > > Note the additional comma, which allows us to have cleaner diffs when
> > > making further changes, and the indentation being only two spaces
> > > instead of three.
> > 
> > The three spaces come from your MUA misquoting me. I see two spaces in
> > my version of the e-mail, as well as the list archive:
> > https://listman.redhat.com/archives/libvir-list/2021-March/msg00252.html
> > 
> > (Not that my MUA is any better in that regard - the indentation in my
> > quoting of Roman's patch is wrong too)
> 
> That's interesting: if I look at the HTML version you linked above or
> copy and paste the snippet from it, the indentation is indeed two
> spaces; however, if I look at the copy in my local mailbox or at the
> full mbox taken from
> 
>   https://listman.redhat.com/archives/libvir-list/2021-March.txt.gz
> 
> there are three spaces.
> 
> Looking at the headers for your message, I see
> 
>   Content-Type: multipart/signed; micalg=pgp-sha256;
>       protocol="application/pgp-signature"; boundary="kmvAAwZj779MjF+K"
> 
> followed by
> 
>   Content-Type: text/plain; charset=iso-8859-1; format=flowed
>   Content-Disposition: inline
>   Content-Transfer-Encoding: quoted-printable
> 
> and the body contains stuff like
> 
>   keycode_dep =3D declare_dependency(
> 
>   Reviewed-by: J=E1n Tomko
> 
> so I think perhaps your MUA's configuration might be to blame for the
> weirdness we're seeing? Honestly, I just don't understand email well
> enough to be able to tell :)
> 
> -- 
> Andrea Bolognani / Red Hat / Virtualization
> 

That's interesting indeed, because my MUA shows 2 space indentation in
all code snippets from this thread.

FWIW, the patch was merged with the formatting fixes applied, thanks.

Roman Bogorodskiy