Hi Drew,
On Tue, Jan 14, 2025 at 07:51:04PM +0100, Andrew Jones wrote:
> On Tue, Jan 14, 2025 at 05:17:28PM +0000, Alexandru Elisei wrote:
> ...
> > > > +# $arch will have changed when cross-compiling.
> > > > +[ -z "$processor" ] && processor=$(get_default_processor $arch)
> > >
> > > The fact that $arch and $processor are wrong until they've had a chance to
> >
> > $processor is never wrong. $processor is unset until either the user sets it
> > with --processor, or until this line. This patch introduces $default_processor
> > only for the purpose of having an accurate help text, it doesn't change when and
> > how $processor is assigned.
>
> I should have said "The fact that $arch and $default_processor are wrong..."
>
> >
> > > be converted might be another reason for the $do_help idea. But it'll
> > > always be fragile since another change that does some sort of conversion
> > > could end up getting added after the '[ $do_help ] && usage' someday.
> >
> > configure needs to distinguish between:
> >
> > 1. The user not having specified --processor when doing ./configure.
> > 2. The user having set --processor.
> >
> > If 1, then kvm-unit-tests can use the default $processor value for $arch,
> > which could have also been specified by the user.
> >
> > If 2, then kvm-unit-tests should not touch $processor because that's what the
> > user wants.
> >
> > Do you see something wrong with that reasoning?
>
> If we output $default_processor in usage() before it's had a chance to be
> set correctly based on a given cross arch, then it won't display the
> correct name.
>
> >
> > Also, I don't understand why you say it's fragile, since configure doesn't
>
> I wrote "it'll always be fragile" where 'it' refers to the most recent
> object of my paragraph ("the $do_help idea"). But, TBH, I'm not sure
> how important it is to get the help text accurate, so we can just not
> care if we call usage() with the wrong strings sometimes.
Got it now, thanks for explaining it.
My opinion is that a help text is there to help the user, and in my experience
an inaccurate help text can be very frustrating - think comments that say
one thing, and the code does something else.
How about this:
diff --git a/configure b/configure
index 3ab0ec208e10..5dbe189816b2 100755
--- a/configure
+++ b/configure
@@ -51,7 +51,6 @@ page_size=
earlycon=
efi=
efi_direct=
-default_processor=$(get_default_processor $arch)
# Enable -Werror by default for git repositories only (i.e. developer builds)
if [ -e "$srcdir"/.git ]; then
@@ -61,13 +60,14 @@ else
fi
usage() {
+ [ -z "$processor" ] && processor=$(get_default_processor $arch)
cat <<-EOF
Usage: $0 [options]
Options include:
--arch=ARCH architecture to compile for ($arch). ARCH can be one of:
arm, arm64/aarch64, i386, ppc64, riscv32, riscv64, s390x, x86_64
- --processor=PROCESSOR processor to compile for ($default_processor). For arm and arm64, the
+ --processor=PROCESSOR processor to compile for ($processor). For arm and arm64, the
value 'max' is special and it will be passed directly to
qemu, bypassing the compiler. In this case, --cflags can be
used to compile for a specific processor.
Should be accurate enough, as far as I can tell. And I don't think there's
a need for $do_help: if the user does ./configure --help --arch=arm64, then
I think it's reasonable to expect that --help will be interpreted before
--arch is parsed.
Thanks,
Alex
