o Use '$SCRIPTNAME' consistently instead of 'edksetup.sh'
o Use the bash environment variable $PWD instead of executing the pwd
command.
o Add quotes around variables to ensure they're evaluated correctly.
o Simplify SetupPython3() and SetupPython() functions. On Linux,
"whereis" matches python3, python3.7, as well as man pages, libs etc.
While on macOS it only matches the specified name, and so misses
python3.7. Improve this by looping over potential version numbers and
seeing if such a binary exists and can be executed.
o Since when parsing options '*' matches anything not already matched,
remove -?, -h and --help since they're redundant.
o When executing arithmetic commands, $ isn't needed before variables.
Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
edksetup.sh | 67 ++++++++++++++---------------------------------------
1 file changed, 18 insertions(+), 49 deletions(-)
diff --git a/edksetup.sh b/edksetup.sh
index 12a3e26a67..a797ff03d2 100755
--- a/edksetup.sh
+++ b/edksetup.sh
@@ -49,11 +49,11 @@ function SetWorkspace()
return 0
fi
- if [ ! ${BASH_SOURCE[0]} -ef ./edksetup.sh ] && [ -z "$PACKAGES_PATH" ]
+ if [ ! ${BASH_SOURCE[0]} -ef ./$SCRIPTNAME ] && [ -z "$PACKAGES_PATH" ]
then
echo Run this script from the base of your tree. For example:
- echo " cd /Path/To/Edk/Root"
- echo " . edksetup.sh"
+ echo " cd /path/to/edk2/root"
+ echo " . $SCRIPTNAME"
return 1
fi
@@ -71,7 +71,7 @@ function SetWorkspace()
#
# Set $WORKSPACE
#
- export WORKSPACE=`pwd`
+ export WORKSPACE=$PWD
return 0
}
@@ -107,24 +107,10 @@ function SetupEnv()
function SetupPython3()
{
- if [ $origin_version ];then
- origin_version=
- fi
- for python in $(whereis python3)
- do
- python=$(echo $python | grep "[[:digit:]]$" || true)
- python_version=${python##*python}
- if [ -z "${python_version}" ] || (! command -v $python >/dev/null 2>&1);then
- continue
- fi
- if [ -z $origin_version ];then
- origin_version=$python_version
- export PYTHON_COMMAND=$python
- continue
- fi
- if [[ "$origin_version" < "$python_version" ]]; then
- origin_version=$python_version
- export PYTHON_COMMAND=$python
+ for python in $(seq -f "python3.%g" 15 -1 1) python3; do
+ if command -v $python >/dev/null 2>&1; then
+ export PYTHON_COMMAND=$(which $python)
+ break
fi
done
return 0
@@ -132,8 +118,8 @@ function SetupPython3()
function SetupPython()
{
- if [ $PYTHON_COMMAND ] && [ -z $PYTHON3_ENABLE ];then
- if ( command -v $PYTHON_COMMAND >/dev/null 2>&1 );then
+ if [ -n "$PYTHON_COMMAND" ] && [ -z "$PYTHON3_ENABLE" ]; then
+ if command -v $PYTHON_COMMAND >/dev/null 2>&1; then
return 0
else
echo $PYTHON_COMMAND Cannot be used to build or execute the python tools.
@@ -141,32 +127,15 @@ function SetupPython()
fi
fi
- if [ $PYTHON3_ENABLE ] && [ $PYTHON3_ENABLE == TRUE ]
- then
+ if [ "$PYTHON3_ENABLE" == "TRUE" ]; then
SetupPython3
fi
- if [ $PYTHON3_ENABLE ] && [ $PYTHON3_ENABLE != TRUE ]
- then
- if [ $origin_version ];then
- origin_version=
- fi
- for python in $(whereis python2)
- do
- python=$(echo $python | grep "[[:digit:]]$" || true)
- python_version=${python##*python}
- if [ -z "${python_version}" ] || (! command -v $python >/dev/null 2>&1);then
- continue
- fi
- if [ -z $origin_version ]
- then
- origin_version=$python_version
- export PYTHON_COMMAND=$python
- continue
- fi
- if [[ "$origin_version" < "$python_version" ]]; then
- origin_version=$python_version
- export PYTHON_COMMAND=$python
+ if [ -n "$PYTHON3_ENABLE" ] && [ "$PYTHON3_ENABLE" != "TRUE" ]; then
+ for python in $(seq -f "python2.%g" 10 -1 1) python2; do
+ if command -v $python >/dev/null 2>&1; then
+ export PYTHON_COMMAND=$(which $python)
+ break
fi
done
return 0
@@ -194,12 +163,12 @@ do
RECONFIG=TRUE
shift
;;
- -?|-h|--help|*)
+ *)
HelpMsg
break
;;
esac
- I=$(($I - 1))
+ I=$((I - 1))
done
if [ $I -gt 0 ]
--
2.22.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#43528): https://edk2.groups.io/g/devel/message/43528
Mute This Topic: https://groups.io/mt/32421563/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi,
On 07/10/19 23:17, Rebecca Cran wrote:
> o Use '$SCRIPTNAME' consistently instead of 'edksetup.sh'
>
> o Use the bash environment variable $PWD instead of executing the pwd
> command.
>
> o Add quotes around variables to ensure they're evaluated correctly.
>
> o Simplify SetupPython3() and SetupPython() functions. On Linux,
> "whereis" matches python3, python3.7, as well as man pages, libs etc.
> While on macOS it only matches the specified name, and so misses
> python3.7. Improve this by looping over potential version numbers and
> seeing if such a binary exists and can be executed.
>
> o Since when parsing options '*' matches anything not already matched,
> remove -?, -h and --help since they're redundant.
>
> o When executing arithmetic commands, $ isn't needed before variables.
As long as my opinion counts... (and I totally don't insist that it do):
the above task list will make for a nice 6-part patch series. :)
(When someone is tempted to capture a *list* of changes in a single
commit message, that frequently indicates that the patch should be split
up, so that each change get its own dedicated patch.)
Thanks,
Laszlo
>
> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> ---
> edksetup.sh | 67 ++++++++++++++---------------------------------------
> 1 file changed, 18 insertions(+), 49 deletions(-)
>
> diff --git a/edksetup.sh b/edksetup.sh
> index 12a3e26a67..a797ff03d2 100755
> --- a/edksetup.sh
> +++ b/edksetup.sh
> @@ -49,11 +49,11 @@ function SetWorkspace()
> return 0
> fi
>
> - if [ ! ${BASH_SOURCE[0]} -ef ./edksetup.sh ] && [ -z "$PACKAGES_PATH" ]
> + if [ ! ${BASH_SOURCE[0]} -ef ./$SCRIPTNAME ] && [ -z "$PACKAGES_PATH" ]
> then
> echo Run this script from the base of your tree. For example:
> - echo " cd /Path/To/Edk/Root"
> - echo " . edksetup.sh"
> + echo " cd /path/to/edk2/root"
> + echo " . $SCRIPTNAME"
> return 1
> fi
>
> @@ -71,7 +71,7 @@ function SetWorkspace()
> #
> # Set $WORKSPACE
> #
> - export WORKSPACE=`pwd`
> + export WORKSPACE=$PWD
> return 0
> }
>
> @@ -107,24 +107,10 @@ function SetupEnv()
>
> function SetupPython3()
> {
> - if [ $origin_version ];then
> - origin_version=
> - fi
> - for python in $(whereis python3)
> - do
> - python=$(echo $python | grep "[[:digit:]]$" || true)
> - python_version=${python##*python}
> - if [ -z "${python_version}" ] || (! command -v $python >/dev/null 2>&1);then
> - continue
> - fi
> - if [ -z $origin_version ];then
> - origin_version=$python_version
> - export PYTHON_COMMAND=$python
> - continue
> - fi
> - if [[ "$origin_version" < "$python_version" ]]; then
> - origin_version=$python_version
> - export PYTHON_COMMAND=$python
> + for python in $(seq -f "python3.%g" 15 -1 1) python3; do
> + if command -v $python >/dev/null 2>&1; then
> + export PYTHON_COMMAND=$(which $python)
> + break
> fi
> done
> return 0
> @@ -132,8 +118,8 @@ function SetupPython3()
>
> function SetupPython()
> {
> - if [ $PYTHON_COMMAND ] && [ -z $PYTHON3_ENABLE ];then
> - if ( command -v $PYTHON_COMMAND >/dev/null 2>&1 );then
> + if [ -n "$PYTHON_COMMAND" ] && [ -z "$PYTHON3_ENABLE" ]; then
> + if command -v $PYTHON_COMMAND >/dev/null 2>&1; then
> return 0
> else
> echo $PYTHON_COMMAND Cannot be used to build or execute the python tools.
> @@ -141,32 +127,15 @@ function SetupPython()
> fi
> fi
>
> - if [ $PYTHON3_ENABLE ] && [ $PYTHON3_ENABLE == TRUE ]
> - then
> + if [ "$PYTHON3_ENABLE" == "TRUE" ]; then
> SetupPython3
> fi
>
> - if [ $PYTHON3_ENABLE ] && [ $PYTHON3_ENABLE != TRUE ]
> - then
> - if [ $origin_version ];then
> - origin_version=
> - fi
> - for python in $(whereis python2)
> - do
> - python=$(echo $python | grep "[[:digit:]]$" || true)
> - python_version=${python##*python}
> - if [ -z "${python_version}" ] || (! command -v $python >/dev/null 2>&1);then
> - continue
> - fi
> - if [ -z $origin_version ]
> - then
> - origin_version=$python_version
> - export PYTHON_COMMAND=$python
> - continue
> - fi
> - if [[ "$origin_version" < "$python_version" ]]; then
> - origin_version=$python_version
> - export PYTHON_COMMAND=$python
> + if [ -n "$PYTHON3_ENABLE" ] && [ "$PYTHON3_ENABLE" != "TRUE" ]; then
> + for python in $(seq -f "python2.%g" 10 -1 1) python2; do
> + if command -v $python >/dev/null 2>&1; then
> + export PYTHON_COMMAND=$(which $python)
> + break
> fi
> done
> return 0
> @@ -194,12 +163,12 @@ do
> RECONFIG=TRUE
> shift
> ;;
> - -?|-h|--help|*)
> + *)
> HelpMsg
> break
> ;;
> esac
> - I=$(($I - 1))
> + I=$((I - 1))
> done
>
> if [ $I -gt 0 ]
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#43675): https://edk2.groups.io/g/devel/message/43675
Mute This Topic: https://groups.io/mt/32421563/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 2019-07-12 16:21, Laszlo Ersek wrote: > > As long as my opinion counts... (and I totally don't insist that it do): > the above task list will make for a nice 6-part patch series. :) > > (When someone is tempted to capture a *list* of changes in a single > commit message, that frequently indicates that the patch should be split > up, so that each change get its own dedicated patch.) While I can see your point, in this case I think splitting the patch up into 6 parts would be excessive. All the changes are to a single file, and a couple of the changes in the list are to single lines. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43723): https://edk2.groups.io/g/devel/message/43723 Mute This Topic: https://groups.io/mt/32421563/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Mon, Jul 15, 2019 at 08:40:21AM -0600, Rebecca Cran wrote:
> On 2019-07-12 16:21, Laszlo Ersek wrote:
> >
> > As long as my opinion counts... (and I totally don't insist that it do):
> > the above task list will make for a nice 6-part patch series. :)
> >
> > (When someone is tempted to capture a *list* of changes in a single
> > commit message, that frequently indicates that the patch should be split
> > up, so that each change get its own dedicated patch.)
>
> While I can see your point, in this case I think splitting the patch up
> into 6 parts would be excessive. All the changes are to a single file,
> and a couple of the changes in the list are to single lines.
For me, the question is more with being able to trivially discern
which patch does what. I agree they're all individually trivial, but
as a single patch there is enough going on at once that it makes it
easier for bugs to slip through review. (And we've had issues with
this in the past in edksetup.sh.)
/
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#43725): https://edk2.groups.io/g/devel/message/43725
Mute This Topic: https://groups.io/mt/32421563/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 2019-07-15 08:55, Leif Lindholm wrote: > > For me, the question is more with being able to trivially discern > which patch does what. I agree they're all individually trivial, but > as a single patch there is enough going on at once that it makes it > easier for bugs to slip through review. (And we've had issues with > this in the past in edksetup.sh.) Thanks, I understand now. I'll resubmit the changes as a series. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43726): https://edk2.groups.io/g/devel/message/43726 Mute This Topic: https://groups.io/mt/32421563/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 07/15/19 17:01, Rebecca Cran wrote: > On 2019-07-15 08:55, Leif Lindholm wrote: >> >> For me, the question is more with being able to trivially discern >> which patch does what. I agree they're all individually trivial, but >> as a single patch there is enough going on at once that it makes it >> easier for bugs to slip through review. (And we've had issues with >> this in the past in edksetup.sh.) > > > > Thanks, I understand now. I'll resubmit the changes as a series. I'm sorry, I shouldn't have followed up, but read Leif's response first -- Leif expressed the same concern that I had, only better, and more politely. :) My apologies for the snark! Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43737): https://edk2.groups.io/g/devel/message/43737 Mute This Topic: https://groups.io/mt/32421563/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 07/15/19 16:40, Rebecca Cran wrote: > On 2019-07-12 16:21, Laszlo Ersek wrote: >> >> As long as my opinion counts... (and I totally don't insist that it do): >> the above task list will make for a nice 6-part patch series. :) >> >> (When someone is tempted to capture a *list* of changes in a single >> commit message, that frequently indicates that the patch should be split >> up, so that each change get its own dedicated patch.) > > > While I can see your point, in this case I think splitting the patch up > into 6 parts would be excessive. All the changes are to a single file, > and a couple of the changes in the list are to single lines. Fair enough, as long as you don't insist on my Reviewed-by in particular. :) I won't block the patch just because of this, but I also won't try to decipher changes made for six different goals from each other. Thanks, Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43736): https://edk2.groups.io/g/devel/message/43736 Mute This Topic: https://groups.io/mt/32421563/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 2019-07-15 11:45, Laszlo Ersek wrote: > > Fair enough, as long as you don't insist on my Reviewed-by in > particular. :) I won't block the patch just because of this, but I also > won't try to decipher changes made for six different goals from each other. Not six different goals: *one* goal, of simplifying the script :) Anyway, I've resubmitted the changes as a series. Now back to my day job. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43749): https://edk2.groups.io/g/devel/message/43749 Mute This Topic: https://groups.io/mt/32421563/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 2019-07-10 15:17, Rebecca Cran wrote: > o Use '$SCRIPTNAME' consistently instead of 'edksetup.sh' > > o Use the bash environment variable $PWD instead of executing the pwd > command. > > o Add quotes around variables to ensure they're evaluated correctly. > > o Simplify SetupPython3() and SetupPython() functions. On Linux, > "whereis" matches python3, python3.7, as well as man pages, libs etc. > While on macOS it only matches the specified name, and so misses > python3.7. Improve this by looping over potential version numbers and > seeing if such a binary exists and can be executed. > > o Since when parsing options '*' matches anything not already matched, > remove -?, -h and --help since they're redundant. > > o When executing arithmetic commands, $ isn't needed before variables. > > Signed-off-by: Rebecca Cran <rebecca@bsdio.com> Ping? Added the TianoCore Stewards to cc list, since the file is in the root of the repo and not a package. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43660): https://edk2.groups.io/g/devel/message/43660 Mute This Topic: https://groups.io/mt/32421563/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.