Attention is currently required from: Martin L Roth, Nico Huber.
Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81290?usp=email )
Change subject: genbuild_h: Fix and harden major/minor version parsing ......................................................................
Patch Set 1: Code-Review+2
(2 comments)
File util/genbuild_h/genbuild_h.sh:
https://review.coreboot.org/c/coreboot/+/81290/comment/826b60cc_4877ec6a : PS1, Line 39: MAJOR_VER="$(echo "${VERSION}" | sed -n 's/^0*([0-9]*).0*([0-9]*).*/\1/p')" : MINOR_VER="$(echo "${VERSION}" | sed -n 's/^0*([0-9]*).0*([0-9]*).*/\2/p')"
But I looked a little into sed now. If sed should support the `-E', seems in flux. The Open Group doesn't mention it [1]. Yet the GNU sed history claims since 2013 it would be standardized.
As always a lot of annoying inconsistency. Personally I don't care and would just assume that it is compatible enough for running on the systems that we actually support. But I agree about not putting it into a bug fix. I guess then lets just play it safe for now (and probably forever).
https://review.coreboot.org/c/coreboot/+/81290/comment/cdb56c96_9584cd30 : PS1, Line 40: *
Without -E we don't have + nor {}.
My proposal was to add `-E`, but I guess we don't do that now.
Also, the {2} would feel like documenting when we started to use the new version scheme (after 2009) and would make other (downstream) versioning harder without gaining us anything.
I try to be as explicit as possible for regular expressions in order to avoid surprises through too ambiguously described regex expressions. But I guess if we really want to support other versioning schemes we don't really have a choice. Fine by me
-
Maximilian Brune (Code Review)