[Linaro-uefi] atf-build.sh: fix to calculate atf version

Message ID 1539922549-5432-1-git-send-email-haojian.zhuang@linaro.org
State New
Headers show
Series
  • [Linaro-uefi] atf-build.sh: fix to calculate atf version
Related show

Commit Message

Haojian Zhuang Oct. 19, 2018, 4:15 a.m.
commit 1a0f11213a539a5d0c5d05bfaa68ccc75cb340aa
Author: Soby Mathew <soby.mathew@arm.com>
Date:   Mon Oct 1 16:16:34 2018 +0100

    Update the version to 2.0

    Change-Id: Icbc556d47a58d0870577b1bf1cd27cc5827fd56d
    Signed-off-by: Soby Mathew <soby.mathew@arm.com>

Since ARM Trusted Firmware version is updated to 2.0, the calculation
of ATF is wrong. It could only calculate version for 1.x.

Update the calculation to support 2.x or n.x.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 atf-build.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Leif Lindholm Oct. 19, 2018, 4:30 a.m. | #1
On Fri, Oct 19, 2018 at 12:15:49PM +0800, Haojian Zhuang wrote:
> commit 1a0f11213a539a5d0c5d05bfaa68ccc75cb340aa
> Author: Soby Mathew <soby.mathew@arm.com>
> Date:   Mon Oct 1 16:16:34 2018 +0100
> 
>     Update the version to 2.0
> 
>     Change-Id: Icbc556d47a58d0870577b1bf1cd27cc5827fd56d
>     Signed-off-by: Soby Mathew <soby.mathew@arm.com>
> 
> Since ARM Trusted Firmware version is updated to 2.0, the calculation
> of ATF is wrong. It could only calculate version for 1.x.

Whoops.

> Update the calculation to support 2.x or n.x.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  atf-build.sh | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/atf-build.sh b/atf-build.sh
> index fb80ad0..c6ec3e3 100755
> --- a/atf-build.sh
> +++ b/atf-build.sh
> @@ -25,9 +25,12 @@ function check_atf_buildver
>  	MAJOR=`grep "^VERSION_MAJOR" Makefile | sed 's/.*:= *\([0-9]*\).*/\1/'`
>  	[ $? -ne 0 ] && return 1
>  	MINOR=`grep "^VERSION_MINOR" Makefile | sed 's/.*:= *\([0-9]*\).*/\1/'`
> -	[ $? -ne 0 ] && return 1

Why is this test deleted? This checks the command success, not the version number.

> -	if [ "$MAJOR" -eq 1 -a "$MINOR" -ge 2 ]; then
> +	if [ "$MAJOR" -eq 1 ]; then
> +		if [ "$MINOR" -ge 2 ]; then
> +			ATF_BUILDVER=2
> +		fi
> +	else

This bit is clearly a bugfix.

/
    Leif

>  		ATF_BUILDVER=2
>  	fi
>  }
> -- 
> 2.7.4
>
Haojian Zhuang Oct. 19, 2018, 4:32 a.m. | #2
On Fri, 19 Oct 2018 at 12:30, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Fri, Oct 19, 2018 at 12:15:49PM +0800, Haojian Zhuang wrote:
> > commit 1a0f11213a539a5d0c5d05bfaa68ccc75cb340aa
> > Author: Soby Mathew <soby.mathew@arm.com>
> > Date:   Mon Oct 1 16:16:34 2018 +0100
> >
> >     Update the version to 2.0
> >
> >     Change-Id: Icbc556d47a58d0870577b1bf1cd27cc5827fd56d
> >     Signed-off-by: Soby Mathew <soby.mathew@arm.com>
> >
> > Since ARM Trusted Firmware version is updated to 2.0, the calculation
> > of ATF is wrong. It could only calculate version for 1.x.
>
> Whoops.
>
> > Update the calculation to support 2.x or n.x.
> >
> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> > ---
> >  atf-build.sh | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/atf-build.sh b/atf-build.sh
> > index fb80ad0..c6ec3e3 100755
> > --- a/atf-build.sh
> > +++ b/atf-build.sh
> > @@ -25,9 +25,12 @@ function check_atf_buildver
> >       MAJOR=`grep "^VERSION_MAJOR" Makefile | sed 's/.*:= *\([0-9]*\).*/\1/'`
> >       [ $? -ne 0 ] && return 1
> >       MINOR=`grep "^VERSION_MINOR" Makefile | sed 's/.*:= *\([0-9]*\).*/\1/'`
> > -     [ $? -ne 0 ] && return 1
>
> Why is this test deleted? This checks the command success, not the version number.
>

