All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] user-mode: Avoid processing unnecessary meson rules
@ 2021-09-26 22:01 Philippe Mathieu-Daudé
  2021-09-26 22:01 ` [PATCH 1/2] bsd-user: Only process meson rules on BSD host Philippe Mathieu-Daudé
  2021-09-26 22:01 ` [PATCH 2/2] linux-user: Only process meson rules on Linux host Philippe Mathieu-Daudé
  0 siblings, 2 replies; 22+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-26 22:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-trivial, Kyle Evans, Richard Henderson, Laurent Vivier,
	Philippe Mathieu-Daudé,
	Paolo Bonzini, Warner Losh

Only process linux-user/meson.build on Linux (respectively on BSD).

Philippe Mathieu-Daudé (2):
  bsd-user: Only process meson rules on BSD host
  linux-user: Only process meson rules on Linux host

 bsd-user/meson.build   | 4 ++++
 linux-user/meson.build | 4 ++++
 2 files changed, 8 insertions(+)

-- 
2.31.1



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-26 22:01 [PATCH 0/2] user-mode: Avoid processing unnecessary meson rules Philippe Mathieu-Daudé
@ 2021-09-26 22:01 ` Philippe Mathieu-Daudé
  2021-09-26 23:08   ` Richard Henderson
                     ` (3 more replies)
  2021-09-26 22:01 ` [PATCH 2/2] linux-user: Only process meson rules on Linux host Philippe Mathieu-Daudé
  1 sibling, 4 replies; 22+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-26 22:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-trivial, Kyle Evans, Richard Henderson, Laurent Vivier,
	Philippe Mathieu-Daudé,
	Paolo Bonzini, Warner Losh

