diff mbox series

[ovs-dev,v1,2/4] travis: Move x86-only addon packages

Message ID 1574237685-4747-1-git-send-email-Lance.Yang@arm.com
State Superseded
Headers show
Series Fix issues and enable Travis CI on arm | expand

Commit Message

Lance Yang Nov. 20, 2019, 8:14 a.m. UTC
To enable multiple CPU architectures support, it is necessary to move the
x86-only addon packages from .travis.yml file. Otherwise, the x86-only
addon packages will break the builds on some other CPU architectures.

Reviewed-by: Yangqin Wei <Yanqin.Wei@arm.com>
Reviewed-by: Malvika Gupta <Malvika.Gupta@arm.com>
Reviewed-by: Gavin Hu <Galvin.Hu@arm.com>
Reviewed-by: Ruifeng Wang <Ruifeng.Wang@arm.com>
Signed-off-by: Lance Yang <Lance.Yang@arm.com>
---
 .travis.yml              |  2 --
 .travis/linux-prepare.sh | 12 ++++++++----
 2 files changed, 8 insertions(+), 6 deletions(-)

Comments

Ilya Maximets Nov. 21, 2019, 7:30 p.m. UTC | #1
On 20.11.2019 9:14, Lance Yang wrote:
> To enable multiple CPU architectures support, it is necessary to move the
> x86-only addon packages from .travis.yml file. Otherwise, the x86-only
> addon packages will break the builds on some other CPU architectures.
> 
> Reviewed-by: Yangqin Wei <Yanqin.Wei@arm.com>
> Reviewed-by: Malvika Gupta <Malvika.Gupta@arm.com>
> Reviewed-by: Gavin Hu <Galvin.Hu@arm.com>
> Reviewed-by: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Signed-off-by: Lance Yang <Lance.Yang@arm.com>
> ---
>  .travis.yml              |  2 --
>  .travis/linux-prepare.sh | 12 ++++++++----
>  2 files changed, 8 insertions(+), 6 deletions(-)

Common comment for all the patches in a series:
* It's better to add a period in the end of a subject line.

> 
> diff --git a/.travis.yml b/.travis.yml
> index 482efd2..2dc4d43 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -14,7 +14,6 @@ addons:
>    apt:
>      packages:
>        - bc
> -      - gcc-multilib
>        - libssl-dev
>        - llvm-dev
>        - libjemalloc1
> @@ -26,7 +25,6 @@ addons:
>        - libelf-dev
>        - selinux-policy-dev
>        - libunbound-dev
> -      - libunbound-dev:i386
>        - libunwind-dev
>  
>  before_install: ./.travis/${TRAVIS_OS_NAME}-prepare.sh
> diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh
> index 9e3ac0d..8096abe 100755
> --- a/.travis/linux-prepare.sh
> +++ b/.travis/linux-prepare.sh
> @@ -15,10 +15,14 @@ cd ..
>  pip install --disable-pip-version-check --user six flake8 hacking
>  pip install --user --upgrade docutils
>  
> -if [ "$M32" ]; then
> -    # 32-bit and 64-bit libunwind can not be installed at the same time.
> -    # This will remove the 64-bit libunwind and install 32-bit version.
> -    sudo apt-get install -y libunwind-dev:i386
> +if [[ "$TRAVIS_ARCH" == "amd64" ]] || [[ -z "$TRAVIS_ARCH" ]]; then

The same comment here as for previous ppc64le patch.
Are you going to ever build 32bit binary on aarch64 on Travis?
Is it really possible to build 32bit binary on aarch64 with '-m32' flag?

> +    if [ "$M32" ]; then
> +        # 32-bit and 64-bit libunwind can not be installed at the same time.
> +        # This will remove the 64-bit libunwind and install 32-bit version.
> +        sudo apt-get install \
> +        -y libunwind-dev:i386 libunbound-dev:i386 gcc-multilib

Please, add additional indentation level for above line.

> +    fi
> +
>  fi
>  
>  # IPv6 is supported by kernel but disabled in TravisCI images:
>
Yanqin Wei Nov. 22, 2019, 9:23 a.m. UTC | #2
Hi Ilya,

Reply inline.

Best Regards,
Wei Yanqin

