Hi Ihar,

On 12/12/25 2:58 AM, Ihar Hrachyshka wrote:
> On Mon, Dec 8, 2025 at 10:45 AM Ilya Maximets <[email protected]> wrote:
> 
>> On 12/8/25 3:26 PM, Dumitru Ceara via dev wrote:
>>> On 12/7/25 9:53 PM, Ihar Hrachyshka wrote:
>>>> `find_command` test helper assumes `which` is present on the system. If
>>>> it is not, the helper silently fails to detect an installed command
>>>> presence, skipping tests that a caller could expect to run (since the
>>>> auxiliary commands *are* installed).
>>>>
>>>> Other build scripts and test cases also use the command, and we install
>>>> it in the fedora container too.
>>>>
>>>> This patch enforces `which` presence via autoconf.
>>>>
>>>> Signed-off-by: Ihar Hrachyshka <[email protected]>
>>>> ---
>>>
>>> Hi Ihar,
>>>
>>> Thanks for the patch!
>>>
>>>>  configure.ac | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/configure.ac b/configure.ac
>>>> index 2f67f9512..ef4ce9ad6 100644
>>>> --- a/configure.ac
>>>> +++ b/configure.ac
>>>> @@ -29,6 +29,9 @@ AC_PROG_CPP
>>>>  AC_PROG_MKDIR_P
>>>>  AC_PROG_FGREP
>>>>  AC_PROG_EGREP
>>>> +AC_PATH_PROG([WHICH], [which])
>>>> +AS_IF([test -z "$WHICH"],
>>>> +      [AC_MSG_ERROR([which command is required to build OVN])])
>>>>  PKG_PROG_PKG_CONFIG
>>>>
>>>>  AM_MISSING_PROG([AUTOM4TE], [autom4te])
>>>
>>> It turns out we also need to update the ovn-kubernetes container image
>>> (that we build ourselves in CI, .ci/ovn-kubernetes/Dockerfile).
>>> Otherwise, we get:
>>>
>>> #37 0.775 checking for which... no
>>> #37 0.775 configure: error: which command is required to build OVN
>>> #37 ERROR: process "/bin/sh -c ./configure" did not complete
>>> successfully: exit code: 1
>>
>>
>> Clearly, we can build without which. :) We shouldn't make it a build
>> dependency, if it's just a test dependency.  The build and test systems
>>
> 
> Fair! Do you agree that we should fail a test run early if `which` is
> missing?
> 
> If so, we can then enforce it for test runs only. For example, we can
> modify `find_command` so that it fails if `which which` fails, to achieve
> the same enforcement effect. WDYT?
> 

That sounds reasonable to me.

> 
>> are not necessarily the same system.
>>
>> What other scripts require it?
>>
> 
> There's nothing intrinsic to the *build* process (so the above point is
> well taken), but `clang-analyze` target uses it too.
> 
> (While looking around, I realized that `find_command` was copied from the
> `ovs` repository, so probably whatever comes out of this patch could be
> applied there too.)
> 

I think so too.

> Ihar
> 

Regards,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to