When the version is 2.0, the MINOR variable is 0. If we keep checking
at here, we'll
get exit and ATF_BUILDVER keeps in 1. So we should only check MAJOR variable.

Best Regards
Haojian
Leif Lindholm Oct. 19, 2018, 4:37 a.m. | #3
On Fri, Oct 19, 2018 at 12:32:47PM +0800, Haojian Zhuang wrote:
> On Fri, 19 Oct 2018 at 12:30, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Fri, Oct 19, 2018 at 12:15:49PM +0800, Haojian Zhuang wrote:
> > > commit 1a0f11213a539a5d0c5d05bfaa68ccc75cb340aa
> > > Author: Soby Mathew <soby.mathew@arm.com>
> > > Date:   Mon Oct 1 16:16:34 2018 +0100
> > >
> > >     Update the version to 2.0
> > >
> > >     Change-Id: Icbc556d47a58d0870577b1bf1cd27cc5827fd56d
> > >     Signed-off-by: Soby Mathew <soby.mathew@arm.com>
> > >
> > > Since ARM Trusted Firmware version is updated to 2.0, the calculation
> > > of ATF is wrong. It could only calculate version for 1.x.
> >
> > Whoops.
> >
> > > Update the calculation to support 2.x or n.x.
> > >
> > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> > > ---
> > >  atf-build.sh | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/atf-build.sh b/atf-build.sh
> > > index fb80ad0..c6ec3e3 100755
> > > --- a/atf-build.sh
> > > +++ b/atf-build.sh
> > > @@ -25,9 +25,12 @@ function check_atf_buildver
> > >       MAJOR=`grep "^VERSION_MAJOR" Makefile | sed 's/.*:= *\([0-9]*\).*/\1/'`
> > >       [ $? -ne 0 ] && return 1
> > >       MINOR=`grep "^VERSION_MINOR" Makefile | sed 's/.*:= *\([0-9]*\).*/\1/'`
> > > -     [ $? -ne 0 ] && return 1
> >
> > Why is this test deleted? This checks the command success, not the version number.
> >
> 
> When the version is 2.0, the MINOR variable is 0. If we keep checking
> at here, we'll
> get exit and ATF_BUILDVER keeps in 1. So we should only check MAJOR variable.

I understand that.
But why are you deleting a test for whether the grep command was
successful?

Regards,

Leif
Haojian Zhuang Oct. 19, 2018, 4:51 a.m. | #4
Fixed.



Best Regards

Haojian



________________________________
From: Leif Lindholm <leif.lindholm@linaro.org>

Sent: Friday, October 19, 2018 12:37:33 PM
To: Haojian Zhuang
Cc: Linaro UEFI Mailman List; Ard Biesheuvel
Subject: Re: [PATCH] atf-build.sh: fix to calculate atf version

On Fri, Oct 19, 2018 at 12:32:47PM +0800, Haojian Zhuang wrote:
> On Fri, 19 Oct 2018 at 12:30, Leif Lindholm <leif.lindholm@linaro.org> wrote:

> >

> > On Fri, Oct 19, 2018 at 12:15:49PM +0800, Haojian Zhuang wrote:

> > > commit 1a0f11213a539a5d0c5d05bfaa68ccc75cb340aa

> > > Author: Soby Mathew <soby.mathew@arm.com>

> > > Date:   Mon Oct 1 16:16:34 2018 +0100

> > >

> > >     Update the version to 2.0

> > >

> > >     Change-Id: Icbc556d47a58d0870577b1bf1cd27cc5827fd56d