> -----Original Message-----
> From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of Ilya Maximets
> Sent: Friday, November 22, 2019 3:30 AM
> To: Lance Yang (Arm Technology China) <Lance.Yang@arm.com>;
> dev@openvswitch.org; ovs-dev@openvswitch.org
> Cc: Jieqiang Wang (Arm Technology China) <Jieqiang.Wang@arm.com>;
> Ruifeng Wang (Arm Technology China) <Ruifeng.Wang@arm.com>; Gavin Hu
> (Arm Technology China) <Gavin.Hu@arm.com>; Jingzhao Ni (Arm Technology
> China) <Jingzhao.Ni@arm.com>; nd <nd@arm.com>
> Subject: Re: [ovs-dev] [PATCH v1 2/4] travis: Move x86-only addon packages
> 
> On 20.11.2019 9:14, Lance Yang wrote:
> > To enable multiple CPU architectures support, it is necessary to move
> > the x86-only addon packages from .travis.yml file. Otherwise, the
> > x86-only addon packages will break the builds on some other CPU
> architectures.
> >
> > Reviewed-by: Yangqin Wei <Yanqin.Wei@arm.com>
> > Reviewed-by: Malvika Gupta <Malvika.Gupta@arm.com>
> > Reviewed-by: Gavin Hu <Galvin.Hu@arm.com>
> > Reviewed-by: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > Signed-off-by: Lance Yang <Lance.Yang@arm.com>
> > ---
> >  .travis.yml              |  2 --
> >  .travis/linux-prepare.sh | 12 ++++++++----
> >  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> Common comment for all the patches in a series:
> * It's better to add a period in the end of a subject line.
[Yanqin] OK.
> 
> >
> > diff --git a/.travis.yml b/.travis.yml index 482efd2..2dc4d43 100644
> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -14,7 +14,6 @@ addons:
> >    apt:
> >      packages:
> >        - bc
> > -      - gcc-multilib
> >        - libssl-dev
> >        - llvm-dev
> >        - libjemalloc1
> > @@ -26,7 +25,6 @@ addons:
> >        - libelf-dev
> >        - selinux-policy-dev
> >        - libunbound-dev
> > -      - libunbound-dev:i386
> >        - libunwind-dev
> >
> >  before_install: ./.travis/${TRAVIS_OS_NAME}-prepare.sh
> > diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh index
> > 9e3ac0d..8096abe 100755
> > --- a/.travis/linux-prepare.sh
> > +++ b/.travis/linux-prepare.sh
> > @@ -15,10 +15,14 @@ cd ..
> >  pip install --disable-pip-version-check --user six flake8 hacking
> > pip install --user --upgrade docutils
> >
> > -if [ "$M32" ]; then
> > -    # 32-bit and 64-bit libunwind can not be installed at the same time.
> > -    # This will remove the 64-bit libunwind and install 32-bit version.
> > -    sudo apt-get install -y libunwind-dev:i386
> > +if [[ "$TRAVIS_ARCH" == "amd64" ]] || [[ -z "$TRAVIS_ARCH" ]]; then
> 
> The same comment here as for previous ppc64le patch.
> Are you going to ever build 32bit binary on aarch64 on Travis?
> Is it really possible to build 32bit binary on aarch64 with '-m32' flag?
[Yanqin] Not yet. Gcc for aarch64 does not support -m32 flag. Cross compiler is required to build 32 bits binary on aarch64 machine.

