Regular expressions are notoriously buggy: please provide an example of the parsing issue fixed and explain how this fixes it.

Marc, thank you for your comments,

About "--dirty", I did not know it was useful. I will restore this parameter, thanks.

The main problem here is that we are using many different versions of git tag to marked branch/releases. That's why I decided to change the formula for catching firmware versions. For example, some recent tags used:

• v2.4.1
• releases/mtl/v2.0-hotfix3
• v2.1.1a
• releases/rpl/v1.0
• v1.9.3-tplg2
• v.2.2-rc1

With the previous formula we only considered tags beginning with "v". This didn't work with some of the tags mentioned above. Now, it should work with tags like below:

• v2.0
• v2.0.1
• v2.0-hotfix1
• v2.0-rc1
• releases/mtl/v2.0
• releases/mtl/v2.0.1
• releases/mtl/v2.0-hotfix1

where:

• SOF_MAJOR=2,
• SOF_MINOR=0,
• SOF_MICRO=1 (if existing in tag e.g. v2.0.1, v2.0.-rc1 etc., otherwise 0).
  In addition, SOF_BUILD will indicate the commits number since the last known tag (if newest commit was tagged then SOF_BUILD = 0)

Thanks, I understand a bit better now.

I think we should simply stop parsing git tags, it's too brittle. Everyone and everything should be able to create any temporary (and impossible to parse) tag in their workspace without breaking functionality! Recent example:

• [BUG][ADL-P] DSP panic on device boot-up or resume-from-suspend for sof-adl-max98357a-rt5682-2way tplg #6964 (comment)

This would also solve better the previously discussed issue

• How do we use tags on master branch? #3730

git describe is still extremely useful; it's still critical to have in the test logs. However no functionality should depend on it.

This exactly how Zephyr solved that problem. On one hand it has BUILD_VERSION = $(git describe ...) defined in zephyr/cmake/gen_version_h.cmake. BUILD_VERSION is not parsed and does not affect functionality.

On the other hand it has zephyr/VERSION which has versions checked in git and manually updated.

The Linux kernel does the same in https://github.com/thesofproject/linux/commits/topic/sof-dev/Makefile

If we really really wanted to parse git describe (we must not), then we should not waste time debugging super error-prone regular expressions. We should instead re-use existing code like for instance Python's standard and well documented "packaging" library:

ipython3

from packaging.version import parse as parse_version

In [16]: [ parse_version(x).release for x in [ 'v2.4.1', 'v2.2.1a', 'v2.2-rc1' ]]
Out[16]: [(2, 4, 1), (2, 2, 1), (2, 2)]

In [17]: [ parse_version(x).pre     for x in [ 'v2.4.1', 'v2.2.1a', 'v2.2-rc1' ]]
Out[17]: [None, ('a', 0), ('rc', 1)]

Separating git describe and functionality would also solve some of the problems with shallow clones, see for instance b5219d8, recent 89ddee6, others.

With the previous formula we only considered tags beginning with "v". This didn't work with some of the tags mentioned above. Now, it should work with tags like below:

• v2.0
• v2.0.1
• v2.0-hotfix1
• v2.0-rc1
• releases/mtl/v2.0
• releases/mtl/v2.0.1
• releases/mtl/v2.0-hotfix1

Not keen on the last ones, the v prefix is pretty well understood to mean a released version. We can suffix any desired metadata after the v1.2.3-somesuffix

where:

• SOF_MAJOR=2,
• SOF_MINOR=0,
• SOF_MICRO=1 (if existing in tag e.g. v2.0.1, v2.0.-rc1 etc., otherwise 0).
  In addition, SOF_BUILD will indicate the commits number since the last known tag (if newest commit was tagged then SOF_BUILD = 0)

So this is no longer a build count, but the number of commits after tag ? The former is useful for developers (but I think we have build date/time encoded somewhere if enabled ?), I've no objections to count if it also aligns with how things are done in Windows.

With the previous formula we only considered tags beginning with "v". This didn't work with some of the tags mentioned above. Now, it should work with tags like below:

• v2.0
• v2.0.1
• v2.0-hotfix1
• v2.0-rc1
• releases/mtl/v2.0
• releases/mtl/v2.0.1
• releases/mtl/v2.0-hotfix1

Not keen on the last ones, the v prefix is pretty well understood to mean a released version. We can suffix any desired metadata after the v1.2.3-somesuffix

Thanks for your input Liam,
The above examples are only taken from our repository. Based on them, I prepared the appropriate regex formula for capturing firmware versions.
I agree that the flexibility of creating git tags can make the regex formula insufficient in future. Maybe we should consider some kind of standardization for git tag?

