Re: [PATCH v1 3/6] configure: also skip deprecated targets with target-list-exclude

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 14 Sep 2020 at 16:27, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Now the user has to make an even more deliberate decision to
>> enable a deprecated target rather than getting it as a side effect of
>> using --target-exclude-list.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  configure | 12 +++++++++---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/configure b/configure
>> index e365a90cc133..50052378e417 100755
>> --- a/configure
>> +++ b/configure
>> @@ -1722,9 +1722,15 @@ if [ "$bsd_user" = "yes" ]; then
>>      mak_wilds="${mak_wilds} $source_path/default-configs/*-bsd-user.mak"
>>  fi
>>
>> -if test -z "$target_list_exclude" -a -z "$target_list"; then
>> -    # if the user doesn't specify anything lets skip deprecating stuff
>> -    target_list_exclude=ppc64abi32-linux-user
>> +# If the user doesn't explicitly specify a deprecated target we will
>> +# skip it.
>> +if test -z "$target_list"; then
>> +    deprecated_targets_list=ppc64abi32-linux-user
>
> If you put this variable setting outside the if...
>
>> +    if test -z "$target_list_exclude"; then
>> +        target_list_exclude="$deprecated_targets_list"
>> +    else
>> +        target_list_exclude="$target_list_exclude,$deprecated_targets_list"
>> +    fi
>>  fi
>>
>>  exclude_list=$(echo "$target_list_exclude" | sed -e 's/,/ /g')
>
> then later on once we've parsed the exclude list and set
> default_target_list we can say something like (untested!)
>
> for dep_target in $(echo "$deprecated_targets_list" | sed -e 's/,/ /g'); do
>     for target in $default_target_list; do
>         if "$dep_target" = "target"; then
>             add_to deprecated_features $target
>             break
>         fi
>     done
> done
>
> and then the only thing necessary to add another deprecated target
> will be to add it to the variable (ie we can just delete the
> add_to line your patch 2 puts in as we don't need to modify
> that code at all any more).

That makes sense - actually I think we can do:

    # if a deprecated target is enabled we note it here
    if echo "$deprecated_targets_list" | grep -q "$1"; then
        add_to deprecated_features $1
    fi

just before test "$tcg" = "yes" && return 0 in supported_target.

>
> (Side note: I just followed the code we have currently for
> doing the "is the target in the exclude list" but this
> seems tremendously clunky given we're really just trying
> to ask "is string X in set Y"...)

Yeah that can be cleaned up.

>
> thanks
> -- PMM


-- 
Alex Bennée