Re: [PATCH 1/2] build: add backward compatibility for nested drivers

[Kevin Traynor <ktraynor@redhat.com>]

On 22/09/2025 16:51, Thomas Monjalon wrote:
> 22/09/2025 13:07, Kevin Traynor:
>> --- a/drivers/meson.build
>> +++ b/drivers/meson.build
>> -# add cmdline disabled drivers and meson disabled drivers together
>> -disable_drivers += ',' + get_option('disable_drivers')
>> +# map legacy driver names
>> +driver_map = {
>> +    'net/e1000': 'net/intel/e1000',
>> +    'net/fm10k': 'net/intel/fm10k',
>> +    'net/i40e': 'net/intel/i40e',
>> +    'net/iavf': 'net/intel/iavf',
>> +    'net/ice': 'net/intel/ice',
>> +    'net/idpf': 'net/intel/idpf',
>> +    'net/ipn3ke': 'net/intel/ipn3ke',
>> +    'net/ixgbe': 'net/intel/ixgbe',
>> +    'net/cpfl': 'net/intel/cpfl',
>> +}
> 
> Should we build this list inside a file drivers/net/intel/meson.build ?
> I'm not sure my idea is better...
> 

Not sure. It's generic as is, so any other driver moved anywhere could
add to this list here easily. Let's see what others think.

>> +
>> +# add cmdline drivers
>> +foreach driver_type : [['disable', get_option('disable_drivers')],
>> +                       ['enable', get_option('enable_drivers')]]
>> +    driver_list_name = driver_type[0] + '_drivers'
>> +    cmdline_drivers = ',' + driver_type[1]
>> +
>> +    foreach driver : cmdline_drivers.split(',')
>> +        if driver_map.has_key(driver)
> 
> I feel we need comments for above parsing.
> 

Ack, will add

>> +            driver_mapped = driver_map[driver]
>> +            warning('Driver name "@0@" is deprecated, please use "@1@" instead.'
> 
> Not sure about this warning.
> We can keep compatibility without saying it is deprecated.
> 

Yes, that is a good point for discussion. Seen as support for the legacy
names were already dropped and I wasn't aware of any ABI like policy
about it, I thought there may be a preference for deprecation
warning/continuing to move to the new name only.

I would be happy to keep the legacy name without a warning/deprecation
for a longer term and we could adopt this as general guideline by
default too. It should not cost much effort to do this.

Another minor point is, if this needs a Fixes tag? Yes, in the sense it
feels like it added a banana skin for users (the patches are because I
hit this issue with 25.07). I didn't add it for now, as no guarantees
were broken and there isn't an upstream stable for backporting to anyway.

>> +                    .format(driver, driver_mapped))
>> +            driver = driver_mapped
>> +        endif
>> +        if driver_list_name == 'disable_drivers'
>> +            disable_drivers += ',' + driver
>> +        else
>> +            enable_drivers += ',' + driver
>> +        endif
>> +    endforeach
>> +endforeach
>> +
>> +# add cmdline drivers and meson drivers together
> 
> This comment is not clear.
> 

I will remove this, it was previously for the enable/disable blocks of
code, but only relevant to the line that was expanded into the loop
above not the below one.

>>  disable_drivers = run_command(list_dir_globs, disable_drivers, check: true).stdout().split()
>> -
>> -# add cmdline enabled drivers and meson enabled drivers together
>> -enable_drivers = ',' + get_option('enable_drivers')
> 
>