diff mbox

[edk2,RFC] edksetup.sh: detect updated BaseTools templates

Message ID 20160904154135.4324-1-leif.lindholm@linaro.org
State New
Headers show

Commit Message

Leif Lindholm Sept. 4, 2016, 3:41 p.m. UTC
Make edksetup.sh automatically update the cached configuration under
Conf/ when the templates under BaseTools/Conf/ change.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

---

We keep getting questions from people starting out with EDK2 development,
whenever certain options in the BaseTools template configs change and
their builds break. I don't know if this naive sledgehammer approach is
acceptable, but I'd like to make life easier for people.

 edksetup.sh | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

-- 
2.9.3

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Ard Biesheuvel Sept. 4, 2016, 3:55 p.m. UTC | #1
On 4 September 2016 at 16:41, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> Make edksetup.sh automatically update the cached configuration under

> Conf/ when the templates under BaseTools/Conf/ change.

>

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---

>

> We keep getting questions from people starting out with EDK2 development,

> whenever certain options in the BaseTools template configs change and

> their builds break. I don't know if this naive sledgehammer approach is

> acceptable, but I'd like to make life easier for people.

>


Could we instead make the files under Conf/ symlinks to the templates?
That way, people can still keep local changes in these files, which is
presumably the reason for this arrangement, while the 'naive' user
[with no interest in keeping local changes] does not have to deal with
this.


>  edksetup.sh | 33 +++++++++++++++++++++++++++++++--

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

>

> diff --git a/edksetup.sh b/edksetup.sh

> index 57368b5..77f0d43 100755

> --- a/edksetup.sh

> +++ b/edksetup.sh

> @@ -33,12 +33,41 @@ function HelpMsg()

>    return 1

>  }

>

> +function ClearCache()

> +{

> +  CONF_FILES="build_rule target tools_def"

> +  if [ -z "$EDK_TOOLS_PATH" ]

> +  then

> +    TEMPLATE_PATH=./BaseTools/Conf/

> +  else

> +    TEMPLATE_PATH="$EDK_TOOLS_PATH/Conf/"

> +  fi

> +

> +  DELETED_FILES=0

> +  for File in $CONF_FILES

> +  do

> +    TEMPLATE_FILE="$TEMPLATE_PATH/$File.template"

> +    CACHE_FILE="Conf/$File.txt"

> +    if [ "$TEMPLATE_FILE" -nt "$CACHE_FILE" ]

> +    then

> +      echo "Removing outdated '$CACHE_FILE'."

> +      rm "$CACHE_FILE"

> +      DELETED_FILES=$(($DELETED_FILES + 1))

> +    fi

> +  done

> +

> +  unset TEMPLATE_PATH TEMPLATE_FILE CACHE_FILE

> +  return $DELETED_FILES

> +}

> +

>  function SetWorkspace()

>  {

>    #

> -  # If WORKSPACE is already set, then we can return right now

> +  # Check for updated BaseTools templates. If none, and

> +  # WORKSPACE is already set, then we can return right now

>    #

> -  if [ -n "$WORKSPACE" ]

> +  ClearCache

> +  if [ $? -ne 0 -a -n "$WORKSPACE" ]

>    then

>      return 0

>    fi

> --

> 2.9.3

>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Sept. 4, 2016, 8:35 p.m. UTC | #2
On 4 September 2016 at 17:10, Michael Zimmermann
<sigmaepsilon92@gmail.com> wrote:
>> Could we instead make the files under Conf/ symlinks to the templates?

> Sounds like a good idea in general but what about windows? afaik only NTFS

> has support for them.

>


Ah yes, excellent point.

So perhaps the BaseTools should simply default to the template if the
Conf/ version is missing? I am concerned about breaking people's
workflow where they keep non-trivial local changes in these files.


> On Sun, Sep 4, 2016 at 5:55 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org>

> wrote:

>>

>> On 4 September 2016 at 16:41, Leif Lindholm <leif.lindholm@linaro.org>

>> wrote:

>> > Make edksetup.sh automatically update the cached configuration under

>> > Conf/ when the templates under BaseTools/Conf/ change.

>> >

>> > Contributed-under: TianoCore Contribution Agreement 1.0

>> > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

>> > ---

>> >

>> > We keep getting questions from people starting out with EDK2

>> > development,

>> > whenever certain options in the BaseTools template configs change and

>> > their builds break. I don't know if this naive sledgehammer approach is

>> > acceptable, but I'd like to make life easier for people.

>> >

>>

>> Could we instead make the files under Conf/ symlinks to the templates?

>> That way, people can still keep local changes in these files, which is

>> presumably the reason for this arrangement, while the 'naive' user

>> [with no interest in keeping local changes] does not have to deal with

>> this.

>>

>>

>> >  edksetup.sh | 33 +++++++++++++++++++++++++++++++--

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

>> >

>> > diff --git a/edksetup.sh b/edksetup.sh

>> > index 57368b5..77f0d43 100755

>> > --- a/edksetup.sh

>> > +++ b/edksetup.sh

>> > @@ -33,12 +33,41 @@ function HelpMsg()

>> >    return 1

>> >  }