> 
> > +    if [ "$M32" ]; then
> > +        # 32-bit and 64-bit libunwind can not be installed at the same time.
> > +        # This will remove the 64-bit libunwind and install 32-bit version.
> > +        sudo apt-get install \
> > +        -y libunwind-dev:i386 libunbound-dev:i386 gcc-multilib
> 
> Please, add additional indentation level for above line.
[Yanqin] Thanks, will be updated in V2.
> 
> > +    fi
> > +
> >  fi
> >
> >  # IPv6 is supported by kernel but disabled in TravisCI images:
> >
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ilya Maximets Nov. 28, 2019, 7:04 p.m. UTC | #3
On 22.11.2019 10:23, Yanqin Wei (Arm Technology China) wrote:
> Hi Ilya,
> 
> Reply inline.
> 
> Best Regards,
> Wei Yanqin
> 
>> -----Original Message-----
>> From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of Ilya Maximets
>> Sent: Friday, November 22, 2019 3:30 AM
>> To: Lance Yang (Arm Technology China) <Lance.Yang@arm.com>;
>> dev@openvswitch.org; ovs-dev@openvswitch.org
>> Cc: Jieqiang Wang (Arm Technology China) <Jieqiang.Wang@arm.com>;
>> Ruifeng Wang (Arm Technology China) <Ruifeng.Wang@arm.com>; Gavin Hu
>> (Arm Technology China) <Gavin.Hu@arm.com>; Jingzhao Ni (Arm Technology
>> China) <Jingzhao.Ni@arm.com>; nd <nd@arm.com>
>> Subject: Re: [ovs-dev] [PATCH v1 2/4] travis: Move x86-only addon packages
>>
>> On 20.11.2019 9:14, Lance Yang wrote:
>>> To enable multiple CPU architectures support, it is necessary to move
>>> the x86-only addon packages from .travis.yml file. Otherwise, the
>>> x86-only addon packages will break the builds on some other CPU
>> architectures.
>>>
>>> Reviewed-by: Yangqin Wei <Yanqin.Wei@arm.com>
>>> Reviewed-by: Malvika Gupta <Malvika.Gupta@arm.com>
>>> Reviewed-by: Gavin Hu <Galvin.Hu@arm.com>
>>> Reviewed-by: Ruifeng Wang <Ruifeng.Wang@arm.com>
>>> Signed-off-by: Lance Yang <Lance.Yang@arm.com>
>>> ---
>>>  .travis.yml              |  2 --
>>>  .travis/linux-prepare.sh | 12 ++++++++----
>>>  2 files changed, 8 insertions(+), 6 deletions(-)
>>
>> Common comment for all the patches in a series:
>> * It's better to add a period in the end of a subject line.
> [Yanqin] OK.
>>
>>>
>>> diff --git a/.travis.yml b/.travis.yml index 482efd2..2dc4d43 100644
>>> --- a/.travis.yml
>>> +++ b/.travis.yml
>>> @@ -14,7 +14,6 @@ addons:
>>>    apt:
>>>      packages:
>>>        - bc
>>> -      - gcc-multilib
>>>        - libssl-dev
>>>        - llvm-dev
>>>        - libjemalloc1
>>> @@ -26,7 +25,6 @@ addons:
>>>        - libelf-dev
>>>        - selinux-policy-dev
>>>        - libunbound-dev
>>> -      - libunbound-dev:i386
>>>        - libunwind-dev
>>>
>>>  before_install: ./.travis/${TRAVIS_OS_NAME}-prepare.sh
>>> diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh index
>>> 9e3ac0d..8096abe 100755
>>> --- a/.travis/linux-prepare.sh
>>> +++ b/.travis/linux-prepare.sh
>>> @@ -15,10 +15,14 @@ cd ..
>>>  pip install --disable-pip-version-check --user six flake8 hacking
>>> pip install --user --upgrade docutils
>>>
>>> -if [ "$M32" ]; then
>>> -    # 32-bit and 64-bit libunwind can not be installed at the same time.
>>> -    # This will remove the 64-bit libunwind and install 32-bit version.
>>> -    sudo apt-get install -y libunwind-dev:i386
>>> +if [[ "$TRAVIS_ARCH" == "amd64" ]] || [[ -z "$TRAVIS_ARCH" ]]; then
>>
>> The same comment here as for previous ppc64le patch.
>> Are you going to ever build 32bit binary on aarch64 on Travis?
>> Is it really possible to build 32bit binary on aarch64 with '-m32' flag?
> [Yanqin] Not yet. Gcc for aarch64 does not support -m32 flag. Cross compiler is required to build 32 bits binary on aarch64 machine.

In this case I don't think that we need to check for ARCH here.
It's enough to check for M32.

> 
>>
>>> +    if [ "$M32" ]; then
>>> +        # 32-bit and 64-bit libunwind can not be installed at the same time.
>>> +        # This will remove the 64-bit libunwind and install 32-bit version.
>>> +        sudo apt-get install \
>>> +        -y libunwind-dev:i386 libunbound-dev:i386 gcc-multilib
>>
>> Please, add additional indentation level for above line.
> [Yanqin] Thanks, will be updated in V2.
>>
>>> +    fi
>>> +
>>>  fi
>>>
>>>  # IPv6 is supported by kernel but disabled in TravisCI images:
>>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/.travis.yml b/.travis.yml
index 482efd2..2dc4d43 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -14,7 +14,6 @@  addons:
   apt:
     packages:
       - bc
-      - gcc-multilib
       - libssl-dev
       - llvm-dev
       - libjemalloc1
@@ -26,7 +25,6 @@  addons:
       - libelf-dev
       - selinux-policy-dev
       - libunbound-dev
-      - libunbound-dev:i386
       - libunwind-dev
 
 before_install: ./.travis/${TRAVIS_OS_NAME}-prepare.sh
diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh
index 9e3ac0d..8096abe 100755
--- a/.travis/linux-prepare.sh
+++ b/.travis/linux-prepare.sh
@@ -15,10 +15,14 @@  cd ..
 pip install --disable-pip-version-check --user six flake8 hacking
 pip install --user --upgrade docutils
 
-if [ "$M32" ]; then
-    # 32-bit and 64-bit libunwind can not be installed at the same time.
-    # This will remove the 64-bit libunwind and install 32-bit version.
-    sudo apt-get install -y libunwind-dev:i386
+if [[ "$TRAVIS_ARCH" == "amd64" ]] || [[ -z "$TRAVIS_ARCH" ]]; then
+    if [ "$M32" ]; then
+        # 32-bit and 64-bit libunwind can not be installed at the same time.
+        # This will remove the 64-bit libunwind and install 32-bit version.
+        sudo apt-get install \
+        -y libunwind-dev:i386 libunbound-dev:i386 gcc-multilib
+    fi
+
 fi
 
 # IPv6 is supported by kernel but disabled in TravisCI images: