Re: [PATCH net-next v4 02/10] selftests: net: add helpers for running a command on other targets

[Petr Machata <petrm@nvidia.com>]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> On Mon, Mar 30, 2026 at 01:32:41PM +0200, Petr Machata wrote:
>> 
>> Petr Machata <petrm@nvidia.com> writes:
>> 
>> > Ioana Ciornei <ioana.ciornei@nxp.com> writes:
>> >
>> >> +run_on()
>> >> +{
>> >> +	local iface=$1; shift
>> >> +	local target="local:"
>> >> +
>> >> +	if declare -p TARGETS &>/dev/null; then
>> >> +		target="${TARGETS[$iface]}"
>> >
>> > So I think Jakub's runs fail because there's a shell export somewhere
>> > that gets inherited through make to the launched test. I guess it would
>> > be enough for the test to validate that TARGETS is an array, because
>> > those don't get inherited.
>> >
>> > Is there a reason not to reuse DRIVER_TEST_CONFORMANT as a tell though?
>> >
>> >> +	fi
>> >> +
>> >> +	__run_on "$target" "$@"
>> >> +}
>> >
>> > Does the latter helper need to be in net/lib.sh? Since it uses TARGETS,
>> > which are a forwarding/lib.sh concept, it seems misplaced there.
>> 
>> Oh, I see, there's an invocation from mac_get() in net/lib.sh itself.
>> Hummm. Not sure how to tackle this.
>> 
>> I think lib.sh might unset TARGETS explicitly? Or declare -A, but leave
>> empty? Since it's now an API, net/lib.sh needs to set it to a reasonable
>> value (or erase). Then forwarding/lib.sh might in theory rely on
>> existence of that variable and not have to declare it at all.
>> 
>> Or, maybe have a stub run_on() like this to satisfy the run_on() API:
>> 
>> run_on()
>> {
>> 	"$@"
>
>          shift; "$@"

Indeed!

>> }
>> 
>> And have the full-blown thing in forward/lib.sh. All the magic with
>> TARGETS really belongs to forwarding/lib.sh. Bash allows function
>> redefinition just fine, so a user importing just net/lib.sh would get
>> the stub, and forwarding/lib.sh users would get the full thing.
>
> I really like the idea of the stub run_on() in net/lib.sh. This would
> also give me the possibility to check for DRIVER_TEST_CONFORMANT = "yes"
> in the full-blown run_on() without having to move its default
> definition in net/lib.sh.
>
> I quickly changed to this approach and it seems to look good.

Yeah, I like it best as well.

Then I think you don't need to worry about the TARGETS export, because
forwarding/lib.sh will add the "array-ness" to whatever comes from the
outside, if anything, and the function will be able to use like that.