user-mode: Avoid processing unnecessary meson rules

[PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Philippe Mathieu-Daudé 4 years, 4 months ago

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

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by WANG Xuerui 4 years, 4 months ago

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>

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by WANG Xuerui 4 years, 4 months ago

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?

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Peter Maydell 4 years, 4 months ago

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

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Philippe Mathieu-Daudé 4 years, 4 months ago

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.

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Peter Maydell 4 years, 4 months ago

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

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Paolo Bonzini 4 years, 4 months ago

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

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Alex Bennée 4 years, 4 months ago

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

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Daniel P. Berrangé 4 years, 4 months ago

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 :|

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Peter Maydell 4 years, 4 months ago

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

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Warner Losh 4 years, 4 months ago

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

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Paolo Bonzini 4 years, 4 months ago

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

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Laurent Vivier 4 years, 4 months ago

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

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Warner Losh 4 years, 4 months ago

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

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Laurent Vivier 4 years, 4 months ago

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

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Richard Henderson 4 years, 4 months ago

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~

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Warner Losh 4 years, 4 months ago

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

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Philippe Mathieu-Daudé 4 years, 4 months ago

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?

Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host

Posted by Paolo Bonzini 4 years, 4 months ago

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?
>