Resolve #770, add baseline and buildnumber to version.h

[astrogeco]

Describe the contribution
Resolve #770

Testing performed
Steps taken to test the contribution:
Built on top of current integration candidate
Built with other integration candidates as well as with nasa/osal#532 and nasa/PSP#178

Expected behavior changes
New macros defined. Startup reporting now looks like

System(s) tested on
Docker Ubuntu-based gcc image on OSX

Additional context
Also Tested with nasa/osal#532 and nasa/PSP#178

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
Gerardo E. Cruz-Ortiz, NASA-GSFC

[jphickey]

I thought the plan was to update the BUILD number via script? Should I write a script to do this?

[astrogeco]

I thought the plan was to update the BUILD number via script? Should I write a script to do this?

Yep, that's the plan. I wanted to do a quick hand implementation for now since I really don't want to increase the Revision number for merging the current IC.

Let's hold off on the script for a little bit and explore what already exists "out there"

[jphickey]

I thought the plan was to update the BUILD number via script? Should I write a script to do this?

Yep, that's the plan. I wanted to do a quick hand implementation for now since I really don't want to increase the Revision number for merging the current IC.

Let's hold off on the script for a little bit and explore what already exists "out there"

The issue is that if we are are intending to use v6.5.0a tag as our build epoch, then we are not at build "1" anymore. We are at build 311 (that is, the current output of git rev-list --count v6.5.0a..integration-candidate).

Do you have anything in mind for what already exists? This seems like a pretty simple script and a fairly unique/project-specific objective so I don't know of anything existing to do this specific task.

What I was thinking is to use cmake in its "script mode" (-E switch) which is basically what we do for the compile-time info but can just as easily be run from command line or on-demand.

[astrogeco]

I don't have anything in mind so far but we should do our due diligence before making our own.

For the epoch, I think we should use the new 6.7 tag so you're totally right that it's not "1"; that's an easy change :)

@skliper, which tag do you prefer?

[jphickey]

I don't have anything in mind so far but we should do our due diligence before making our own.

Normally I would agree, but this seems so simple I'm not seeing the benefit of trying to adapt someone else's solution - it will probably take more time in the end than writing a simple script that we can own.

But the other thing to consider is that it may impact which file this goes in. For instance right now its in a "version.h" file but perhaps we really need to have a separate "build" header file that contains nothing other than these defines. Whatever script is doing the update to this value may also impact the name and format of this file too.

I do think we need at least a notional plan for updating this value before we change header files. Because it is probably easier to implement if the tool (whatever it is) can update/regenerate the entire file, not just a single line or two within a file that also has other hand-maintained data.

[skliper]

@skliper, which tag do you prefer?