> > >     Signed-off-by: Soby Mathew <soby.mathew@arm.com>

> > >

> > > Since ARM Trusted Firmware version is updated to 2.0, the calculation

> > > of ATF is wrong. It could only calculate version for 1.x.

> >

> > Whoops.

> >

> > > Update the calculation to support 2.x or n.x.

> > >

> > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

> > > ---

> > >  atf-build.sh | 7 +++++--

> > >  1 file changed, 5 insertions(+), 2 deletions(-)

> > >

> > > diff --git a/atf-build.sh b/atf-build.sh

> > > index fb80ad0..c6ec3e3 100755

> > > --- a/atf-build.sh

> > > +++ b/atf-build.sh

> > > @@ -25,9 +25,12 @@ function check_atf_buildver

> > >       MAJOR=`grep "^VERSION_MAJOR" Makefile | sed 's/.*:= *\([0-9]*\).*/\1/'`

> > >       [ $? -ne 0 ] && return 1

> > >       MINOR=`grep "^VERSION_MINOR" Makefile | sed 's/.*:= *\([0-9]*\).*/\1/'`

> > > -     [ $? -ne 0 ] && return 1

> >

> > Why is this test deleted? This checks the command success, not the version number.

> >

>

> When the version is 2.0, the MINOR variable is 0. If we keep checking

> at here, we'll

> get exit and ATF_BUILDVER keeps in 1. So we should only check MAJOR variable.


I understand that.
But why are you deleting a test for whether the grep command was
successful?

Regards,

Leif
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from text --><style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: #800000 2px solid; } --></style>
</head>
<body>
<meta content="text/html; charset=utf-8">
<meta name="x_Generator" content="Microsoft Word 15 (filtered medium)">
<style>
<!--
@font-face
	{font-family:"Cambria Math"}
@font-face
	{font-family:DengXian}
@font-face
	{}
p.x_MsoNormal, li.x_MsoNormal, div.x_MsoNormal
	{margin:0cm;
	margin-bottom:.0001pt;
	font-size:11.0pt;
	font-family:DengXian}
a:x_link, span.x_MsoHyperlink
	{color:blue;
	text-decoration:underline}
a:x_visited, span.x_MsoHyperlinkFollowed
	{color:#954F72;
	text-decoration:underline}
.x_MsoChpDefault
	{}
@page WordSection1
	{margin:72.0pt 90.0pt 72.0pt 90.0pt}
div.x_WordSection1
	{}