Reported-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 bsd-user/meson.build | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/bsd-user/meson.build b/bsd-user/meson.build
index 03695493408..a7607e1c884 100644
--- a/bsd-user/meson.build
+++ b/bsd-user/meson.build
@@ -1,3 +1,7 @@
+if not config_host.has_key('CONFIG_BSD')
+  subdir_done()
+endif
+
 bsd_user_ss.add(files(
   'bsdload.c',
   'elfload.c',
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/2] linux-user: Only process meson rules on Linux host
  2021-09-26 22:01 [PATCH 0/2] user-mode: Avoid processing unnecessary meson rules Philippe Mathieu-Daudé
  2021-09-26 22:01 ` [PATCH 1/2] bsd-user: Only process meson rules on BSD host Philippe Mathieu-Daudé
@ 2021-09-26 22:01 ` Philippe Mathieu-Daudé
  2021-09-27  2:42   ` WANG Xuerui
  1 sibling, 1 reply; 22+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-26 22:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-trivial, Kyle Evans, Richard Henderson, Laurent Vivier,
	Philippe Mathieu-Daudé,
	Paolo Bonzini, Warner Losh

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 linux-user/meson.build | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/linux-user/meson.build b/linux-user/meson.build
index 9549f81682d..1c4e94f0cc8 100644
--- a/linux-user/meson.build
+++ b/linux-user/meson.build
@@ -1,3 +1,7 @@
+if not config_host.has_key('CONFIG_LINUX')
+  subdir_done()
+endif
+
 linux_user_ss.add(files(
   'elfload.c',
   'exit.c',
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-26 22:01 ` [PATCH 1/2] bsd-user: Only process meson rules on BSD host Philippe Mathieu-Daudé
@ 2021-09-26 23:08   ` Richard Henderson
  2021-09-26 23:31     ` Warner Losh
  2021-09-27  5:24     ` Philippe Mathieu-Daudé
  2021-09-27  2:42   ` WANG Xuerui
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 22+ messages in thread
From: Richard Henderson @ 2021-09-26 23:08 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-trivial, Paolo Bonzini, Laurent Vivier, Warner Losh, Kyle Evans

On 9/26/21 6:01 PM, Philippe Mathieu-Daudé wrote:
> Reported-by: Warner Losh <imp@bsdimp.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   bsd-user/meson.build | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> index 03695493408..a7607e1c884 100644
> --- a/bsd-user/meson.build
> +++ b/bsd-user/meson.build
> @@ -1,3 +1,7 @@
> +if not config_host.has_key('CONFIG_BSD')
> +  subdir_done()
> +endif

Why here and not in the parent meson.build?


r~


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-26 23:08   ` Richard Henderson
@ 2021-09-26 23:31     ` Warner Losh
  2021-09-27  5:24     ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 22+ messages in thread
From: Warner Losh @ 2021-09-26 23:31 UTC (permalink / raw)
  To: Richard Henderson
  Cc: QEMU Developers, QEMU Trivial, Kyle Evans, Laurent Vivier,
	Philippe Mathieu-Daudé,
	Paolo Bonzini

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

On Sun, Sep 26, 2021 at 5:08 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 9/26/21 6:01 PM, Philippe Mathieu-Daudé wrote:
> > Reported-by: Warner Losh <imp@bsdimp.com>
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> >   bsd-user/meson.build | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> > index 03695493408..a7607e1c884 100644
> > --- a/bsd-user/meson.build
> > +++ b/bsd-user/meson.build
> > @@ -1,3 +1,7 @@
> > +if not config_host.has_key('CONFIG_BSD')
> > +  subdir_done()
> > +endif
>
> Why here and not in the parent meson.build?
>

I have the same question... I never know where to 'filter' when it comes to
meson...

Waner

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-26 22:01 ` [PATCH 1/2] bsd-user: Only process meson rules on BSD host Philippe Mathieu-Daudé
  2021-09-26 23:08   ` Richard Henderson
@ 2021-09-27  2:42   ` WANG Xuerui
  2021-09-27  2:59     ` WANG Xuerui
  2021-09-27  9:14   ` Peter Maydell
  2021-09-27  9:52   ` Daniel P. Berrangé
  3 siblings, 1 reply; 22+ messages in thread
From: WANG Xuerui @ 2021-09-27  2:42 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-trivial, Kyle Evans, Richard Henderson, Laurent Vivier,
	Paolo Bonzini, Warner Losh

On 9/27/21 06:01, Philippe Mathieu-Daudé wrote:
> Reported-by: Warner Losh <imp@bsdimp.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   bsd-user/meson.build | 4 ++++
>   1 file changed, 4 insertions(+)

I'm newcomer here, but this is just 4 lines of Meson, and two similar 
usages already exist within QEMU proper so...

Reviewed-by: WANG Xuerui <git@xen0n.name>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] linux-user: Only process meson rules on Linux host
  2021-09-26 22:01 ` [PATCH 2/2] linux-user: Only process meson rules on Linux host Philippe Mathieu-Daudé
@ 2021-09-27  2:42   ` WANG Xuerui
  0 siblings, 0 replies; 22+ messages in thread
From: WANG Xuerui @ 2021-09-27  2:42 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-trivial, Kyle Evans, Richard Henderson, Laurent Vivier,
	Paolo Bonzini, Warner Losh

On 9/27/21 06:01, Philippe Mathieu-Daudé wrote:

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   linux-user/meson.build | 4 ++++
>   1 file changed, 4 insertions(+)
Reviewed-by: WANG Xuerui <git@xen0n.name>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-27  2:42   ` WANG Xuerui
@ 2021-09-27  2:59     ` WANG Xuerui
  0 siblings, 0 replies; 22+ messages in thread
From: WANG Xuerui @ 2021-09-27  2:59 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-trivial, Kyle Evans, Richard Henderson, Laurent Vivier,
	Paolo Bonzini, Warner Losh

On 9/27/21 10:42, WANG Xuerui wrote:

> On 9/27/21 06:01, Philippe Mathieu-Daudé wrote:
>> Reported-by: Warner Losh <imp@bsdimp.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   bsd-user/meson.build | 4 ++++
>>   1 file changed, 4 insertions(+)
>
> I'm newcomer here, but this is just 4 lines of Meson, and two similar 
> usages already exist within QEMU proper so...
>
> Reviewed-by: WANG Xuerui <git@xen0n.name>
>
>
Hmm, other replies pointed out that one can also make the top-level 
subdir() call conditional... And we seem to have more than 2 usages 
following the latter pattern instead. However putting the conditional in 
sub-directory makes the logic localized, which is arguably more friendly 
to someone casually browsing the source code.

