| Message ID | 20220413093544.5587-1-abongwabonalais@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | "bitbake-prserv-tool: Added quotes to variables to prevent splitting and gobbling" | expand |
See comment below (our mail server injects signatures, sorry for that) Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Products B.V. Materiaalweg 4, 5681 RJ Best The Netherlands T: +31 (0) 499 33 69 69 E: mike.looijmans@topicproducts.com W: www.topic.nl Please consider the environment before printing this e-mail On 13-04-2022 11:35, Abongwa Amahnui Bonalais via lists.openembedded.org wrote: > Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com> > --- > scripts/bitbake-prserv-tool | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/scripts/bitbake-prserv-tool b/scripts/bitbake-prserv-tool > index e55d98c72e..68caa9fb66 100755 > --- a/scripts/bitbake-prserv-tool > +++ b/scripts/bitbake-prserv-tool > @@ -5,7 +5,7 @@ > > help () > { > - base=`basename $0` > + base=`basename "$0"` > echo -e "Usage: $base command" > echo "Avaliable commands:" > echo -e "\texport <file.conf>: export and lock down the AUTOPR values from the PR service into a file for release." > @@ -16,7 +16,7 @@ clean_cache() > { > s=`bitbake -e | grep ^CACHE= | cut -f2 -d\"` > if [ "x${s}" != "x" ]; then > - rm -rf ${s} > + rm -rf "${s}" > fi > } > > @@ -24,14 +24,14 @@ do_export () > { > file=$1 You'd want to quote this one too I think. > [ "x${file}" == "x" ] && help && exit 1 > - rm -f ${file} > + rm -f "${file}" > > clean_cache > bitbake -R conf/prexport.conf -p > s=`bitbake -R conf/prexport.conf -e | grep ^PRSERV_DUMPFILE= | cut -f2 -d\"` > if [ "x${s}" != "x" ]; > then > - [ -e $s ] && mv -f $s $file && echo "Exporting to file $file succeeded!" > + [ -e "$s" ] && mv -f "$s" "$file" && echo "Exporting to file $file succeeded!" > return 0 > fi > echo "Exporting to file $file failed!" > @@ -44,7 +44,7 @@ do_import () > [ "x${file}" == "x" ] && help && exit 1 > > clean_cache > - bitbake -R conf/primport.conf -R $file -p > + bitbake -R conf/primport.conf -R "$file" -p > ret=$? > [ $ret -eq 0 ] && echo "Importing from file $file succeeded!" || echo "Importing from file $file failed!" > return $ret > @@ -60,13 +60,13 @@ do_migrate_localcount () > return 1 > fi > > - rm -rf $df > + rm -rf "$df" > clean_cache > echo "Exporting LOCALCOUNT to AUTOINCs..." > bitbake -R conf/migrate_localcount.conf -p > [ ! $? -eq 0 ] && echo "Exporting to file $df failed!" && exit 1 > > - if [ -e $df ]; > + if [ -e "$df" ]; > then > echo "Exporting to file $df succeeded!" > else > @@ -75,7 +75,7 @@ do_migrate_localcount () > fi > > echo "Importing generated AUTOINC entries..." > - [ -e $df ] && do_import $df > + [ -e "$df" ] && do_import "$df" > > if [ ! $? -eq 0 ] > then > @@ -93,17 +93,17 @@ case $2 in > *.conf|*.inc) > ;; > *) > - echo ERROR: $2 must end with .conf or .inc! > + echo ERROR: "$2" must end with .conf or .inc! > exit 1 > ;; > esac > > case $1 in > export) > - do_export $2 > + do_export "$2" > ;; > import) > - do_import $2 > + do_import "$2" > ;; > migrate_localcount) > do_migrate_localcount > > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#164310): https://lists.openembedded.org/g/openembedded-core/message/164310 > Mute This Topic: https://lists.openembedded.org/mt/90437025/3618446 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [mike.looijmans@topic.nl] > -=-=-=-=-=-=-=-=-=-=-=- >
> -----Original Message----- > From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Mike Looijmans > Sent: den 13 april 2022 11:52 > To: openembedded-core@lists.openembedded.org > Subject: Re: [OE-core] [PATCH] "bitbake-prserv-tool: Added quotes to variables to prevent splitting and gobbling" There shouldn't be quotes in the subject above. No idea how you managed to get them there... Also, "gobbling" is not a word, AFAIK. Did you mean "globbing"? > > See comment below (our mail server injects signatures, sorry for that) > > > Met vriendelijke groet / kind regards, > > Mike Looijmans > System Expert > > > TOPIC Embedded Products B.V. > Materiaalweg 4, 5681 RJ Best > The Netherlands > > T: +31 (0) 499 33 69 69 > E: mike.looijmans@topicproducts.com > W: www.topic.nl > > Please consider the environment before printing this e-mail > On 13-04-2022 11:35, Abongwa Amahnui Bonalais via lists.openembedded.org wrote: > > Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com> > > --- > > scripts/bitbake-prserv-tool | 22 +++++++++++----------- > > 1 file changed, 11 insertions(+), 11 deletions(-) > > > > diff --git a/scripts/bitbake-prserv-tool b/scripts/bitbake-prserv-tool > > index e55d98c72e..68caa9fb66 100755 > > --- a/scripts/bitbake-prserv-tool > > +++ b/scripts/bitbake-prserv-tool > > @@ -5,7 +5,7 @@ > > > > help () > > { > > - base=`basename $0` > > + base=`basename "$0"` > > echo -e "Usage: $base command" > > echo "Avaliable commands:" > > echo -e "\texport <file.conf>: export and lock down the AUTOPR values from the PR service into a file for release." > > @@ -16,7 +16,7 @@ clean_cache() > > { > > s=`bitbake -e | grep ^CACHE= | cut -f2 -d\"` > > if [ "x${s}" != "x" ]; then > > - rm -rf ${s} > > + rm -rf "${s}" > > fi > > } > > > > @@ -24,14 +24,14 @@ do_export () > > { > > file=$1 > > You'd want to quote this one too I think. Actually, as inconsistent as it may seem, quotes are _not_ needed above. > > > [ "x${file}" == "x" ] && help && exit 1 > > - rm -f ${file} > > + rm -f "${file}" > > > > clean_cache > > bitbake -R conf/prexport.conf -p > > s=`bitbake -R conf/prexport.conf -e | grep ^PRSERV_DUMPFILE= | cut -f2 -d\"` > > if [ "x${s}" != "x" ]; > > then > > - [ -e $s ] && mv -f $s $file && echo "Exporting to file $file succeeded!" > > + [ -e "$s" ] && mv -f "$s" "$file" && echo "Exporting to file $file succeeded!" > > return 0 > > fi > > echo "Exporting to file $file failed!" > > @@ -44,7 +44,7 @@ do_import () > > [ "x${file}" == "x" ] && help && exit 1 > > > > clean_cache > > - bitbake -R conf/primport.conf -R $file -p > > + bitbake -R conf/primport.conf -R "$file" -p > > ret=$? > > [ $ret -eq 0 ] && echo "Importing from file $file succeeded!" || echo "Importing from file $file failed!" > > return $ret > > @@ -60,13 +60,13 @@ do_migrate_localcount () > > return 1 > > fi > > > > - rm -rf $df > > + rm -rf "$df" > > clean_cache > > echo "Exporting LOCALCOUNT to AUTOINCs..." > > bitbake -R conf/migrate_localcount.conf -p > > [ ! $? -eq 0 ] && echo "Exporting to file $df failed!" && exit 1 > > > > - if [ -e $df ]; > > + if [ -e "$df" ]; > > then > > echo "Exporting to file $df succeeded!" > > else > > @@ -75,7 +75,7 @@ do_migrate_localcount () > > fi > > > > echo "Importing generated AUTOINC entries..." > > - [ -e $df ] && do_import $df > > + [ -e "$df" ] && do_import "$df" > > > > if [ ! $? -eq 0 ] > > then > > @@ -93,17 +93,17 @@ case $2 in > > *.conf|*.inc) > > ;; > > *) > > - echo ERROR: $2 must end with .conf or .inc! > > + echo ERROR: "$2" must end with .conf or .inc! The quote above is strictly not needed as the output from echo will be the same regardless if there are spaces in $2 or not. However, if you really want to quote that line, a more natural way would be: echo "ERROR: $2 must end with .conf or .inc!" > > exit 1 > > ;; > > esac > > > > case $1 in > > export) > > - do_export $2 > > + do_export "$2" > > ;; > > import) > > - do_import $2 > > + do_import "$2" > > ;; > > migrate_localcount) > > do_migrate_localcount //Peter
On Wed, 2022-04-13 at 10:35 +0100, Abongwa Amahnui Bonalais wrote: > Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com> > --- > scripts/bitbake-prserv-tool | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) I want to suggest a little bit of caution with the direction these changes are going. Showing the user a clear error if that have spaces in a build path is a good thing. Making oe-init-build-env handle that correctly is therefore desirable so we can get the proper error from bitbake. This is likely the entry point most users would find (toaster is another) Since we don't support spaces in build paths and we're unlikely to do so given that autotools can't, I'm not sure we want to go through too many of our scripts doing this kind of change though. Most users would use the prserv tool as an initial introduction to the project. We did already see potential problems being introduced in the toaster changes on the bitbake list and I do worry about potential regressions. Cheers, Richard
Okay thank you what about Using $(...) notation instead of legacy backticked `...`. base=`basename $0`, changing it to base=$(basename $0) Should I commence those changes? I'm seeing that based om recommendations when I run shellcheck <name of file>
On Wed, 2022-04-13 at 03:50 -0700, Abongwa Amahnui Bonalais wrote: > Okay thank you what about Using $(...) notation instead of legacy backticked > `...`. > base=`basename $0`, changing it to base=$(basename $0) > > Should I commence those changes? > I'm seeing that based om recommendations when I run shellcheck <name of file> It depends on how you're testing the end result. I don't mind improving the scripts but what we don't really want are patches made by just doing what that tool says without the result being tested. How are you testing the results? Cheers, Richard
On Wed, 2022-04-13 at 05:52 -0700, Abongwa Amahnui Bonalais wrote: > I test the results by running bitbake core-image-sato. > If it builds successsfully, then I assume that it is fine, > Please what method do you recommend I use to test the results. You need to find a test which tests the component you're changing. "bitbake core-image-sato" would test oe-init-build-env but it does not test toaster or bitbake-prserv-tool since that are not involved with that command. An easy way to check is to actively break the script, put something in there you know will break. If your test doesn't show it not working, it must not be testing it. Cheers, Richard
diff --git a/scripts/bitbake-prserv-tool b/scripts/bitbake-prserv-tool index e55d98c72e..68caa9fb66 100755 --- a/scripts/bitbake-prserv-tool +++ b/scripts/bitbake-prserv-tool @@ -5,7 +5,7 @@ help () { - base=`basename $0` + base=`basename "$0"` echo -e "Usage: $base command" echo "Avaliable commands:" echo -e "\texport <file.conf>: export and lock down the AUTOPR values from the PR service into a file for release." @@ -16,7 +16,7 @@ clean_cache() { s=`bitbake -e | grep ^CACHE= | cut -f2 -d\"` if [ "x${s}" != "x" ]; then - rm -rf ${s} + rm -rf "${s}" fi } @@ -24,14 +24,14 @@ do_export () { file=$1 [ "x${file}" == "x" ] && help && exit 1 - rm -f ${file} + rm -f "${file}" clean_cache bitbake -R conf/prexport.conf -p s=`bitbake -R conf/prexport.conf -e | grep ^PRSERV_DUMPFILE= | cut -f2 -d\"` if [ "x${s}" != "x" ]; then - [ -e $s ] && mv -f $s $file && echo "Exporting to file $file succeeded!" + [ -e "$s" ] && mv -f "$s" "$file" && echo "Exporting to file $file succeeded!" return 0 fi echo "Exporting to file $file failed!" @@ -44,7 +44,7 @@ do_import () [ "x${file}" == "x" ] && help && exit 1 clean_cache - bitbake -R conf/primport.conf -R $file -p + bitbake -R conf/primport.conf -R "$file" -p ret=$? [ $ret -eq 0 ] && echo "Importing from file $file succeeded!" || echo "Importing from file $file failed!" return $ret @@ -60,13 +60,13 @@ do_migrate_localcount () return 1 fi - rm -rf $df + rm -rf "$df" clean_cache echo "Exporting LOCALCOUNT to AUTOINCs..." bitbake -R conf/migrate_localcount.conf -p [ ! $? -eq 0 ] && echo "Exporting to file $df failed!" && exit 1 - if [ -e $df ]; + if [ -e "$df" ]; then echo "Exporting to file $df succeeded!" else @@ -75,7 +75,7 @@ do_migrate_localcount () fi echo "Importing generated AUTOINC entries..." - [ -e $df ] && do_import $df + [ -e "$df" ] && do_import "$df" if [ ! $? -eq 0 ] then @@ -93,17 +93,17 @@ case $2 in *.conf|*.inc) ;; *) - echo ERROR: $2 must end with .conf or .inc! + echo ERROR: "$2" must end with .conf or .inc! exit 1 ;; esac case $1 in export) - do_export $2 + do_export "$2" ;; import) - do_import $2 + do_import "$2" ;; migrate_localcount) do_migrate_localcount
Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com> --- scripts/bitbake-prserv-tool | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)