Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35553 )
Change subject: util/mainboard/google: Fix hatch variant script ......................................................................
util/mainboard/google: Fix hatch variant script
The script had a couple of bugs: * It didn't create the required directory under variants/ * It was treating the wildcard as literal and so couldn't find variant files to copy.
Change-Id: Ie6f4179014b79ea45d0fcf406ca192046438dbf7 Signed-off-by: Edward O'Callaghan quasisec@chromium.org --- M util/mainboard/google/hatch/create_coreboot_variant.sh 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/35553/1
diff --git a/util/mainboard/google/hatch/create_coreboot_variant.sh b/util/mainboard/google/hatch/create_coreboot_variant.sh index 32720ca..352db8d 100755 --- a/util/mainboard/google/hatch/create_coreboot_variant.sh +++ b/util/mainboard/google/hatch/create_coreboot_variant.sh @@ -53,7 +53,8 @@ git checkout -b "create_${variant}_${DATE}"
# Copy the template tree to the target -cp -r "${SRC}/template/*" "variants/${variant}/" +mkdir "variants/${variant}/" +cp -vr "${SRC}/template"/* "variants/${variant}/" git add "variants/${variant}/"
# Now add the new variant to Kconfig and Kconfig.name
Andrew McRae has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35553 )
Change subject: util/mainboard/google: Fix hatch variant script ......................................................................
Patch Set 1:
(2 comments)
I'm always in favour of idempotent scripts hence the suggestion to use mkdir -p. Not sure if the script has other places where fixes would be useful to make the script idempotent.
https://review.coreboot.org/c/coreboot/+/35553/1/util/mainboard/google/hatch... File util/mainboard/google/hatch/create_coreboot_variant.sh:
https://review.coreboot.org/c/coreboot/+/35553/1/util/mainboard/google/hatch... PS1, Line 56: mkdir Use mkdir -p
https://review.coreboot.org/c/coreboot/+/35553/1/util/mainboard/google/hatch... PS1, Line 57: - Use cp -pr "$(SRC)/template/."
Using a wildcard will miss any dot files, and -p will preserve any exec bits. Using '-v' is perhaps not necessary IMHO, since my own preference is to avoid noisy scripts. It would be nice if there was a verbose flag that could be passed, but often sh -x is used to provide a trace of what happens anyway.
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35553 )
Change subject: util/mainboard/google: Fix hatch variant script ......................................................................
Patch Set 1:
+1 to all of amcrae@'s comments.
Please note that these scripts are still in the very early stage. There is some ongoing discussion with the build team about support responsibilities, and this script (along with others in the main chromimumos tree) may get moved wholesale into another location, rather than living in the trees that they modify.
Hello Andrew McRae, Paul Fagerburg, Daniel Kurtz, build bot (Jenkins), Furquan Shaikh, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35553
to look at the new patch set (#2).
Change subject: util/mainboard/google: Fix hatch variant script ......................................................................
util/mainboard/google: Fix hatch variant script
The script had a couple of bugs: * It didn't create the required directory under variants/ * It was treating the wildcard as literal and so couldn't find variant files to copy.
V.2: Drop verbose cp && fixup wild card usage.
Change-Id: Ie6f4179014b79ea45d0fcf406ca192046438dbf7 Signed-off-by: Edward O'Callaghan quasisec@chromium.org --- M util/mainboard/google/hatch/create_coreboot_variant.sh 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/35553/2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35553 )
Change subject: util/mainboard/google: Fix hatch variant script ......................................................................
Patch Set 2:
(2 comments)
Patch Set 1:
(2 comments)
I'm always in favour of idempotent scripts hence the suggestion to use mkdir -p. Not sure if the script has other places where fixes would be useful to make the script idempotent.
https://review.coreboot.org/c/coreboot/+/35553/1/util/mainboard/google/hatch... File util/mainboard/google/hatch/create_coreboot_variant.sh:
https://review.coreboot.org/c/coreboot/+/35553/1/util/mainboard/google/hatch... PS1, Line 56: mkdir
Use mkdir -p
Done
https://review.coreboot.org/c/coreboot/+/35553/1/util/mainboard/google/hatch... PS1, Line 57: -
Use cp -pr "$(SRC)/template/." […]
Done
Andrew McRae has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35553 )
Change subject: util/mainboard/google: Fix hatch variant script ......................................................................
Patch Set 2: Code-Review+1
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35553 )
Change subject: util/mainboard/google: Fix hatch variant script ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35553 )
Change subject: util/mainboard/google: Fix hatch variant script ......................................................................
util/mainboard/google: Fix hatch variant script
The script had a couple of bugs: * It didn't create the required directory under variants/ * It was treating the wildcard as literal and so couldn't find variant files to copy.
V.2: Drop verbose cp && fixup wild card usage.
Change-Id: Ie6f4179014b79ea45d0fcf406ca192046438dbf7 Signed-off-by: Edward O'Callaghan quasisec@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/35553 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Andrew McRae amcrae@chromium.org Reviewed-by: Paul Fagerburg pfagerburg@chromium.org --- M util/mainboard/google/hatch/create_coreboot_variant.sh 1 file changed, 2 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Paul Fagerburg: 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 32720ca..d4256a6 100755 --- a/util/mainboard/google/hatch/create_coreboot_variant.sh +++ b/util/mainboard/google/hatch/create_coreboot_variant.sh @@ -53,7 +53,8 @@ git checkout -b "create_${variant}_${DATE}"
# Copy the template tree to the target -cp -r "${SRC}/template/*" "variants/${variant}/" +mkdir -p "variants/${variant}/" +cp -pr "${SRC}/template/." "variants/${variant}/" git add "variants/${variant}/"
# Now add the new variant to Kconfig and Kconfig.name
-
Andrew McRae (Code Review) -
Edward O'Callaghan (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Fagerburg (Code Review)