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:
(2 comments)
File util/genbuild_h/genbuild_h.sh:
https://review.coreboot.org/c/coreboot/+/81290/comment/7b85f4fe_b16ac903 : 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')"
Would that be posix conform?
The `-E` is posix compliant extended regex. `-r` would be the non posix compliant extended regex (although I am not sure if or how much they actually differ, but to be safe lets stay with `-E`).
https://review.coreboot.org/c/coreboot/+/81290/comment/4c9ffbb4_b7cf235e : PS1, Line 40: *
Can you give an example? I wouldn't be sure how to do that and handle leading […]
We only need the leading zero for the month since the year will never have one. But I forgot about the month, which may have a leading zero. But we could replace the `*` with a `+` for the month, because we will always have at least one number for the minor version (month). ``` 's/^([0-9]{2}).0*([0-9]+).*/\2/p' ```
Regarding other version formats as described in the commit: The substitution that you have currently in place also does not work for the version tag described in the commit-msg ("v1.9308_26_0.0.22"), because your substitution requires a version number to be at the start (`^`) of the line, which means a `v` at the beginning is not valid. We would need something like this: ``` s/^v*0*([0-9]... ``` But honestly I am not sure how far we want to stretch the support for people creating tags with their own custom version format, especially now after we have completely changed it. If it was up to me I would just force our format for the tags and let others deal with their special cases.