Change in coreboot[master]: util/mb/google/hatch: add optional bug parameter, fix style issues
Paul Fagerburg has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: add optional bug parameter, fix style issues ......................................................................
util/mb/google/hatch: add optional bug parameter, fix style issues
Add optional bug number parameter Use all caps for variables Use a single exit code
Change-Id: I63aa0aa633f36b9543e809fc42fac955da5960a3 --- M util/mainboard/google/hatch/create_coreboot_variant.sh 1 file changed, 27 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/35605/1
diff --git a/util/mainboard/google/hatch/create_coreboot_variant.sh b/util/mainboard/google/hatch/create_coreboot_variant.sh index d4256a6..065fb6d 100755 --- a/util/mainboard/google/hatch/create_coreboot_variant.sh +++ b/util/mainboard/google/hatch/create_coreboot_variant.sh @@ -13,9 +13,9 @@ # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU General Public License for more details.
-if [[ "$#" -ne 1 ]]; then - echo "Usage: $0 variant_name" - echo "e.g. $0 kohaku" +if [[ "$#" -lt 1 ]]; then + echo "Usage: $0 variant_name [bug_number]" + echo "e.g. $0 kohaku b:140261109" echo "Adds a new variant of Hatch to Kconfig and Kconfig.name, creates the" echo "skeleton files for acpi, ec, and gpio, copies the makefile for" echo "SPD sources, and sets up a basic overridetree" @@ -26,54 +26,55 @@ # you to specify the baseboard as one of the cmdline arguments. # # This is the name of the base board that we're cloning to make the variant. -base="hatch" -# This is the name of the variant that is being cloned -# ${var,,} converts to all lowercase -variant="${1,,}" +BASE="hatch" +# This is the name of the variant that is being cloned. +# ${var,,} converts to all lowercase; ${var^^} is all uppercase. +VARIANT="${1,,}" +VARIANT_UPPER="${VARIANT^^}" + +# Assign BUG= text, or "None" if that parameter wasn't specified. +BUG=${2:-None}
# This script and the templates live in util/mainboard/google/hatch # We need to create files in src/mainboard/google/hatch -pushd "${BASH_SOURCE%/*}" || exit +pushd "${BASH_SOURCE%/*}" || exit 1 SRC=$(pwd) -popd || exit +popd || exit 1 pushd "${SRC}/../../../../src/mainboard/google/${base}" || { echo "The baseboard directory for ${base} does not exist."; - exit; } + exit 1; }
-# Make sure the variant doesn't already exist -if [[ -e variants/${variant} ]]; then - echo "variants/${variant} already exists." +# Make sure the variant doesn't already exist. +if [[ -e variants/${VARIANT} ]]; then + echo "variants/${VARIANT} already exists." echo "Have you already created this variant?" - popd || exit - exit 2 + exit 1 fi
# Start a branch. Use YMD timestamp to avoid collisions. DATE=$(date +%Y%m%d) -git checkout -b "create_${variant}_${DATE}" +git checkout -b "create_${VARIANT}_${DATE}" || exit 1
-# Copy the template tree to the target +# Copy the template tree to the target. mkdir -p "variants/${variant}/" cp -pr "${SRC}/template/." "variants/${variant}/" -git add "variants/${variant}/" +git add "variants/${VARIANT}/"
# Now add the new variant to Kconfig and Kconfig.name # These files are in the current directory, e.g. src/mainboard/google/hatch -"${SRC}/kconfig.py" --name "${variant}" +"${SRC}/kconfig.py" --name "${VARIANT}"
mv Kconfig.new Kconfig mv Kconfig.name.new Kconfig.name
git add Kconfig Kconfig.name
-# Now commit the files -git commit -sm "${base}: Create ${variant} variant +# Now commit the files. +git commit -sm "${BASE}: Create ${VARIANT} variant
-BUG=none -TEST=util/abuild/abuild -p none -t google/${base} -x -a -make sure the build includes GOOGLE_${variant^^}" - -popd || exit +BUG=${BUG} +TEST=util/abuild/abuild -p none -t google/${BASE} -x -a +make sure the build includes GOOGLE_${VARIANT_UPPER}"
echo "Please check all the files (git show), make any changes you want," echo "and then push to coreboot HEAD:refs/for/master"
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: add optional bug parameter, fix style issues ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35605/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35605/1//COMMIT_MSG@7 PS1, Line 7: util/mb/google/hatch: add optional bug parameter, fix style issues Could you split the commit?
https://review.coreboot.org/c/coreboot/+/35605/1//COMMIT_MSG@7 PS1, Line 7: util It’s not clear, that it’s about the script.
https://review.coreboot.org/c/coreboot/+/35605/1//COMMIT_MSG@14 PS1, Line 14: Your Signed-off-by line is missing.
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: add optional bug parameter, fix style issues ......................................................................
Patch Set 1:
(3 comments)
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35605/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35605/1//COMMIT_MSG@7 PS1, Line 7: util/mb/google/hatch: add optional bug parameter, fix style issues
Could you split the commit?
OK.
https://review.coreboot.org/c/coreboot/+/35605/1//COMMIT_MSG@7 PS1, Line 7: util
It’s not clear, that it’s about the script.
Will clarify.
https://review.coreboot.org/c/coreboot/+/35605/1//COMMIT_MSG@14 PS1, Line 14:
Your Signed-off-by line is missing.
Oops! I'll fix it.
Hello Andrew McRae, Edward O'Callaghan, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35605
to look at the new patch set (#2).
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
util/mb/google/hatch: fix style issues in shell script
Use all caps for variables. Use a single exit code for failures. No need to popd before exiting the script. Do ${var,,} and ${var^^} into variables instead of using it everywhere. Add more punctuation in comments.
Signed-off-by: Paul Fagerburg pfagerburg@chromium.org Change-Id: I63aa0aa633f36b9543e809fc42fac955da5960a3 --- M util/mainboard/google/hatch/create_coreboot_variant.sh 1 file changed, 20 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/35605/2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/35605/1/util/mainboard/google/hatch... File util/mainboard/google/hatch/create_coreboot_variant.sh:
https://review.coreboot.org/c/coreboot/+/35605/1/util/mainboard/google/hatch... PS1, Line 31: uppercase For a follow up commit, but: I suppose these transformations are locale aware? If so, we should default to LC_ALL=C: For example I think for LC_ALL=tr, uppercase(i) isn't I but İ
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35605/1/util/mainboard/google/hatch... File util/mainboard/google/hatch/create_coreboot_variant.sh:
https://review.coreboot.org/c/coreboot/+/35605/1/util/mainboard/google/hatch... PS1, Line 31: uppercase
For a follow up commit, but: I suppose these transformations are locale aware? If so, we should defa […]
I had not even considered that. From what I can find, bash 4 is locale-aware when doing case transformations. That said, I expect that we'll continue to use 7-bit ASCII names for variants.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35605/1/util/mainboard/google/hatch... File util/mainboard/google/hatch/create_coreboot_variant.sh:
https://review.coreboot.org/c/coreboot/+/35605/1/util/mainboard/google/hatch... PS1, Line 31: uppercase
I had not even considered that. […]
Note that I and i are ASCII while when in a Turkish locale the conversion into the other case is not (in both directions!). Hence the suggestion to use the C locale to eliminate any such issues.
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35605/1/util/mainboard/google/hatch... File util/mainboard/google/hatch/create_coreboot_variant.sh:
https://review.coreboot.org/c/coreboot/+/35605/1/util/mainboard/google/hatch... PS1, Line 31: uppercase
Note that I and i are ASCII while when in a Turkish locale the conversion into the other case is not […]
Oh, yeah, Turkish "i" with a dot and without a dot, in both lower- and upper-case forms. Now I remember dealing with that several years ago when doing localization on an embedded GUI. Good times.
Can I be stubborn and say that we'll only support Romanized names for board variants? I mean, we write "kohaku" instead using native characters ...
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35605/1/util/mainboard/google/hatch... File util/mainboard/google/hatch/create_coreboot_variant.sh:
https://review.coreboot.org/c/coreboot/+/35605/1/util/mainboard/google/hatch... PS1, Line 31: uppercase
Oh, yeah, Turkish "i" with a dot and without a dot, in both lower- and upper-case forms. […]
You're right, we should stay with romanized names, but: somebody with turkish locale running this script (extended to work beyond hatch) for, say, fizz will get FİZZ, not FIZZ
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35605/1/util/mainboard/google/hatch... File util/mainboard/google/hatch/create_coreboot_variant.sh:
https://review.coreboot.org/c/coreboot/+/35605/1/util/mainboard/google/hatch... PS1, Line 31: uppercase
You're right, we should stay with romanized names, but: somebody with turkish locale running this sc […]
OK, *now* I get it. Thank you for your patience in explaining it. I'll add LC_ALL=C in the next commit, when I'm also adding more features.
Andrew McRae has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35605/2/util/mainboard/google/hatch... File util/mainboard/google/hatch/create_coreboot_variant.sh:
https://review.coreboot.org/c/coreboot/+/35605/2/util/mainboard/google/hatch... PS2, Line 56: variant ${variant} -> ${VARIANT}
https://review.coreboot.org/c/coreboot/+/35605/2/util/mainboard/google/hatch... PS2, Line 57: cp -pr "${SRC}/template/." "variants/${variant}/" ${variant} -> ${VARIANT}
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35605/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35605/3//COMMIT_MSG@9 PS3, Line 9: Use all caps for variables. : Use a single exit code for failures. : No need to popd before exiting the script. : Do ${var,,} and ${var^^} into variables instead of using it everywhere. : Add more punctuation in comments. Please format this as a list.
Hello Andrew McRae, Edward O'Callaghan, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35605
to look at the new patch set (#4).
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
util/mb/google/hatch: fix style issues in shell script
* Use all caps for variables. * Use a single exit code for failures. * No need to popd before exiting the script. * Do ${var,,} and ${var^^} into variables instead of using it everywhere. * Add more punctuation in comments.
Signed-off-by: Paul Fagerburg pfagerburg@chromium.org Change-Id: I63aa0aa633f36b9543e809fc42fac955da5960a3 --- M util/mainboard/google/hatch/create_coreboot_variant.sh 1 file changed, 20 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/35605/4
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35605/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35605/3//COMMIT_MSG@9 PS3, Line 9: Use all caps for variables. : Use a single exit code for failures. : No need to popd before exiting the script. : Do ${var,,} and ${var^^} into variables instead of using it everywhere. : Add more punctuation in comments.
Please format this as a list.
Done
Hello Andrew McRae, Edward O'Callaghan, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35605
to look at the new patch set (#5).
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
util/mb/google/hatch: fix style issues in shell script
Use all caps for variables. Use a single exit code for failures. No need to popd before exiting the script. Do ${var,,} and ${var^^} into variables instead of using it everywhere. Add more punctuation in comments.
Signed-off-by: Paul Fagerburg pfagerburg@chromium.org Change-Id: I63aa0aa633f36b9543e809fc42fac955da5960a3 --- M util/mainboard/google/hatch/create_coreboot_variant.sh 1 file changed, 20 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/35605/5
Hello Andrew McRae, Edward O'Callaghan, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35605
to look at the new patch set (#7).
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
util/mb/google/hatch: fix style issues in shell script
* Use all caps for variables. * Use a single exit code for failures. * No need to popd before exiting the script. * Do ${var,,} and ${var^^} into variables instead of using it everywhere. * Add more punctuation in comments.
Signed-off-by: Paul Fagerburg pfagerburg@chromium.org Change-Id: I63aa0aa633f36b9543e809fc42fac955da5960a3 --- M util/mainboard/google/hatch/create_coreboot_variant.sh 1 file changed, 24 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/35605/7
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35605/2/util/mainboard/google/hatch... File util/mainboard/google/hatch/create_coreboot_variant.sh:
https://review.coreboot.org/c/coreboot/+/35605/2/util/mainboard/google/hatch... PS2, Line 56: variant
${variant} -> ${VARIANT}
Done
https://review.coreboot.org/c/coreboot/+/35605/2/util/mainboard/google/hatch... PS2, Line 57: cp -pr "${SRC}/template/." "variants/${variant}/"
${variant} -> ${VARIANT}
Done
Hello Andrew McRae, Edward O'Callaghan, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35605
to look at the new patch set (#8).
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
util/mb/google/hatch: fix style issues in shell script
* Use all caps for variables. * Use a single exit code for failures. * No need to popd before exiting the script. * Do ${var,,} and ${var^^} into variables instead of using it everywhere. * Add more punctuation in comments. * Specify LC_ALL=C so that upper/lower case show the desired behavior.
Signed-off-by: Paul Fagerburg pfagerburg@chromium.org Change-Id: I63aa0aa633f36b9543e809fc42fac955da5960a3 --- M util/mainboard/google/hatch/create_coreboot_variant.sh 1 file changed, 26 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/35605/8
Andrew McRae has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
Patch Set 8: Code-Review+1
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
Patch Set 8: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35605 )
Change subject: util/mb/google/hatch: fix style issues in shell script ......................................................................
util/mb/google/hatch: fix style issues in shell script
* Use all caps for variables. * Use a single exit code for failures. * No need to popd before exiting the script. * Do ${var,,} and ${var^^} into variables instead of using it everywhere. * Add more punctuation in comments. * Specify LC_ALL=C so that upper/lower case show the desired behavior.
Signed-off-by: Paul Fagerburg pfagerburg@chromium.org Change-Id: I63aa0aa633f36b9543e809fc42fac955da5960a3 Reviewed-on: https://review.coreboot.org/c/coreboot/+/35605 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Andrew McRae amcrae@chromium.org Reviewed-by: Edward O'Callaghan quasisec@chromium.org --- M util/mainboard/google/hatch/create_coreboot_variant.sh 1 file changed, 26 insertions(+), 26 deletions(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Andrew McRae: Looks good to me, but someone else must approve
diff --git a/util/mainboard/google/hatch/create_coreboot_variant.sh b/util/mainboard/google/hatch/create_coreboot_variant.sh index d4256a6..739c2f1 100755 --- a/util/mainboard/google/hatch/create_coreboot_variant.sh +++ b/util/mainboard/google/hatch/create_coreboot_variant.sh @@ -13,6 +13,8 @@ # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU General Public License for more details.
+export LC_ALL=C + if [[ "$#" -ne 1 ]]; then echo "Usage: $0 variant_name" echo "e.g. $0 kohaku" @@ -26,54 +28,52 @@ # you to specify the baseboard as one of the cmdline arguments. # # This is the name of the base board that we're cloning to make the variant. -base="hatch" -# This is the name of the variant that is being cloned -# ${var,,} converts to all lowercase -variant="${1,,}" +BASE="hatch" +# This is the name of the variant that is being cloned. +# ${var,,} converts to all lowercase; ${var^^} is all uppercase. +VARIANT="${1,,}" +VARIANT_UPPER="${VARIANT^^}"
# This script and the templates live in util/mainboard/google/hatch # We need to create files in src/mainboard/google/hatch -pushd "${BASH_SOURCE%/*}" || exit +pushd "${BASH_SOURCE%/*}" || exit 1 SRC=$(pwd) -popd || exit -pushd "${SRC}/../../../../src/mainboard/google/${base}" || { - echo "The baseboard directory for ${base} does not exist."; - exit; } +popd || exit 1 +pushd "${SRC}/../../../../src/mainboard/google/${BASE}" || { + echo "The baseboard directory for ${BASE} does not exist."; + exit 1; }
-# Make sure the variant doesn't already exist -if [[ -e variants/${variant} ]]; then - echo "variants/${variant} already exists." +# Make sure the variant doesn't already exist. +if [[ -e variants/${VARIANT} ]]; then + echo "variants/${VARIANT} already exists." echo "Have you already created this variant?" - popd || exit - exit 2 + exit 1 fi
# Start a branch. Use YMD timestamp to avoid collisions. DATE=$(date +%Y%m%d) -git checkout -b "create_${variant}_${DATE}" +git checkout -b "create_${VARIANT}_${DATE}" || exit 1
-# Copy the template tree to the target -mkdir -p "variants/${variant}/" -cp -pr "${SRC}/template/." "variants/${variant}/" -git add "variants/${variant}/" +# Copy the template tree to the target. +mkdir -p "variants/${VARIANT}/" +cp -pr "${SRC}/template/." "variants/${VARIANT}/" +git add "variants/${VARIANT}/"
# Now add the new variant to Kconfig and Kconfig.name # These files are in the current directory, e.g. src/mainboard/google/hatch -"${SRC}/kconfig.py" --name "${variant}" +"${SRC}/kconfig.py" --name "${VARIANT}"
mv Kconfig.new Kconfig mv Kconfig.name.new Kconfig.name
git add Kconfig Kconfig.name
-# Now commit the files -git commit -sm "${base}: Create ${variant} variant +# Now commit the files. +git commit -sm "${BASE}: Create ${VARIANT} variant
BUG=none -TEST=util/abuild/abuild -p none -t google/${base} -x -a -make sure the build includes GOOGLE_${variant^^}" - -popd || exit +TEST=util/abuild/abuild -p none -t google/${BASE} -x -a +make sure the build includes GOOGLE_${VARIANT_UPPER}"
echo "Please check all the files (git show), make any changes you want," echo "and then push to coreboot HEAD:refs/for/master"
-
Andrew McRae (Code Review) -
Edward O'Callaghan (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Fagerburg (Code Review)
-
Paul Menzel (Code Review)