Skip to content

Conversation

@marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Apr 28, 2023

As discussed in #6952, relying on git describe for defining SOF_MAJOR and friends is very brittle and unreliable. Switch to a static versions.json file instead.

Note the full git describe --tags output is still present in the binary, this is useful and left unchanged. It's just not used any more to guess SOF_MAJOR, SOF_MINOR and SOF_MICRO.

Use JSON because CMake and pretty much every other piece of software has a JSON parser out of the box.

This aligns with linux/Makefile, Zephyr/VERSIONS and probably many others. - except we use JSON because we're in 2023 so we don't spend time having fun re-implementing parsers any more.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Apr 28, 2023

This is ready for review but merge is blocked on the upgrade of the Docker image (#7540)

CMake 3.19 is required for the JSON parser. The first 3.19 version was released at the end of 2020: https://github.com/Kitware/CMake/releases/tag/v3.19.0

These all passed for the most part

EDIT: now just one suspend/resume failure in https://sof-ci.01.org/sofpr/PR7532/build6883/devicetest/index.html, everything else green.

As discussed in thesofproject#6952, relying on `git describe` for defining SOF_MAJOR
and friends is very brittle and unreliable. Switch to a static
versions.json file instead.

Note the full `git describe --tags` output is _still_ present in the
binary, this is useful and left unchanged. It's just not used any more
to guess SOF_MAJOR, SOF_MINOR and SOF_MICRO.

Use JSON because CMake and pretty much every other piece of software has
a JSON parser out of the box.

This aligns with linux/Makefile, Zephyr/VERSIONS and probably many
others. - except we use JSON because we're in 2023 so we don't spend
time having fun re-implementing parsers any more.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb marc-hb force-pushed the static-versions branch from 3ca9547 to ab43253 Compare May 2, 2023 20:06
@marc-hb marc-hb marked this pull request as ready for review May 2, 2023 20:16
Copy link
Collaborator

@dbaluta dbaluta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good start from moving on relying on git in order to get the version.

Copy link
Collaborator

@paulstelian97 paulstelian97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great change. While it does create issues with redundant information between tags (which won't disappear) and the actual version, I believe we can take enough care to keep it in sync. So I'm more on the "yes this is good" side of the fence, approving.

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @marc-hb ! We need to add this to the release checklist for 2.6 once that is created. FYI @mengdonglin @lgirdwood

@kv2019i
Copy link
Collaborator

kv2019i commented May 4, 2023

One fail in https://sof-ci.01.org/sofpr/PR7532/build6883/devicetest/index.html , not related, proceeding with merge.

@kv2019i kv2019i merged commit 11592fe into thesofproject:main May 4, 2023
@marc-hb marc-hb deleted the static-versions branch May 5, 2023 23:32
@marc-hb
Copy link
Collaborator Author

marc-hb commented May 5, 2023

Thanks @marc-hb ! We need to add this to the release checklist for 2.6 once that is created. FYI @mengdonglin @lgirdwood

Submitted:

Edited:

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.

4 participants