build system: add machine-readable RIOT_VERSION_CODE macro

[benpicco]

Contribution description

For external modules it can be handy to determine the RIOT version using a define.
Linux provides a KERNEL_VERSION macro for that. It splits a 64 bit version number into 16 bit mayor version, 16 bit minor version 16 bit patch level and user defined 16 bit extraversion.

This also works for RIOT's versioning scheme, so let's adopt the same scheme.

Testing procedure

A new unit test is provided in tests/unittests/tests-kernel_defines.

Issues/PRs references

https://forum.riot-os.org/t/detect-riot-os-version/3329

[maribu]

Note that for Linux stable versions the 8 bit counter had to few bits. I don't think that RIOT will have stable releases with so many bugfix releases any time soon. But since this is hardly something that is going into hot code paths, why not pack 32 bit, 16 bit, and 16 bit into a 64 bit number. Just to reduce the chance that a future version of us travels back in time to kick our asses.

[aabadie]

This PR expects that RIOT_VERSION is always containing something like 2021.10 or 2021.10.1 but on a development branch like master, RIOT_VERSION contains something like 2021.10-devel-130-g934c87 and on a different branch it also contains the branch name.
With the example above, 2021.10-devel-130-g934c87, the resulting RIOT_VERSION_CODE macro would contain KERNEL_VERSION\(2021,10,0\) which returns a wrong result since the master branch doesn't correspond to an exact RIOT release version but a development version.
We could keep the extra devel-130-g934c87 in the result (e.g KERNEL_VERSION\(2021,10,0devel-130-g934c87\)) but then the KERNEL_VERSION would have to be adapted (and I don't know how).

[benpicco]

I don't think there is another way to do it - arbitrary branches can't be encoded in a numerical format.

I would expect external modules to only build against released branches anyway.

[fjmolinas]

I don't think there is another way to do it - arbitrary branches can't be encoded in a numerical format.

I would expect external modules to only build against released branches anyway.

Lets just add a comment that this only applies for released branches, and include this.

[kaspar030]

should we think of some meaning of "version"? like, make it somehow semantic? give it two numbers, one for "breaking changes", one for additions?

edit and make CI check that any PR marked "API change" increases the number? and add a document that explains any breaking changes and how to fix older code?

[benpicco]

That's actually hard to track and the reason why most projects abandoned the idea.

Also this PR is not about changing the versioning scheme that RIOT uses, but rather just adding a macro to check the version at compile time.

[kaspar030]

That's actually hard to track and the reason why most projects abandoned the idea.

semver you mean? not sure about "most projects abandoned the idea". Also, if it is hard to track when our public API changes, we have a problem.

I agree that this PR just allows checking the version in code. we can discuss the actual versioning scheme elsewhere.

[miri64]

2021.10-devel-130-g934c87

Alternatively, a workaround could be that a string like this resolves to KERNEL_VERSION(2021, 9, 130). Of course for different devel branches this could yield the same result.

[kaspar030]

Of course for different devel branches this could yield the same result.

do we want to support finer grained versioning than our releases? dev branches could resolve to e.g., KERNEL_VERSION(2021,9,9999)?

[miri64]

do we want to support finer grained versioning than our releases? dev branches could resolve to e.g., KERNEL_VERSION(2021,9,9999)?

Alternatively the actual release could have an extra flag (1) and the devel describe towards it has it just unset.

[miri64]

And for testing it could be beneficial to also have comparability to dev branches at least.

[benpicco]

do we want to support finer grained versioning than our releases?

Where are those numbers supposed to come from?

What I could imagine could be a RIOT_EXTRA_VERSION ?= 0 and then KERNEL_VERSION(mayor, minor, patch, extra) for downstream forks.

e.g. we have a 2021.07-mlpa-5 branch that is the 5th iteration of the base branch based on the 2021.07 branch.

[miri64]

Where are those numbers supposed to come from?

git describe counts the number of commits from the base tag. That's where the 130 in my original proposal came.

[aabadie]

Should this PR also take into account the fact that on the CI (or when building with RIOT_CI_BUILD=1), RIOT_VERSION contains only buildtest ?

[benpicco]

Since the unit test does some sanity checks on RIOT_VERSION_CODE, those would then fail on CI.
Is there no way to get the RIOT version on CI?

[miri64]

Is there no way to get the RIOT version on CI?

We could use the verbatim version, we use in devel branches, but then the build size would not be reliable anymore.

[benpicco]

We could use the verbatim version, we use in devel branches, but then the build size would not be reliable anymore.

git describe | sed -E 's/([0-9]+).([0-9]+).*/\1.\2/'

always only returns the release branch

[miri64]

git describe | sed -E 's/([0-9]+).([0-9]+).*/\1.\2/'

always only returns the release branch

1. Why the excessive use of sed?

   git describe | grep -Eo '([0-9]{4}).([0-9]{2})'
   

2. That would be imprecise. All RCs and all commits towards a release would then be versioned as THE release.

[benpicco]

2. That would be imprecise. All RCs and all commits towards a release would then be versioned as THE release.

It would still increase the precision over calling them all buildtest

[miri64]

Did you check how often this is executed?

[benpicco]

--- a/Makefile.include
+++ b/Makefile.include
@@ -519,7 +519,7 @@ ifneq (,$(RIOT_VERSION_OVERRIDE))
 endif
 
 # Generate machine readable RIOT VERSION macro
-RIOT_VERSION_CODE ?= $(shell echo ${RIOT_VERSION} | \
+RIOT_VERSION_CODE ?= $(shell echo eval 1>&2 && echo ${RIOT_VERSION} | \
                       sed -E 's/([0-9]+).([0-9]+).?([0-9]+)?.*/KERNEL_VERSION\\\(\1,\2,0\3,${RIOT_EXTRAVERSION}\\\)/')
 
 # Set module by prepending APPLICATION name with 'application_'.

prints eval exactly once

[miri64]

Confirmed with piping the to ${RIOTBASE}/test.eval instead of stderr.

[miri64]

(ping @kaspar030)

[aabadie]

I think the RIOT_VERSION_CODE should be added to the DOCKER_ENV_VARS:

RIOT/makefiles/docker.inc.mk

Line 27 in fb05da4

export DOCKER_ENV_VARS += \

otherwise I think this won't work when BUILD_IN_DOCKER is set to 1.

[kfessel]

if somone changes the release cycle may(5) in 2042 may be a regular Release ->

    RIOT_VERSION_CODE ?= RIOT_VERSION_NUM\(2042,99,23,0\)

Is that Feb 2050?

[miri64]

Same goes, if we change for a semantic versioning scheme, but keep the year number for backwards compatibility. IMHO this is not really important. The main thing, is that the version in CI is static and since it is not planned to use this macro in-tree, it will never come to a versioning conflict within the CI.

[kfessel]

I think this macro might have some use inside packages that have a source that has the changes to make them work with riot upstream.
Some packages are tested in the CI ?afaik?

[miri64]

If a package requires a different RIOT version, a different version of that package should be used. If we allow RIOT_VERSION_CODE within things the CI is using, it has to be non-static and we would lose as such both caching advantages and size comparison capabilities offered by the CI.

[miri64]

(i.e. we would loose deterministic build results within the CI)

[miri64]

You may squash after addressing my last change request.

[chrysn]

Summarizing some chat exchange, I still think that rather than having anyone rely on particular versions we should rather expose features declared in, say, <apifeatures.h>, that indicate that a particular change was enacted. Unlike versions, this would naturally work with any number of branches, releases or merges, for as long as these names (eg. #define RIOT_APIFEATURE_CPUINIT_CALLED_FROM_STARTUP) are unique. They'd be kept indefinitely (but might after many releases be marked for deprecation in order to hint to users that unless they use really old versions they can just as well assume the feature to be present).

Sadly I don't have the time right now to follow this through completely, but I still suggest that it be held as an available option for cases in which version number checking reaches its limitations.

[miri64]

Sadly I don't have the time right now to follow this through completely, but I still suggest that it be held as an available option for cases in which version number checking reaches its limitations.

I think having something like you propose can be done in addition to the proposal here. RIOT_VERSION_CODE would then be more coarse grained, while feature defines / API versions would be used for finer grained code checking.

[benpicco]

The problem I see with that approach is that it depends on the contributors to always keep the apifeatures.h file up to date.

The version number on the other hand changes automatically and external boards/modules are not at the mercy of the attentiveness of contributors to flag possible API changes properly.

[fjmolinas]

@kaspar030 are you happy with the current state?

[kaspar030]

@kaspar030 are you happy with the current state?

Medium, but the CI issues are gone.

CI is fine.

[miri64]

Might be a good idea to add a coccinelle rule, to prevent further merging of RIOT_VERSION_CODE to master.