>> >

>> > +function ClearCache()

>> > +{

>> > +  CONF_FILES="build_rule target tools_def"

>> > +  if [ -z "$EDK_TOOLS_PATH" ]

>> > +  then

>> > +    TEMPLATE_PATH=./BaseTools/Conf/

>> > +  else

>> > +    TEMPLATE_PATH="$EDK_TOOLS_PATH/Conf/"

>> > +  fi

>> > +

>> > +  DELETED_FILES=0

>> > +  for File in $CONF_FILES

>> > +  do

>> > +    TEMPLATE_FILE="$TEMPLATE_PATH/$File.template"

>> > +    CACHE_FILE="Conf/$File.txt"

>> > +    if [ "$TEMPLATE_FILE" -nt "$CACHE_FILE" ]

>> > +    then

>> > +      echo "Removing outdated '$CACHE_FILE'."

>> > +      rm "$CACHE_FILE"

>> > +      DELETED_FILES=$(($DELETED_FILES + 1))

>> > +    fi

>> > +  done

>> > +

>> > +  unset TEMPLATE_PATH TEMPLATE_FILE CACHE_FILE

>> > +  return $DELETED_FILES

>> > +}

>> > +

>> >  function SetWorkspace()

>> >  {

>> >    #

>> > -  # If WORKSPACE is already set, then we can return right now

>> > +  # Check for updated BaseTools templates. If none, and

>> > +  # WORKSPACE is already set, then we can return right now

>> >    #

>> > -  if [ -n "$WORKSPACE" ]

>> > +  ClearCache

>> > +  if [ $? -ne 0 -a -n "$WORKSPACE" ]

>> >    then

>> >      return 0

>> >    fi

>> > --

>> > 2.9.3

>> >

>> _______________________________________________

>> edk2-devel mailing list

>> edk2-devel@lists.01.org

>> https://lists.01.org/mailman/listinfo/edk2-devel

>

>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Oct. 19, 2016, 12:21 p.m. UTC | #3
So, this thread kind of petered out...

I had an internal report of a user failing to build ArmVirtQemu, and
narrowed it down to a case of outdated cached configurations.

As a temporary workaround, I've now put the proposed timestamp check
into my uefi-tools helper scripts, but that's hiding the problem
instead of solving it.

On Sun, Sep 04, 2016 at 09:35:19PM +0100, Ard Biesheuvel wrote:
> On 4 September 2016 at 17:10, Michael Zimmermann

> <sigmaepsilon92@gmail.com> wrote:

> >> Could we instead make the files under Conf/ symlinks to the templates?

> > Sounds like a good idea in general but what about windows? afaik only NTFS

> > has support for them.

> >

> 

> Ah yes, excellent point.

> 

> So perhaps the BaseTools should simply default to the template if the

> Conf/ version is missing? I am concerned about breaking people's

> workflow where they keep non-trivial local changes in these files.


This would absolutely be my preferred solution.

Andrew, Mike?

/
    Leif

> > On Sun, Sep 4, 2016 at 5:55 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org>

> > wrote:

> >>

> >> On 4 September 2016 at 16:41, Leif Lindholm <leif.lindholm@linaro.org>

> >> wrote:

> >> > Make edksetup.sh automatically update the cached configuration under

> >> > Conf/ when the templates under BaseTools/Conf/ change.

> >> >

> >> > Contributed-under: TianoCore Contribution Agreement 1.0

> >> > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

> >> > ---

> >> >

> >> > We keep getting questions from people starting out with EDK2

> >> > development,

> >> > whenever certain options in the BaseTools template configs change and

> >> > their builds break. I don't know if this naive sledgehammer approach is

> >> > acceptable, but I'd like to make life easier for people.

> >> >

> >>

> >> Could we instead make the files under Conf/ symlinks to the templates?

> >> That way, people can still keep local changes in these files, which is

> >> presumably the reason for this arrangement, while the 'naive' user

> >> [with no interest in keeping local changes] does not have to deal with

> >> this.

> >>

> >>

> >> >  edksetup.sh | 33 +++++++++++++++++++++++++++++++--

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

> >> >

> >> > diff --git a/edksetup.sh b/edksetup.sh

> >> > index 57368b5..77f0d43 100755

> >> > --- a/edksetup.sh

> >> > +++ b/edksetup.sh

> >> > @@ -33,12 +33,41 @@ function HelpMsg()

> >> >    return 1

> >> >  }

