On 11/28/23 19:15, Eelco Chaudron wrote:
>
>
> On 28 Nov 2023, at 13:07, Ilya Maximets wrote:
>
>> On 11/28/23 08:39, Eelco Chaudron wrote:
>>>
>>>
>>> On 27 Nov 2023, at 18:58, Ilya Maximets wrote:
>>>
>>>> On 11/27/23 13:38, Eelco Chaudron wrote:
>>>>> Signed-off-by: Eelco Chaudron <echau...@redhat.com>
>>>>> ---
>>>>> .ci/linux-build.sh | 9 +++++----
>>>>> .github/workflows/build-and-test.yml | 3 +++
>>>>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>>>>> index aa2ecc505..e9e1e24b5 100755
>>>>> --- a/.ci/linux-build.sh
>>>>> +++ b/.ci/linux-build.sh
>>>>> @@ -6,6 +6,7 @@ set -x
>>>>> CFLAGS_FOR_OVS="-g -O2"
>>>>> SPARSE_FLAGS=""
>>>>> EXTRA_OPTS="--enable-Werror"
>>>>> +JOBS=${JOBS:-"-j4"}
>>>>
>>>> Is this used anywhere? Seems a little orthogonal to the
>>>> purpose of this set.
>>>
>>> Thought rather than adding -j4 in more places, having a seperate definition
>>> for it would allow it to be changed easily if we ever need to (and when
>>> using the script outside of GitHub actions).
>>>
>>> I can make it a seperate patch if that is preferred.
>>
>> Would be better. The only thing that annoys me is that the definition
>> above allows passing JOBS via environment in order to override, but no
>> other variable is defined this way, and the functionality is not actually
>> used.
>
> I’ll make it a seperate patch. I know it’s the only value so far but for a
> reason :) If I want to use this script in my own container for testing I can
> give it more cores, and it will run faster.
Hmm. I didn't consider this scenario. :)
Let's keep it then, no problem.
>
> But if you really do not like it, I’ll do some ‘sed’ in my scripts to make a
> hard-coded change.
>
>>>>> function install_dpdk()
>>>>> {
>>>>> @@ -46,7 +47,7 @@ function build_ovs()
>>>>> configure_ovs $OPTS
>>>>> make selinux-policy
>>>>>
>>>>> - make -j4
>>>>> + make $JOBS
>>>>> }
>>>>>
>>>>> if [ "$DEB_PACKAGE" ]; then
>>>>> @@ -122,8 +123,8 @@ if [ "$TESTSUITE" = 'test' ]; then
>>>>> configure_ovs
>>>>>
>>>>> export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
>>>>> - make distcheck -j4 CFLAGS="${CFLAGS_FOR_OVS}" \
>>>>> - TESTSUITEFLAGS=-j4 RECHECK=yes
>>>>> + make distcheck $JOBS CFLAGS="${CFLAGS_FOR_OVS}" \
>>>>> + TESTSUITEFLAGS=$JOBS RECHECK=yes
>>
>> Nit: It's better to use ${JOBS} syntax. Not necessary though.
>
> Will fix those.
>
>>>>> else
>>>>> build_ovs
>>>>> for testsuite in $TESTSUITE; do
>>>>> @@ -134,7 +135,7 @@ else
>>>>> export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1"
>>>>> run_as_root="sudo -E PATH=$PATH"
>>>>> fi
>>>>> - $run_as_root make $testsuite TESTSUITEFLAGS=-j4 RECHECK=yes
>>>>> + $run_as_root make $testsuite TESTSUITEFLAGS=$JOBS RECHECK=yes
>>
>> Ditto.
>>
>>>>> done
>>>>> fi
>>>>>
>>>>> diff --git a/.github/workflows/build-and-test.yml
>>>>> b/.github/workflows/build-and-test.yml
>>>>> index 09654205e..5d441157c 100644
>>>>> --- a/.github/workflows/build-and-test.yml
>>>>> +++ b/.github/workflows/build-and-test.yml
>>>>> @@ -164,6 +164,9 @@ jobs:
>>>>> m32: m32
>>>>> opts: --disable-ssl
>>>>>
>>>>> + - compiler: gcc
>>>>> + testsuite: check-ovsdb-cluster
>>>>
>>>> FWIW, there is no need to run this testsuite as root.
>>>> But I'm not sure it worth changing the scripts in order
>>>> to avoid that.
>>>
>>> ACK, will keep it as is.
>>>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