I don't have a preference as long as when combined with the code name it's unique (which it is if we follow our plan, so that doesn't help narrow it down at all). As long as we just pick one and don't change it then the logic never has to change (as long as the tag exists and we stick with git). Forced to choose I'd just pick the first release in each repo.

[astrogeco]

I was thinking we would reset the build number to zero whenever a new tag was out but maybe using the first release is a good idea since we would never have any aliasing. Then the codename is strictly a "branding" and compatibility tag while the build number keeps increasing to infinity.

[skliper]

I could be convinced either way (commit count for each cycle by referencing the last release or rc tag, or a fixed reference with increasing count), but a fixed tag means less changes to the script (although trivial) and doesn't depend on a fresh tag like the one I recently messed up in cFS.

[astrogeco]

I keep going back and forth on this, I think that it makes sense to think of the Build Name (and therefore the last released tag) as the epoch.

[jphickey]

I could also be convinced either way - to reset numbers to 1 with every release or to keep a constant epoch. I don't see major problems/advantages to one or the other.

BUT - with regards to naming - should we call this a "baseline" number rather than a "build" number? That is, I think a "build" implies production of binary files, and this is only source code. For each baseline there are currently at least 4 binary builds that are produced from it with different configs.

[jphickey]

And not to muddy the water even further, but we could also just do a ISO date code (YYYYMMDD) using the date of final merge to main/master. This is super-simple and VC system agnostic - but the downside is we couldn't produce more than one baseline a day. If you publish a baseline and find a bug post-publication, you must either wait until the next day to baseline a patch, or use some type of suffix.

[astrogeco]

Good point about the perils of using "buildname". I liked Jake's original nomenclature of "codename" though baseline is growing on me.

[jphickey]

Good point about the perils of using "buildname". I liked Jake's original nomenclature of "codename" though baseline is growing on me.

I interpret "Codename" is the name i.e. Aquila or Bootes ... that indicates the family or group that this belongs to.
Whereas the baseline number is the incrementing numeric value that gets changed on every merge to the mainline branch.

So we have both, a code name and a number, but I'm just questioning whether it should be called a "build" number (because we are referring to source, not binaries).

[astrogeco]

I think we can keep build number since we do actually try to ensure everything builds before adding it to the main branch. In theory there is an associated build (created by CI) for that source.

Looking more into nomenclature, I think it makes more sense to have BASELINE = "Bootes" and BUILDNUM = $commits_since_baseline. since the baseline is a "fixed reference point".

Definition of baseline:
A baseline is an intermediate status of your work results that you record/save and approve at certain points in time. It serves to provide a fixed reference point for change management.
From https://www.microtool.de/en/knowledge-base/what-is-a-baseline/

[jphickey]

Note that the BUILDTYPE variable also has meaning to CMake, see here: https://cmake.org/cmake/help/v3.0/variable/CMAKE_BUILD_TYPE.html. (The wrapper environment variable BUILDTYPE is mapped to CMAKE_BUILD_TYPE)

What exactly is the CFS_RELEASE_BUILD macro intended to be used for? In line with my previous comments on nasa/elf2cfetbl#50 this seems to be getting way too complex in code. Wouldn't this be dependent on the tag you pulled (and hence "fixed") and not dependent on the setting of an environment variable?

Either way I don't recommend setting a macro to true or false - the typical use case for boolean options is to define it or not define it, so #ifdef works as expected. If it is defined both ways, #ifdef CFE_RELEASE_BUILD evaluates true always.

Finally, if this really is necessary, we should confine it to the minimum scope. I'd hate to see #if CFE_RELEASE_BUILD appearing in apps.

[jphickey]

Elaborating a bit on what I brought up in CCB - One thing I think we really need to consider - The reason that the "GLOBAL_PSP_CONFIGDATA" exists deals with where version information actually resides inside the library files.

When setting a "Version number" as a #define macro inside a PSP header, then including that file from another entity (like CFE), the PSP version number actually gets compiled directly into the CFE Core library. In other words, the string/integers that contains the PSP version info is baked into the CFE library itself.

Most importantly - If one were to re-link with an updated PSP but not re-compile CFE, then you'd still be reporting the version of PSP that CFE was originally compiled against, not necessarily the one you are running with.

That is why the PSP uses this GLOBAL_PSP_CONFIGDATA structure - the data resides in the PSP library, not the CFE library. CFE actually reads it from PSP, which is how it should be - and if you update the PSP but not recompile CFE - it works.

[jphickey]

Although the current build script statically links the two and dependency rules stipulate that CFE is be recompiled whenever PSP headers change, that doesn't have to be the case. We could also keep these (CFE, PSP, OSAL) as separate shared libs which has some advantages. You really want the version info to be stored within the binary file (either .a or .so) that it is associated with, not compiled into a different file.

[astrogeco]

Although the current build script statically links the two and dependency rules stipulate that CFE is be recompiled whenever PSP headers change, that doesn't have to be the case. We could also keep these (CFE, PSP, OSAL) as separate shared libs which has some advantages. You really want the version info to be stored within the binary file (either .a or .so) that it is associated with, not compiled into a different file.

I like this idea. Honestly I think it would be better if the version report was gathered through a function instead of a macro.

Something like GetOSALVersion() which then returns the string.

Digging a bit deeper it looks like osapi-version.h is included into cfe by way of osapi.h

For PSP, cfe_psp.h (which I think should be renamed since naively I think that cfe_* should only be cfe-residing files) gets the psp_version.h information through the GLOBAL_PSP_CONFIGDATA. Which works but I think it can be streamlined if we really version the different PSPs individually or we just have a single version for our three "supported" PSPs.

[jphickey]

Right - in fact I would go so far as to say that getting the value via preprocessor macro is actually wrong and broken - I would avoid it! It is, by definition, evaluated when the unit is compiled - but its the wrong unit - not the unit to which the version is associated.

Getting the version number via global variable (e.g. GLOBAL_PSP_CONFIGDATA) or a runtime API (possibly better) doesn't have this problem.

The objective is that the PSP binary (libpsp.a/so) should hold the PSP version number, the CFE binary (libcfe-core.a/so) should hold the CFE version number, and so forth. If using preprocessor macros, the libcfe-core binary actually ends up holding all this data, which is the wrong place to keep it, and can be really confusing and cause incorrect version to be reported in some cases.

[astrogeco]

The more I learn about how the code works the more I realize that our pretty and clean layer diagram is more of an aspiration than reality :P

[jphickey]

This is actually one reason for the separate "target_config.c" file in cfe/cmake/target/src. It is the thing which is compiled to form the CFE executable, and contains references to GLOBAL_PSP_CONFIGDATA, GLOBAL_CFE_CONFIGDATA, etc causing all those units to be pulled in when linked as static libs.

This is why I recommend reading this variable to get the PSP version info, rather than using macros to get the data.

[jphickey]

The more I learn about how the code works the more I realize that our pretty and clean layer diagram is more of an aspiration than reality :P

True - the key to the "layer diagram" is to stick to APIs (functions) and types to define the interface. Preprocessor macros make things get fuzzy - macros are evaluated separately/independently for every source unit that uses it. So it becomes a "clone" than one data value that is referenced in all places. This distinction makes a big difference when trying to have a stable ABI (something I've long said that CFE doesn't do a good job of, currently -- too many dependencies on compile-time macros that define ABI elements).

Not meaning to digress - the main point being - get the version at runtime either through an API or the global config struct - don't rely on preprocessor macros.