As both ways work, I think maybe being consistent and moving things to 
top-level meson.build has more value?



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-26 23:08   ` Richard Henderson
  2021-09-26 23:31     ` Warner Losh
@ 2021-09-27  5:24     ` Philippe Mathieu-Daudé
  2021-10-05 19:16       ` Paolo Bonzini
  1 sibling, 1 reply; 22+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-27  5:24 UTC (permalink / raw)
  To: Richard Henderson, Paolo Bonzini, qemu-devel
  Cc: qemu-trivial, Kyle Evans, Laurent Vivier, Warner Losh

On 9/27/21 01:08, Richard Henderson wrote:
> On 9/26/21 6:01 PM, Philippe Mathieu-Daudé wrote:
>> Reported-by: Warner Losh <imp@bsdimp.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   bsd-user/meson.build | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/bsd-user/meson.build b/bsd-user/meson.build
>> index 03695493408..a7607e1c884 100644
>> --- a/bsd-user/meson.build
>> +++ b/bsd-user/meson.build
>> @@ -1,3 +1,7 @@
>> +if not config_host.has_key('CONFIG_BSD')
>> +  subdir_done()
>> +endif
> 
> Why here and not in the parent meson.build?

This is what Paolo recommended me to do last time I added a
conditional inclusion.

Personally I prefer having it in the call site rather than
the callee (no need to read the callee to notice it isn't
called). I guess this is for readability, to not clutter
meson.build files more...

Paolo, what is your preference?


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-26 22:01 ` [PATCH 1/2] bsd-user: Only process meson rules on BSD host Philippe Mathieu-Daudé
  2021-09-26 23:08   ` Richard Henderson
  2021-09-27  2:42   ` WANG Xuerui
@ 2021-09-27  9:14   ` Peter Maydell
  2021-09-27  9:40     ` Philippe Mathieu-Daudé
  2021-09-27  9:52   ` Daniel P. Berrangé
  3 siblings, 1 reply; 22+ messages in thread
From: Peter Maydell @ 2021-09-27  9:14 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: QEMU Trivial, Kyle Evans, Richard Henderson, QEMU Developers,
	Laurent Vivier, Paolo Bonzini, Warner Losh

