Skip to content

Make micro version optional for firmware file version string argument - #95

Closed
mengdonglin wants to merge 1 commit into
thesofproject:mainfrom
mengdonglin:zephyr-build-version
Closed

Make micro version optional for firmware file version string argument#95
mengdonglin wants to merge 1 commit into
thesofproject:mainfrom
mengdonglin:zephyr-build-version

Conversation

@mengdonglin

@mengdonglin mengdonglin commented May 23, 2022

Copy link
Copy Markdown
Contributor

This is a preparation to fix thesofproject/sof#5775. Another SOF PR thesofproject/sof#5846 is to update xtensa-build-zephyr.py

rimage can accept firmware file version string like major.minor
or major.minor.micro. The 3rd field 'micro' is optional.

For building with XOTS, SOF CMake always passes version string
major.minor.micro to rimage.

But for building with Zephyr, xtensa-build-zephyr.py will extract
version string from latest git tag which can be either major.minor
or major.minor.micro

Signed-off-by: mengdonglin mengdong.lin@intel.com

@aiChaoSONG

Copy link
Copy Markdown

I don't think we should make the version number flexible. If we want to use MAJOR.MINOR.HOTFIX, we should stick to it. It looks not good to me that sometimes MAJOR.MINOR.HOTFIX is used, but sometimes MAJOR.MINOR is used. For the place we use MAJOR.MINOR, we should really use MAJOR.MINOR.0. Or we should really improve the tag naming conversion.

rimage can accept firmware file version string like major.minor
or major.minor.micro. The 3rd field 'micro' is optional.

For building with XOTS, SOF CMake always passes version string
major.minor.micro to rimage.

But for building with Zephyr, xtensa-build-zephyr.py will extract
version string from latest git tag which can be either major.minor
or major.minor.micro

Signed-off-by: mengdonglin <mengdong.lin@intel.com>
@mengdonglin

mengdonglin commented May 23, 2022

Copy link
Copy Markdown
Contributor Author

I don't think we should make the version number flexible. If we want to use MAJOR.MINOR.HOTFIX, we should stick to it. It looks not good to me that sometimes MAJOR.MINOR.HOTFIX is used, but sometimes MAJOR.MINOR is used. For the place we use MAJOR.MINOR, we should really use MAJOR.MINOR.0. Or we should really improve the tag naming conversion.

@aiChaoSONG Thanks for your review. Both major.minor and major.minor.micro are widely used in software versioning. That's why I hope to keep some flexibility here. When major.minor is enough, we needn't use major.minor.0

@aiChaoSONG

Copy link
Copy Markdown

When major.minor is enough, we needn't use major.minor.0

In the underlining code, all three slots for these three numbers are in the C structure, even we don't pass them from command line, they get the default value 0.0.0. We have been using MAJOR.MINOR.HOTFIX pattern always.

@lgirdwood If the hotfix(micro) number is zero, it is removed from the tag, we just simply use major.minor in the tag, which I think is not good. Since we use MAJOR.MINOR.HOTFIX versioning, we should have them all in the tags. Actually, Zephyr/CPython and many other softwares use x.y.z explicitly even z==0.

@marc-hb

marc-hb commented May 23, 2022

Copy link
Copy Markdown
Contributor

But for building with Zephyr, xtensa-build-zephyr.py will extract version string from latest git tag which can be either major.minor or major.minor.micro

I don't understand why building with Zephyr can make any difference to the SOF version, I don't think it should. When building from the same SOF commit then the SOF version should always be the same whether we're building with or without Zephyr. There should be other differences but the SOF version should stay the same.

@aiChaoSONG

Copy link
Copy Markdown

When building from the same SOF commit then the SOF version should always be the same whether we're building with or without Zephyr.

I think this is because we name tag differently. If z == 0 in x.y.z, we just drop the z field.
image

@lgirdwood

Copy link
Copy Markdown
Member

When building from the same SOF commit then the SOF version should always be the same whether we're building with or without Zephyr.

I think this is because we name tag differently. If z == 0 in x.y.z, we just drop the z field. image

Ack, if z is zero it is dropped from the version. Likewise if tag is not used we also append the commit ID.

SOF uses exactly the same versioning scheme as Linux, our scripts need to do the same. @mengdonglin pls confirm if your PRs follow Linux versioning.

@marc-hb

marc-hb commented May 24, 2022

Copy link
Copy Markdown
Contributor

the SOF version should always be the same whether we're building with or without Zephyr.

I think this is because we name tag differently. If z == 0 in x.y.z, we just drop the z field.

You seem to be referring to something else, not sure what.

What I'm saying is: sof_versions.h is the same with Zephyr and without Zephyr. Try it and see for yourself. The commit message seems to imply we should make a difference there. I think we should not. Whatever we do with the content of sof_versions.h, it should be the same with or without Zephyr.

@mengdonglin

Copy link
Copy Markdown
Contributor Author

@lgirdwood I think my PRs follow Linux versioning.
I'll try to update xtensa-build-zephyr.py as @marc-hb and @aiChaoSONG: we always pass a 3-field version string major.minor.micro to rimage. If git tag doesn't include the micro version, the build script will generate a version string major.minor.0 and pass it to rimage. So we needn't change SOF versioning or rimage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] embed SOF version to FW manifest header on Zephyr builds

4 participants