> >> >

> >> > +function ClearCache()

> >> > +{

> >> > +  CONF_FILES="build_rule target tools_def"

> >> > +  if [ -z "$EDK_TOOLS_PATH" ]

> >> > +  then

> >> > +    TEMPLATE_PATH=./BaseTools/Conf/

> >> > +  else

> >> > +    TEMPLATE_PATH="$EDK_TOOLS_PATH/Conf/"

> >> > +  fi

> >> > +

> >> > +  DELETED_FILES=0

> >> > +  for File in $CONF_FILES

> >> > +  do

> >> > +    TEMPLATE_FILE="$TEMPLATE_PATH/$File.template"

> >> > +    CACHE_FILE="Conf/$File.txt"

> >> > +    if [ "$TEMPLATE_FILE" -nt "$CACHE_FILE" ]

> >> > +    then

> >> > +      echo "Removing outdated '$CACHE_FILE'."

> >> > +      rm "$CACHE_FILE"

> >> > +      DELETED_FILES=$(($DELETED_FILES + 1))

> >> > +    fi

> >> > +  done

> >> > +

> >> > +  unset TEMPLATE_PATH TEMPLATE_FILE CACHE_FILE

> >> > +  return $DELETED_FILES

> >> > +}

> >> > +

> >> >  function SetWorkspace()

> >> >  {

> >> >    #

> >> > -  # If WORKSPACE is already set, then we can return right now

> >> > +  # Check for updated BaseTools templates. If none, and

> >> > +  # WORKSPACE is already set, then we can return right now

> >> >    #

> >> > -  if [ -n "$WORKSPACE" ]

> >> > +  ClearCache

> >> > +  if [ $? -ne 0 -a -n "$WORKSPACE" ]

> >> >    then

> >> >      return 0

> >> >    fi

> >> > --

> >> > 2.9.3

> >> >

> >> _______________________________________________

> >> edk2-devel mailing list

> >> edk2-devel@lists.01.org

> >> https://lists.01.org/mailman/listinfo/edk2-devel

> >

> >

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Gao, Liming Oct. 20, 2016, 6:58 a.m. UTC | #4
Leif:
  Windows edksetup.bat has option Reconfig to always copy template files to Conf directory. For Linux, I suggest to add Reconfig option to edksetup.sh, then let user use this way.

Thanks
Liming
From: Leif Lindholm [mailto:leif.lindholm@linaro.org]

Sent: Wednesday, October 19, 2016 8:21 PM
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Michael Zimmermann <sigmaepsilon92@gmail.com>; edk2-devel-01 <edk2-devel@lists.01.org>; Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [RFC] edksetup.sh: detect updated BaseTools templates

So, this thread kind of petered out...

I had an internal report of a user failing to build ArmVirtQemu, and
narrowed it down to a case of outdated cached configurations.

As a temporary workaround, I've now put the proposed timestamp check
into my uefi-tools helper scripts, but that's hiding the problem
instead of solving it.

On Sun, Sep 04, 2016 at 09:35:19PM +0100, Ard Biesheuvel wrote:
> On 4 September 2016 at 17:10, Michael Zimmermann

> wrote:

> >> Could we instead make the files under Conf/ symlinks to the templates?

> > Sounds like a good idea in general but what about windows? afaik only NTFS

> > has support for them.

> >

>

> Ah yes, excellent point.

>

> So perhaps the BaseTools should simply default to the template if the

> Conf/ version is missing? I am concerned about breaking people's

> workflow where they keep non-trivial local changes in these files.


This would absolutely be my preferred solution.

Andrew, Mike?

/
Leif

> > On Sun, Sep 4, 2016 at 5:55 PM, Ard Biesheuvel

> > wrote:

> >>

> >> On 4 September 2016 at 16:41, Leif Lindholm

> >> wrote:

> >> > Make edksetup.sh automatically update the cached configuration under

> >> > Conf/ when the templates under BaseTools/Conf/ change.

> >> >

> >> > Contributed-under: TianoCore Contribution Agreement 1.0

> >> > Signed-off-by: Leif Lindholm

> >> > ---

> >> >

> >> > We keep getting questions from people starting out with EDK2

> >> > development,

> >> > whenever certain options in the BaseTools template configs change and

> >> > their builds break. I don't know if this naive sledgehammer approach is

> >> > acceptable, but I'd like to make life easier for people.

> >> >

> >>

> >> Could we instead make the files under Conf/ symlinks to the templates?

> >> That way, people can still keep local changes in these files, which is

> >> presumably the reason for this arrangement, while the 'naive' user

> >> [with no interest in keeping local changes] does not have to deal with

> >> this.

> >>

> >>

> >> > edksetup.sh | 33 +++++++++++++++++++++++++++++++--

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

> >> >

> >> > diff --git a/edksetup.sh b/edksetup.sh

> >> > index 57368b5..77f0d43 100755

> >> > --- a/edksetup.sh

> >> > +++ b/edksetup.sh

> >> > @@ -33,12 +33,41 @@ function HelpMsg()

> >> > return 1

> >> > }