On Sun, 26 Sept 2021 at 23:04, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Reported-by: Warner Losh <imp@bsdimp.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  bsd-user/meson.build | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> index 03695493408..a7607e1c884 100644
> --- a/bsd-user/meson.build
> +++ b/bsd-user/meson.build
> @@ -1,3 +1,7 @@
> +if not config_host.has_key('CONFIG_BSD')
> +  subdir_done()
> +endif
> +
>  bsd_user_ss.add(files(
>    'bsdload.c',
>    'elfload.c',


So, what's the reason for this change? The commit messages and
the cover letter don't really explain it. Is this fixing a bug
(if so what?), a precaution to avoid possible future bugs,
fixing a performance issue with how long meson takes to run (if
so, how much effect does this have), or something else?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-27  9:14   ` Peter Maydell
@ 2021-09-27  9:40     ` Philippe Mathieu-Daudé
  2021-09-27  9:54       ` Peter Maydell
  2021-10-05 18:28       ` Alex Bennée
  0 siblings, 2 replies; 22+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-27  9:40 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Trivial, Kyle Evans, Richard Henderson, QEMU Developers,
	Laurent Vivier, Paolo Bonzini, Warner Losh

On Mon, Sep 27, 2021 at 11:15 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> On Sun, 26 Sept 2021 at 23:04, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > Reported-by: Warner Losh <imp@bsdimp.com>
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> >  bsd-user/meson.build | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> > index 03695493408..a7607e1c884 100644
> > --- a/bsd-user/meson.build
> > +++ b/bsd-user/meson.build
> > @@ -1,3 +1,7 @@
> > +if not config_host.has_key('CONFIG_BSD')
> > +  subdir_done()
> > +endif
> > +
> >  bsd_user_ss.add(files(
> >    'bsdload.c',
> >    'elfload.c',
>
>
> So, what's the reason for this change?

https://lore.kernel.org/qemu-devel/CANCZdfprC16ezJQCWJmYEApX6eym9nxSOqAtBAGr+cziS4r2qw@mail.gmail.com/

linux-user/meson.build is evaluated on bsd, and bsd-user/meson.build on Linux.

> The commit messages and
> the cover letter don't really explain it. Is this fixing a bug
> (if so what?), a precaution to avoid possible future bugs,
> fixing a performance issue with how long meson takes to run (if
> so, how much effect does this have), or something else?

I'll wait for feedback from Paolo, then work on the explanation.

Regards,

Phil.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-26 22:01 ` [PATCH 1/2] bsd-user: Only process meson rules on BSD host Philippe Mathieu-Daudé
                     ` (2 preceding siblings ...)
  2021-09-27  9:14   ` Peter Maydell
@ 2021-09-27  9:52   ` Daniel P. Berrangé
  2021-09-27 10:07     ` Peter Maydell
  2021-10-05 19:26     ` Paolo Bonzini
  3 siblings, 2 replies; 22+ messages in thread
From: Daniel P. Berrangé @ 2021-09-27  9:52 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-trivial, Kyle Evans, Richard Henderson, qemu-devel,
	Laurent Vivier, Paolo Bonzini, Warner Losh

On Mon, Sep 27, 2021 at 12:01:02AM +0200, Philippe Mathieu-Daudé wrote:
> Reported-by: Warner Losh <imp@bsdimp.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  bsd-user/meson.build | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> index 03695493408..a7607e1c884 100644
> --- a/bsd-user/meson.build
> +++ b/bsd-user/meson.build
> @@ -1,3 +1,7 @@
> +if not config_host.has_key('CONFIG_BSD')
> +  subdir_done()
> +endif
> +
>  bsd_user_ss.add(files(
>    'bsdload.c',
>    'elfload.c',

If we look at the big picture across the root meson.build, and this
meson.build we have


  bsd_user_ss = ss.source_set()

  ...

  bsd_user_ss.add(files(
    'bsdload.c',
    'elfload.c',
    'main.c',
    'mmap.c',
    'signal.c',
    'strace.c',
    'syscall.c',
    'uaccess.c',
  ))

  ...

  bsd_user_ss.add(files('gdbstub.c'))
  specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss)


So without this change, we're already correctly dropping bsd_user_ss
in its entirity, when not on BSD.

With this change, we're dropping some, but not all, of bsd_user_ss
files - gdbstub.c remains.

So this change on its own doesn't make a whole lot of sense.

If we look at linux-user/meson.build though things are more complex.
There we have alot of sub-dirs, and meson.biuld in those dirs adds
generators for various files. So conceivably skipping linux-user
will mean we won't auto-generate files we don't need on non-Linux.

With that in mind, I think it makes conceptual sense to have this
bsd-user/meson.build change, for the purpose of design consistency,
even if it doesn't have any real world benefit for bsd-user today.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-27  9:40     ` Philippe Mathieu-Daudé
@ 2021-09-27  9:54       ` Peter Maydell
  2021-10-05 19:23         ` Paolo Bonzini
  2021-10-05 18:28       ` Alex Bennée
  1 sibling, 1 reply; 22+ messages in thread
From: Peter Maydell @ 2021-09-27  9:54 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: QEMU Trivial, Kyle Evans, Richard Henderson, QEMU Developers,
	Laurent Vivier, Paolo Bonzini, Warner Losh

On Mon, 27 Sept 2021 at 10:40, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On Mon, Sep 27, 2021 at 11:15 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> > On Sun, 26 Sept 2021 at 23:04, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > > Reported-by: Warner Losh <imp@bsdimp.com>
> > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > > ---
> > >  bsd-user/meson.build | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> > > index 03695493408..a7607e1c884 100644
> > > --- a/bsd-user/meson.build
> > > +++ b/bsd-user/meson.build
> > > @@ -1,3 +1,7 @@
> > > +if not config_host.has_key('CONFIG_BSD')
> > > +  subdir_done()
> > > +endif
> > > +
> > >  bsd_user_ss.add(files(
> > >    'bsdload.c',
> > >    'elfload.c',
> >
> >
> > So, what's the reason for this change?
>
> https://lore.kernel.org/qemu-devel/CANCZdfprC16ezJQCWJmYEApX6eym9nxSOqAtBAGr+cziS4r2qw@mail.gmail.com/
>
> linux-user/meson.build is evaluated on bsd, and bsd-user/meson.build on Linux.

True, but "meson.build is evaluated but just does nothing or
adds files to a sourceset that isn't used" is pretty common
(hw/pci/meson.build is evaluated even if we're not building
a system with PCI support, for example). If there's a reason
why bsd-user and linux-user are special we should explain it,
I think.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-27  9:52   ` Daniel P. Berrangé
@ 2021-09-27 10:07     ` Peter Maydell
  2021-09-27 10:53       ` Warner Losh
  2021-10-05 19:26     ` Paolo Bonzini
  1 sibling, 1 reply; 22+ messages in thread
From: Peter Maydell @ 2021-09-27 10:07 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: QEMU Developers, QEMU Trivial, Kyle Evans, Richard Henderson,
	Laurent Vivier, Philippe Mathieu-Daudé,
	Paolo Bonzini, Warner Losh

On Mon, 27 Sept 2021 at 10:59, Daniel P. Berrangé <berrange@redhat.com> wrote:
> If we look at linux-user/meson.build though things are more complex.
> There we have alot of sub-dirs, and meson.biuld in those dirs adds
> generators for various files. So conceivably skipping linux-user
> will mean we won't auto-generate files we don't need on non-Linux.

The top level meson.build doesn't call process on the
syscall_nr_generators[] list unless we're building linux-user
targets, so I don't think we will auto-generate anything we
don't need.

-- PMM


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-27 10:07     ` Peter Maydell
@ 2021-09-27 10:53       ` Warner Losh
  0 siblings, 0 replies; 22+ messages in thread
From: Warner Losh @ 2021-09-27 10:53 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Daniel P. Berrangé,
	QEMU Trivial, Kyle Evans, Richard Henderson,
	Philippe Mathieu-Daudé,
	QEMU Developers, Paolo Bonzini, Laurent Vivier

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

On Mon, Sep 27, 2021 at 4:08 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Mon, 27 Sept 2021 at 10:59, Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> > If we look at linux-user/meson.build though things are more complex.
> > There we have alot of sub-dirs, and meson.biuld in those dirs adds
> > generators for various files. So conceivably skipping linux-user
> > will mean we won't auto-generate files we don't need on non-Linux.
>
> The top level meson.build doesn't call process on the
> syscall_nr_generators[] list unless we're building linux-user
> targets, so I don't think we will auto-generate anything we
> don't need.
>

The problem is that I'm trying to add some os-specific files
to the bsd-user in a patch set I sent off. bsd-user compiles
for different BSDs, and I'd like to start pulling in per-bsd
files as I'm upstreaming.  To do that, I have this construct
in the bsd-user/meson.build file:

# Pull in the OS-specific build glue, if any
if fs.exists(targetos)
   subdir(targetos)
endif

primarily because the builds failed on Linux when it tried to
find the non-existent bsd-user/linunx directory. The question
came up on how to cope with this situation, and Philippe
sent off this series as an alternative to that construct. The
whole reason why we descend into the linux-user directory
on BSD and into the bsd-user directory on linux is unclear
to me as well.

So this is preparing the ground for my future work of upstreaming.
I'm agnostic as to how it's resolved, but it needs to be resolved.

Warner

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-27  9:40     ` Philippe Mathieu-Daudé
  2021-09-27  9:54       ` Peter Maydell
@ 2021-10-05 18:28       ` Alex Bennée
  1 sibling, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2021-10-05 18:28 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, QEMU Trivial, Kyle Evans, Richard Henderson,
	Laurent Vivier, qemu-devel, Paolo Bonzini, Warner Losh


Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> On Mon, Sep 27, 2021 at 11:15 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>> On Sun, 26 Sept 2021 at 23:04, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> > Reported-by: Warner Losh <imp@bsdimp.com>
>> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> > ---
>> >  bsd-user/meson.build | 4 ++++
>> >  1 file changed, 4 insertions(+)
>> >
>> > diff --git a/bsd-user/meson.build b/bsd-user/meson.build
>> > index 03695493408..a7607e1c884 100644
>> > --- a/bsd-user/meson.build
>> > +++ b/bsd-user/meson.build
>> > @@ -1,3 +1,7 @@
>> > +if not config_host.has_key('CONFIG_BSD')
>> > +  subdir_done()
>> > +endif
>> > +
>> >  bsd_user_ss.add(files(
>> >    'bsdload.c',
>> >    'elfload.c',
>>
>>
>> So, what's the reason for this change?
>
> https://lore.kernel.org/qemu-devel/CANCZdfprC16ezJQCWJmYEApX6eym9nxSOqAtBAGr+cziS4r2qw@mail.gmail.com/
>
> linux-user/meson.build is evaluated on bsd, and bsd-user/meson.build on Linux.
>
>> The commit messages and
>> the cover letter don't really explain it. Is this fixing a bug
>> (if so what?), a precaution to avoid possible future bugs,
>> fixing a performance issue with how long meson takes to run (if
>> so, how much effect does this have), or something else?
>
> I'll wait for feedback from Paolo, then work on the explanation.

Ping Paolo?

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-27  5:24     ` Philippe Mathieu-Daudé
@ 2021-10-05 19:16       ` Paolo Bonzini
  0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2021-10-05 19:16 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Richard Henderson, qemu-devel
  Cc: qemu-trivial, Kyle Evans, Laurent Vivier, Warner Losh

On 27/09/21 07:24, Philippe Mathieu-Daudé wrote:
>> Why here and not in the parent meson.build?
> This is what Paolo recommended me to do last time I added a
> conditional inclusion.
> 
> Personally I prefer having it in the call site rather than
> the callee (no need to read the callee to notice it isn't
> called). I guess this is for readability, to not clutter
> meson.build? files more...

Yes, pretty much.  In this case it's quite obvious that bsd-user is 
BSD-only, but I prefer it if dir/meson.build has the knowledge of what 
goes on in dir/.

That said, we're not terribly consistent, see have_block and have_tools, 
so either will be okay.

Paolo

> Paolo, what is your preference?
> 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-27  9:54       ` Peter Maydell
@ 2021-10-05 19:23         ` Paolo Bonzini
  0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2021-10-05 19:23 UTC (permalink / raw)
  To: Peter Maydell, Philippe Mathieu-Daudé
  Cc: QEMU Trivial, Kyle Evans, Richard Henderson, QEMU Developers,
	Laurent Vivier, Warner Losh

On 27/09/21 11:54, Peter Maydell wrote:
> True, but "meson.build is evaluated but just does nothing or
> adds files to a sourceset that isn't used" is pretty common
> (hw/pci/meson.build is evaluated even if we're not building
> a system with PCI support, for example).

Selection of files from hw/pci/meson.build is based on per-target 
definitions, so there's no way around when:/if_true:.  (Technically, 
hw/pci/meson.build also as an if_false, so there's *really* no way 
around parsing it).

Instead, when the definition is constant across all targets, it is 
possible to use either when:/if_true: or an "if" as in

if have_user
   util_ss.add(files('selfmap.c'))
endif

or the various "if m[0].found()" found in directories that build shared 
modules.  In this case I personally lean more towards the latter, but 
when:/if_true: is a little more compact so both are acceptable.

Paolo



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-09-27  9:52   ` Daniel P. Berrangé
  2021-09-27 10:07     ` Peter Maydell
@ 2021-10-05 19:26     ` Paolo Bonzini
  2021-10-05 20:41       ` Laurent Vivier
  1 sibling, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2021-10-05 19:26 UTC (permalink / raw)
  To: Daniel P. Berrangé, Philippe Mathieu-Daudé
  Cc: qemu-trivial, Kyle Evans, Richard Henderson, qemu-devel,
	Laurent Vivier, Warner Losh

On 27/09/21 11:52, Daniel P. Berrangé wrote:
>    bsd_user_ss.add(files('gdbstub.c'))
>    specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss)
> 
> 
> So without this change, we're already correctly dropping bsd_user_ss
> in its entirity, when not on BSD.
> 
> With this change, we're dropping some, but not all, of bsd_user_ss
> files - gdbstub.c remains.
> 
> So this change on its own doesn't make a whole lot of sense.

Agreed; that said, the gdbstub.c files added by

   bsd_user_ss.add(files('gdbstub.c'))
   linux_user_ss.add(files('gdbstub.c', 'thunk.c'))

are unnecessary because there is already

   specific_ss.add(files('cpu.c', 'disas.c', 'gdbstub.c'), capstone)

So, with those two instances of gdbstub.c removed, both patches are

   Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks,

Paolo



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-10-05 19:26     ` Paolo Bonzini
@ 2021-10-05 20:41       ` Laurent Vivier
  2021-10-05 20:46         ` Warner Losh
  0 siblings, 1 reply; 22+ messages in thread
From: Laurent Vivier @ 2021-10-05 20:41 UTC (permalink / raw)
  To: Paolo Bonzini, Daniel P. Berrangé, Philippe Mathieu-Daudé
  Cc: qemu-trivial, Kyle Evans, Richard Henderson, qemu-devel, Warner Losh

Le 05/10/2021 à 21:26, Paolo Bonzini a écrit :
> On 27/09/21 11:52, Daniel P. Berrangé wrote:
>>    bsd_user_ss.add(files('gdbstub.c'))
>>    specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss)
>>
>>
>> So without this change, we're already correctly dropping bsd_user_ss
>> in its entirity, when not on BSD.
>>
>> With this change, we're dropping some, but not all, of bsd_user_ss
>> files - gdbstub.c remains.
>>
>> So this change on its own doesn't make a whole lot of sense.
> 
> Agreed; that said, the gdbstub.c files added by
> 
>   bsd_user_ss.add(files('gdbstub.c'))
>   linux_user_ss.add(files('gdbstub.c', 'thunk.c'))
> 
> are unnecessary because there is already
> 
>   specific_ss.add(files('cpu.c', 'disas.c', 'gdbstub.c'), capstone)
> 
> So, with those two instances of gdbstub.c removed, both patches are
> 
>   Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> 

I can take them if a v2 is sent updated accordingly...

Thanks,
Laurent



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-10-05 20:41       ` Laurent Vivier
@ 2021-10-05 20:46         ` Warner Losh
  2021-10-06  6:02           ` Laurent Vivier
  0 siblings, 1 reply; 22+ messages in thread
From: Warner Losh @ 2021-10-05 20:46 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Daniel P. Berrangé,
	QEMU Trivial, Kyle Evans, Richard Henderson,
	Philippe Mathieu-Daudé,
	QEMU Developers, Paolo Bonzini

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

On Tue, Oct 5, 2021 at 2:41 PM Laurent Vivier <laurent@vivier.eu> wrote:

> Le 05/10/2021 à 21:26, Paolo Bonzini a écrit :
> > On 27/09/21 11:52, Daniel P. Berrangé wrote:
> >>    bsd_user_ss.add(files('gdbstub.c'))
> >>    specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss)
> >>
> >>
> >> So without this change, we're already correctly dropping bsd_user_ss
> >> in its entirity, when not on BSD.
> >>
> >> With this change, we're dropping some, but not all, of bsd_user_ss
> >> files - gdbstub.c remains.
> >>
> >> So this change on its own doesn't make a whole lot of sense.
> >
> > Agreed; that said, the gdbstub.c files added by
> >
> >   bsd_user_ss.add(files('gdbstub.c'))
> >   linux_user_ss.add(files('gdbstub.c', 'thunk.c'))
> >
> > are unnecessary because there is already
> >
> >   specific_ss.add(files('cpu.c', 'disas.c', 'gdbstub.c'), capstone)
> >
> > So, with those two instances of gdbstub.c removed, both patches are
> >
> >   Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> >
>
> I can take them if a v2 is sent updated accordingly...
>

I'd planned on rolling them into the bsd-user patch set that I'd been
working on
that is almost ready to go in (this is I think the only remaining issue
with the patch
train, though I need to double check threads). I'd planned on doing that
tomorrow,
but would be happy to coordinate with you since one of them does touch
linux-user.

Warner

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
  2021-10-05 20:46         ` Warner Losh
@ 2021-10-06  6:02           ` Laurent Vivier
  0 siblings, 0 replies; 22+ messages in thread
From: Laurent Vivier @ 2021-10-06  6:02 UTC (permalink / raw)
  To: Warner Losh
  Cc: Daniel P. Berrangé,
	QEMU Trivial, Kyle Evans, Richard Henderson,
	Philippe Mathieu-Daudé,
	QEMU Developers, Paolo Bonzini

Le 05/10/2021 à 22:46, Warner Losh a écrit :
> 
> 
> On Tue, Oct 5, 2021 at 2:41 PM Laurent Vivier <laurent@vivier.eu <mailto:laurent@vivier.eu>> wrote:
> 
>     Le 05/10/2021 à 21:26, Paolo Bonzini a écrit :
>     > On 27/09/21 11:52, Daniel P. Berrangé wrote:
>     >>    bsd_user_ss.add(files('gdbstub.c'))
>     >>    specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss)
>     >>
>     >>
>     >> So without this change, we're already correctly dropping bsd_user_ss
>     >> in its entirity, when not on BSD.
>     >>
>     >> With this change, we're dropping some, but not all, of bsd_user_ss
>     >> files - gdbstub.c remains.
>     >>
>     >> So this change on its own doesn't make a whole lot of sense.
>     >
>     > Agreed; that said, the gdbstub.c files added by
>     >
>     >   bsd_user_ss.add(files('gdbstub.c'))
>     >   linux_user_ss.add(files('gdbstub.c', 'thunk.c'))
>     >
>     > are unnecessary because there is already
>     >
>     >   specific_ss.add(files('cpu.c', 'disas.c', 'gdbstub.c'), capstone)
>     >
>     > So, with those two instances of gdbstub.c removed, both patches are
>     >
>     >   Acked-by: Paolo Bonzini <pbonzini@redhat.com <mailto:pbonzini@redhat.com>>
>     >
> 
>     I can take them if a v2 is sent updated accordingly...
> 
> 
> I'd planned on rolling them into the bsd-user patch set that I'd been working on
> that is almost ready to go in (this is I think the only remaining issue with the patch
> train, though I need to double check threads). I'd planned on doing that tomorrow,
> but would be happy to coordinate with you since one of them does touch linux-user.

You can take both.

Thanks,
Laurent



^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-10-06  6:04 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-26 22:01 [PATCH 0/2] user-mode: Avoid processing unnecessary meson rules Philippe Mathieu-Daudé
2021-09-26 22:01 ` [PATCH 1/2] bsd-user: Only process meson rules on BSD host Philippe Mathieu-Daudé
2021-09-26 23:08   ` Richard Henderson
2021-09-26 23:31     ` Warner Losh
2021-09-27  5:24     ` Philippe Mathieu-Daudé
2021-10-05 19:16       ` Paolo Bonzini
2021-09-27  2:42   ` WANG Xuerui
2021-09-27  2:59     ` WANG Xuerui
2021-09-27  9:14   ` Peter Maydell
2021-09-27  9:40     ` Philippe Mathieu-Daudé
2021-09-27  9:54       ` Peter Maydell
2021-10-05 19:23         ` Paolo Bonzini
2021-10-05 18:28       ` Alex Bennée
2021-09-27  9:52   ` Daniel P. Berrangé
2021-09-27 10:07     ` Peter Maydell
2021-09-27 10:53       ` Warner Losh
2021-10-05 19:26     ` Paolo Bonzini
2021-10-05 20:41       ` Laurent Vivier
2021-10-05 20:46         ` Warner Losh
2021-10-06  6:02           ` Laurent Vivier
2021-09-26 22:01 ` [PATCH 2/2] linux-user: Only process meson rules on Linux host Philippe Mathieu-Daudé
2021-09-27  2:42   ` WANG Xuerui

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.