Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33509 )
Change subject: csb_patcher.sh: gets,checks,installs the coreboot and SeaBIOS patches ......................................................................
Patch Set 7:
(8 comments)
https://review.coreboot.org/c/coreboot/+/33509/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/33509/7//COMMIT_MSG@29 PS7, Line 29: ./csb_patcher.sh Maybe move it into util/scripts or make a new directory under util for it? It won't get merged into the coreboot root directory.
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh File csb_patcher.sh:
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh@10 PS7, Line 10: Add a license?
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh@34 PS7, Line 34: printf "${bred}TERMINATED${bend}\n" Here's what I'm using in my scripts. This avoids all the shellcheck warnings.
_echo_color() { local color if [[ -z ${NOCOLOR} ]]; then color="$1" else unset NC fi local text=$2 local cr=$3
if [[ -z ${cr} ]]; then printf "${color}%s${NC}\n" "${text}" else printf "${color}%s${NC}" "${text}" fi }
_echo_debug() { if [[ -z ${VERBOSE} ]]; then return fi printf "${ORANGERED}%s${NC}\n" "$2" >&2 }
_echo_error() { (_echo_color >&2 "${RED}" "$*") }
then just:
_echo_color "${bred}" "TERMINATED"
I have a command line argument to run the scripts without color codes - that's where the ${NOCOLOR} gets set. obviously my ${NC} is the same as your ${bend}.
Feel free to use this or ignore it. I'll give you under whatever license you prefer.
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh@79 PS7, Line 79: else : return 0 : fi Get rid of the else? This does the same thing. Same pattern below.
fi return 0
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh@172 PS7, Line 172: ! -z -n?
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh@277 PS7, Line 277: floppy_mass_downloader () Move to a separate script? This isn't really related to doing patches, and would be useful by itself.
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh@279 PS7, Line 279: "./floppies/" payloads/floppies? 3rdparty/flopppies?
https://review.coreboot.org/c/coreboot/+/33509/7/csb_patcher.sh@765 PS7, Line 765: csb_mass_patcher Instead of hardcoding these, it would be good to maybe pull them from a patch. It would be nice to be able to supply that patch on the command line, although defaulting something that points to your patchlist is fine, IMO.
That way this script would be useful other people's patch sets as well.