-->
</style>
<div lang="ZH-CN" link="blue" vlink="#954F72">
<div class="x_WordSection1">
<p class="x_MsoNormal"><span lang="EN-US">Fixed.</span></p>
<p class="x_MsoNormal"><span lang="EN-US" style="font-size:12.0pt">&nbsp;</span></p>
<p class="x_MsoNormal"><span lang="EN-US" style="font-size:12.0pt">Best Regards</span></p>
<p class="x_MsoNormal"><span lang="EN-US" style="font-size:12.0pt">Haojian</span></p>
<p class="x_MsoNormal"><span lang="EN-US">&nbsp;</span></p>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Leif Lindholm &lt;leif.lindholm@linaro.org&gt;<br>
<b>Sent:</b> Friday, October 19, 2018 12:37:33 PM<br>
<b>To:</b> Haojian Zhuang<br>
<b>Cc:</b> Linaro UEFI Mailman List; Ard Biesheuvel<br>
<b>Subject:</b> Re: [PATCH] atf-build.sh: fix to calculate atf version</font>
<div>&nbsp;</div>
</div>
</div>
<font size="2"><span style="font-size:11pt;">
<div class="PlainText">On Fri, Oct 19, 2018 at 12:32:47PM &#43;0800, Haojian Zhuang wrote:<br>
&gt; On Fri, 19 Oct 2018 at 12:30, Leif Lindholm &lt;leif.lindholm@linaro.org&gt; wrote:<br>
&gt; &gt;<br>
&gt; &gt; On Fri, Oct 19, 2018 at 12:15:49PM &#43;0800, Haojian Zhuang wrote:<br>
&gt; &gt; &gt; commit 1a0f11213a539a5d0c5d05bfaa68ccc75cb340aa<br>
&gt; &gt; &gt; Author: Soby Mathew &lt;soby.mathew@arm.com&gt;<br>
&gt; &gt; &gt; Date:&nbsp;&nbsp; Mon Oct 1 16:16:34 2018 &#43;0100<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp; Update the version to 2.0<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp; Change-Id: Icbc556d47a58d0870577b1bf1cd27cc5827fd56d<br>
&gt; &gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp; Signed-off-by: Soby Mathew &lt;soby.mathew@arm.com&gt;<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; Since ARM Trusted Firmware version is updated to 2.0, the calculation<br>
&gt; &gt; &gt; of ATF is wrong. It could only calculate version for 1.x.<br>
&gt; &gt;<br>
&gt; &gt; Whoops.<br>
&gt; &gt;<br>
&gt; &gt; &gt; Update the calculation to support 2.x or n.x.<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; Signed-off-by: Haojian Zhuang &lt;haojian.zhuang@linaro.org&gt;<br>
&gt; &gt; &gt; ---<br>
&gt; &gt; &gt;&nbsp; atf-build.sh | 7 &#43;&#43;&#43;&#43;&#43;--<br>
&gt; &gt; &gt;&nbsp; 1 file changed, 5 insertions(&#43;), 2 deletions(-)<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; diff --git a/atf-build.sh b/atf-build.sh<br>
&gt; &gt; &gt; index fb80ad0..c6ec3e3 100755<br>
&gt; &gt; &gt; --- a/atf-build.sh<br>
&gt; &gt; &gt; &#43;&#43;&#43; b/atf-build.sh<br>
&gt; &gt; &gt; @@ -25,9 &#43;25,12 @@ function check_atf_buildver<br>
&gt; &gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; MAJOR=`grep &quot;^VERSION_MAJOR&quot; Makefile | sed 's/.*:= *\([0-9]*\).*/\1/'`<br>
&gt; &gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; [ $? -ne 0 ] &amp;&amp; return 1<br>
&gt; &gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; MINOR=`grep &quot;^VERSION_MINOR&quot; Makefile | sed 's/.*:= *\([0-9]*\).*/\1/'`<br>
&gt; &gt; &gt; -&nbsp;&nbsp;&nbsp;&nbsp; [ $? -ne 0 ] &amp;&amp; return 1<br>
&gt; &gt;<br>
&gt; &gt; Why is this test deleted? This checks the command success, not the version number.<br>
&gt; &gt;<br>
&gt; <br>
&gt; When the version is 2.0, the MINOR variable is 0. If we keep checking<br>
&gt; at here, we'll<br>
&gt; get exit and ATF_BUILDVER keeps in 1. So we should only check MAJOR variable.<br>
<br>
I understand that.<br>
But why are you deleting a test for whether the grep command was<br>
successful?<br>
<br>
Regards,<br>
<br>
Leif<br>
</div>
</span></font>
</body>
</html>

Patch

diff --git a/atf-build.sh b/atf-build.sh
index fb80ad0..c6ec3e3 100755
--- a/atf-build.sh
+++ b/atf-build.sh
@@ -25,9 +25,12 @@  function check_atf_buildver
 	MAJOR=`grep "^VERSION_MAJOR" Makefile | sed 's/.*:= *\([0-9]*\).*/\1/'`
 	[ $? -ne 0 ] && return 1
 	MINOR=`grep "^VERSION_MINOR" Makefile | sed 's/.*:= *\([0-9]*\).*/\1/'`
-	[ $? -ne 0 ] && return 1
 
-	if [ "$MAJOR" -eq 1 -a "$MINOR" -ge 2 ]; then
+	if [ "$MAJOR" -eq 1 ]; then
+		if [ "$MINOR" -ge 2 ]; then
+			ATF_BUILDVER=2
+		fi
+	else
 		ATF_BUILDVER=2
 	fi
 }