> >> >

> >> > +function ClearCache()

> >> > +{

> >> > + CONF_FILES="build_rule target tools_def"

> >> > + if [ -z "$EDK_TOOLS_PATH" ]

> >> > + then

> >> > + TEMPLATE_PATH=./BaseTools/Conf/

> >> > + else

> >> > + TEMPLATE_PATH="$EDK_TOOLS_PATH/Conf/"

> >> > + fi

> >> > +

> >> > + DELETED_FILES=0

> >> > + for File in $CONF_FILES

> >> > + do

> >> > + TEMPLATE_FILE="$TEMPLATE_PATH/$File.template"

> >> > + CACHE_FILE="Conf/$File.txt"

> >> > + if [ "$TEMPLATE_FILE" -nt "$CACHE_FILE" ]

> >> > + then

> >> > + echo "Removing outdated '$CACHE_FILE'."

> >> > + rm "$CACHE_FILE"

> >> > + DELETED_FILES=$(($DELETED_FILES + 1))

> >> > + fi

> >> > + done

> >> > +

> >> > + unset TEMPLATE_PATH TEMPLATE_FILE CACHE_FILE

> >> > + return $DELETED_FILES

> >> > +}

> >> > +

> >> > function SetWorkspace()

> >> > {

> >> > #

> >> > - # If WORKSPACE is already set, then we can return right now

> >> > + # Check for updated BaseTools templates. If none, and

> >> > + # WORKSPACE is already set, then we can return right now

> >> > #

> >> > - if [ -n "$WORKSPACE" ]

> >> > + ClearCache

> >> > + if [ $? -ne 0 -a -n "$WORKSPACE" ]

> >> > then

> >> > return 0

> >> > fi

> >> > --

> >> > 2.9.3

> >> >

> >> _______________________________________________

> >> edk2-devel mailing list

> >> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

> >> https://lists.01.org/mailman/listinfo/edk2-devel

> >

> >

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox

Patch

diff --git a/edksetup.sh b/edksetup.sh
index 57368b5..77f0d43 100755
--- a/edksetup.sh
+++ b/edksetup.sh
@@ -33,12 +33,41 @@  function HelpMsg()
   return 1
 }
 
+function ClearCache()
+{
+  CONF_FILES="build_rule target tools_def"
+  if [ -z "$EDK_TOOLS_PATH" ]
+  then
+    TEMPLATE_PATH=./BaseTools/Conf/
+  else
+    TEMPLATE_PATH="$EDK_TOOLS_PATH/Conf/"
+  fi
+
+  DELETED_FILES=0
+  for File in $CONF_FILES
+  do
+    TEMPLATE_FILE="$TEMPLATE_PATH/$File.template"
+    CACHE_FILE="Conf/$File.txt"
+    if [ "$TEMPLATE_FILE" -nt "$CACHE_FILE" ]
+    then
+      echo "Removing outdated '$CACHE_FILE'."
+      rm "$CACHE_FILE"
+      DELETED_FILES=$(($DELETED_FILES + 1))
+    fi
+  done
+
+  unset TEMPLATE_PATH TEMPLATE_FILE CACHE_FILE
+  return $DELETED_FILES
+}
+
 function SetWorkspace()
 {
   #
-  # If WORKSPACE is already set, then we can return right now
+  # Check for updated BaseTools templates. If none, and
+  # WORKSPACE is already set, then we can return right now
   #
-  if [ -n "$WORKSPACE" ]
+  ClearCache
+  if [ $? -ne 0 -a -n "$WORKSPACE" ]
   then
     return 0
   fi