where:

• SOF_MAJOR=2,
• SOF_MINOR=0,
• SOF_MICRO=1 (if existing in tag e.g. v2.0.1, v2.0.-rc1 etc., otherwise 0).
  In addition, SOF_BUILD will indicate the commits number since the last known tag (if newest commit was tagged then SOF_BUILD = 0)

So this is no longer a build count, but the number of commits after tag ? The former is useful for developers (but I think we have build date/time encoded somewhere if enabled ?), I've no objections to count if it also aligns with how things are done in Windows.

Yes, in my proposal for change, the SOF_BUILD will be points us the number of commits after last known/recognized git tag. Currently, SOF_BUILD is always equal to 1 on SOF (this is why I decided to use this variable)

Another thing I noticed is that currently fw versioning is not working properly on SOF. When creating a fw binary, the process does not properly recognize the last git tag so we need a solution that will help us patch this problem.

However, since the above solution is not perfect, we can plan here or in another discussion how to solve the versioning problem better. What do you think about this?

If we really really wanted to parse git describe (we must not), then we should not waste time debugging super error-prone regular expressions. We should instead re-use existing code like for instance Python's standard and well documented "packaging" library:

ipython3

from packaging.version import parse as parse_version

In [16]: [ parse_version(x).release for x in [ 'v2.4.1', 'v2.2.1a', 'v2.2-rc1' ]]
Out[16]: [(2, 4, 1), (2, 2, 1), (2, 2)]

In [17]: [ parse_version(x).pre     for x in [ 'v2.4.1', 'v2.2.1a', 'v2.2-rc1' ]]
Out[17]: [None, ('a', 0), ('rc', 1)]

It looks interesting, however we currently have a problem with fw versioning on SOF - it is not working properly. I think my proposal could solve this problem temporarily. In the meantime, we could work out a better working solution. What do you think about this?

Currently, SOF_BUILD is always equal to 1 on SOF (this is why I decided to use this variable)

SOF_BUILD is disabled by default but already used for something else, see commit 9f8cce1 and of course git grep -C 5 SOF_BUILD

releases/mtl/v2.0-hotfix1

I agree with Liam: this is most definitely not a version number. Let's please not make complex and error-prone regular expressions even more complex and even more buggy while trying to parse a git tag like that. This could be v2.0.mtl-hotfix or something like it.

Maybe we should consider some kind of standardization for git tag?

BTW have you considered the possibility below? It would make git describe ignore light-weight tags. So light-weight tags could be any crazy thing people want without affecting git describe.

--- a/scripts/cmake/version.cmake
+++ b/scripts/cmake/version.cmake
@@ -53,7 +53,7 @@ if(EXISTS ${TARBALL_VERSION_SOURCE_PATH})
 else()
        # execute_process() errors are not fatal by default!
        execute_process(
-               COMMAND git describe --tags --abbrev=12 --match v* --dirty
+               COMMAND git describe        --abbrev=12 --match v* --dirty
                WORKING_DIRECTORY ${SOF_ROOT_SOURCE_DIRECTORY}
                OUTPUT_VARIABLE GIT_TAG
                OUTPUT_STRIP_TRAILING_WHITESPACE

Standardization and parsing would then be required only for annotated tags.

I think my proposal could solve this problem temporarily.

My from packaging.version import parse as parse_version suggestion is ALSO a small and temporary solution. A smaller and less buggy suggestion than regular expressions.

In any case temporary solutions can be OK but they must still be well defined and well documented. As you noticed this is a brittle area so hasty or big changes would only makes the situation even more complex and pour fuel on the fire. Until now the comments and commit message in this PR are basically empty: they do not describe what worked, neither what did not work and is fixed by this commit and neither what still does not work after it.

Some code changes in this PR also seem irrelevant and/or unnecessary, best avoided in workarounds trying to make a broken design slightly less broken https://wiki.openstack.org/wiki/GitCommitMessages#Structural_split_of_changes

(again the best, long term solution is to STOP parsing git tags altogether and manually edit version numbers in a .h file instead)

I closed this PR draft - I will prepare a new solution based on a manually changed *.h file containing the current version of FW. New solution will be prepared in the new PR.

@dnikodem do you still intend to fix this?

@dnikodem do you still intend to fix this?

this task is waiting in my queue - I am now unable to specify when I will be able to take care of it again.

Submitted in #7532. Still needs a bit more testing but should be ready to review.

EDIT: #7532 is merged. Not merged yet:

• xtensa-build-zephyr.py: don't extract SOF_BUILD from generated .h file #7546