Extend version handling and class #601

Merged

muellerr merged 36 commits from eive/fsfw:mueller/extend-version-class into development 2022-05-16 14:52:15 +02:00

Conversation 11 Commits 36 Files Changed 27 +1326 -59

muellerr commented 2022-04-22 14:39:35 +02:00

Owner

Copy Link

• Add helper functions provided by cmake-modules manually now. Those should not change too often and only a small subset is needed
• Separate folder for easier update and for distinction
• LICENSE file included for cmake-modules files
• use int for version numbers to allow unset or uninitialized version
• Initialize Version object with numbers set to -1
• Instead of hardcoding the version, it is now retrieved with git describe
• Version now allows specifying additional version information like the git SHA1 hash and the versions since the last tag
• Additional information is set to the last part of the git describe output for FSFW_VERSION now.
• Version still need to be hand-updated if the FSFW is not included as a submodule for now.
• Add message prefix for FSFW CMake output

- Add helper functions provided by [`cmake-modules`](https://github.com/bilke/cmake-modules) manually now. Those should not change too often and only a small subset is needed - Separate folder for easier update and for distinction - LICENSE file included for cmake-modules files - use `int` for version numbers to allow unset or uninitialized version - Initialize Version object with numbers set to -1 - Instead of hardcoding the version, it is now retrieved with `git describe` - `Version` now allows specifying additional version information like the git SHA1 hash and the versions since the last tag - Additional information is set to the last part of the git describe output for `FSFW_VERSION` now. - Version still need to be hand-updated if the FSFW is not included as a submodule for now. - Add message prefix for FSFW CMake output

muellerr added 6 commits 2022-04-22 14:39:40 +02:00

8c6c8ad3c0

exntend version class to allow add info

07155e2546

extend version handling with git describe

7e1aed6ad9

apply afmt

b951cb736a

add fsfw specific cmake message prefix

effecd4662

include cmake-modules manually instead ...

- Instead of using FetchContent
- Separate folder for easier update and for distintion
- LICENSE file included

513ae9dc10

prefixed git info variable

muellerr added 1 commit 2022-04-22 14:47:39 +02:00

9c7eba4431

git version handler more robust now

muellerr added 1 commit 2022-04-22 14:50:56 +02:00

a569990ca2

fix tests

muellerr added 2 commits 2022-04-22 14:53:19 +02:00

4079edc80e

update changelog

539e01deee

minor form change

muellerr added 1 commit 2022-04-22 14:55:28 +02:00

750369b0a6

small addition and possible fix

muellerr added 1 commit 2022-04-22 15:00:11 +02:00

cccdced74d

unique helper file name

muellerr added 1 commit 2022-04-22 15:08:25 +02:00

617d41c7d5

maybe this fixed CI/CD issues

muellerr added 1 commit 2022-04-22 15:10:16 +02:00

85e849ca00

small remaining fix

muellerr added 1 commit 2022-04-22 15:37:23 +02:00

68231db9a1

changelog typo

muellerr added 1 commit 2022-04-22 15:39:57 +02:00

18f9958332

add git CST and sha info to version ctor

muellerr added the

feature

label 2022-04-22 15:40:33 +02:00

muellerr added this to the v5.0.0 milestone 2022-04-22 15:40:35 +02:00

muellerr requested review from gaisser 2022-04-22 15:40:39 +02:00

muellerr requested review from mohr 2022-04-22 15:40:46 +02:00

muellerr changed title from Extend version class to Extend version handling and class 2022-04-22 15:40:57 +02:00

muellerr referenced this pull request from eive/fsfw 2022-04-22 15:57:03 +02:00

Extend version class #67

muellerr referenced this pull request from eive/fsfw 2022-04-22 16:04:27 +02:00

mueller/extend-version-class #68

muellerr referenced this pull request from eive/fsfw 2022-04-22 16:05:44 +02:00

mueller/extend-version-class-fork #69

muellerr added 1 commit 2022-04-25 15:16:02 +02:00

4032228005

Merge remote-tracking branch 'upstream/development' into mueller/extend-version-class

muellerr added 1 commit 2022-04-25 15:23:52 +02:00

d62ee6a611 Merge remote-tracking branch 'upstream/development' into mueller/extend-version-class

muellerr added 1 commit 2022-04-27 08:46:22 +02:00

8e4ad10627 Merge remote-tracking branch 'upstream/development' into mueller/extend-version-class

muellerr added 1 commit 2022-05-02 14:48:29 +02:00

b62c19a364 Merge branch 'development' into mueller/extend-version-class

muellerr added 1 commit 2022-05-09 11:04:21 +02:00

5d5a355110 Merge remote-tracking branch 'upstream/development' into mueller/extend-version-class

muellerr added 1 commit 2022-05-09 11:06:53 +02:00

80a5ed3c5b

added back fsfw namespace

muellerr commented 2022-05-09 11:12:01 +02:00

Author

Owner

Copy Link

Update: Added back Version class to fsfw namespace. Its a generic name, so doing this can avoid nameclashes. I have seen that Catch2 has its own Version class which is
really similar.

Update: Added back `Version` class to `fsfw` namespace. Its a generic name, so doing this can avoid nameclashes. I have seen that Catch2 has its own Version class which is really similar.

muellerr added 1 commit 2022-05-09 11:15:25 +02:00

398d04dc50

fixed tests

muellerr added 1 commit 2022-05-09 15:48:01 +02:00

f1acf8e18b Merge remote-tracking branch 'upstream/development' into mueller/extend-version-class

muellerr added 1 commit 2022-05-09 15:50:46 +02:00

736f8d0238 order fix

muellerr added 1 commit 2022-05-09 22:34:13 +02:00

be6a492022 Merge branch 'development' into mueller/extend-version-class

mohr commented 2022-05-10 10:30:40 +02:00

Owner

Copy Link

So, I thought about this for a while.

I think an important point is to have the actual point in git history available in a produced binary for tracing faults during debugging.

On the other hand, I think relying on this for the version, which is used by downstream projects, seems a bit too ambitious for me. There seem to be some corner cases where it will not work. Also, a downstream project might not want to be sensitive to small changes which are version compliant breaking things. And lastly, it is kind of a circular reference to have the version information in the code be updated by metadata from the vcs in which it is stored (I think there is something problematic there waiting, which I can not put into words yet).

So my proposal would be the following:

• Keep the exposed (via cmake) version manually defined during releases
• Pull in this version into the binary as already done
• Replace the FSFW_VERSION_CST_GIT_SHA1 or as it is already called in code addInfo with a vcs_info (aka version control system) which is the output of git describe at the time of compilation.

To facilitate having the right git output in every build, I suggest using add_custom_command as a PRE_BUILD step which then calls git describe and puts the output into an include header which is then used by version.cpp. As far as I can understand from the documentation this should be called whenever the actual target, the fsfw, is built and as such should always be in sync with the actual git state.
There needs to be a check if git is present on the system as well as if the code is in a git repo, but I guess/hope cmake has some provisions for that.

So, I thought about this for a while. I think an important point is to have the actual point in git history available in a produced binary for tracing faults during debugging. On the other hand, I think relying on this for the version, which is used by downstream projects, seems a bit too ambitious for me. There seem to be some corner cases where it will not work. Also, a downstream project might not want to be sensitive to small changes which are version compliant breaking things. And lastly, it is kind of a circular reference to have the version information in the code be updated by metadata from the vcs in which it is stored (I think there is something problematic there waiting, which I can not put into words yet). So my proposal would be the following: * Keep the exposed (via cmake) version manually defined during releases * Pull in this version into the binary as already done * Replace the `FSFW_VERSION_CST_GIT_SHA1` or as it is already called in code `addInfo` with a `vcs_info` (aka version control system) which is the output of `git describe` at the time of compilation. To facilitate having the right git output in every build, I suggest using `add_custom_command` as a `PRE_BUILD` step which then calls `git describe` and puts the output into an include header which is then used by `version.cpp`. As far as I can understand from the documentation this should be called whenever the actual target, the fsfw, is built and as such should always be in sync with the actual git state. There needs to be a check if git is present on the system as well as if the code is in a git repo, but I guess/hope cmake has some provisions for that.

muellerr added 1 commit 2022-05-10 11:19:54 +02:00

b38329aa0e Merge branch 'development' into mueller/extend-version-class

muellerr commented 2022-05-10 11:32:07 +02:00

Author

Owner

Copy Link

Concerning the point that the version should still be defined manually: There is still a way to do this in the CMakeLists.txt . Currently ,it falls back to that way if retrieving with git fails.

The used CMake module already has internal checks like checking that git is installed. If we do not use this, we need add a lot of custom code, because the CMake module already handled these cases.

See the function

function(git_describe _var)
    if(NOT GIT_FOUND)
        find_package(Git QUIET)
    endif()
    get_git_head_revision(refspec hash)
    if(NOT GIT_FOUND)
        set(${_var} "GIT-NOTFOUND" PARENT_SCOPE)
        return()
    endif()
    if(NOT hash)
        set(${_var} "HEAD-HASH-NOTFOUND" PARENT_SCOPE)
        return()
    endif()

    # TODO sanitize
    #if((${ARGN}" MATCHES "&&") OR
    #    (ARGN MATCHES "||") OR
    #    (ARGN MATCHES "\\;"))
    #    message("Please report the following error to the project!")
    #    message(FATAL_ERROR "Looks like someone's doing something nefarious with git_describe! Passed arguments ${ARGN}")
    #endif()

    #message(STATUS "Arguments to execute_process: ${ARGN}")

    execute_process(COMMAND
        ${GIT_EXECUTABLE}
        describe
        ${hash}
        ${ARGN}
        WORKING_DIRECTORY
        "${CMAKE_SOURCE_DIR}"
        RESULT_VARIABLE
        res
        OUTPUT_VARIABLE
        out
        ERROR_QUIET
        OUTPUT_STRIP_TRAILING_WHITESPACE)
    if(NOT res EQUAL 0)
        set(out "${out}-${res}-NOTFOUND")
    endif()

    set(${_var} "${out}" PARENT_SCOPE)
endfunction()

I don't think it is possible to use CMake functions in a PRE_BUILD command simply like this. But I can look at this, see if its maybe possible.

About the suggestion to output this into a header file: This then needs to be a valid header file, and valid C++ code. This would be best done as a configure fule. As is already done right now. The only difference is that it would be done as a pre-build command, which would re-trigger it every build instead of requiring resetting the CMake cache.

Concerning the point that the version should still be defined manually: There is still a way to do this in the `CMakeLists.txt` . Currently ,it falls back to that way if retrieving with git fails. The used CMake module already has internal checks like checking that git is installed. If we do not use this, we need add a lot of custom code, because the CMake module already handled these cases. See the function ```cmake function(git_describe _var) if(NOT GIT_FOUND) find_package(Git QUIET) endif() get_git_head_revision(refspec hash) if(NOT GIT_FOUND) set(${_var} "GIT-NOTFOUND" PARENT_SCOPE) return() endif() if(NOT hash) set(${_var} "HEAD-HASH-NOTFOUND" PARENT_SCOPE) return() endif() # TODO sanitize #if((${ARGN}" MATCHES "&&") OR # (ARGN MATCHES "||") OR # (ARGN MATCHES "\\;")) # message("Please report the following error to the project!") # message(FATAL_ERROR "Looks like someone's doing something nefarious with git_describe! Passed arguments ${ARGN}") #endif() #message(STATUS "Arguments to execute_process: ${ARGN}") execute_process(COMMAND ${GIT_EXECUTABLE} describe ${hash} ${ARGN} WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}" RESULT_VARIABLE res OUTPUT_VARIABLE out ERROR_QUIET OUTPUT_STRIP_TRAILING_WHITESPACE) if(NOT res EQUAL 0) set(out "${out}-${res}-NOTFOUND") endif() set(${_var} "${out}" PARENT_SCOPE) endfunction() ``` I don't think it is possible to use CMake functions in a PRE_BUILD command simply like this. But I can look at this, see if its maybe possible. About the suggestion to output this into a header file: This then needs to be a valid header file, and valid C++ code. This would be best done as a configure fule. As is already done right now. The only difference is that it would be done as a pre-build command, which would re-trigger it every build instead of requiring resetting the CMake cache.

muellerr added 1 commit 2022-05-10 11:51:39 +02:00

dd986fefd3

experimenting with PRE_BUILD command

muellerr added 1 commit 2022-05-10 11:52:48 +02:00

efb3d982f3

added missing prefix

muellerr added 1 commit 2022-05-10 12:16:47 +02:00

377c3325d2

update cmake-modules file

muellerr commented 2022-05-10 12:26:56 +02:00

Author

Owner

Copy Link

Alright after some more testing I think I managed to create a solution for some issues.

• Update some cmake modules. The Git helpers from the bilke repo were obsolete, add the ones from the https://github.com/rpavlik/cmake-modules repository

The following PRE_BUILD command

# This causes the version file to be updated each time the sources change
add_custom_command(
    TARGET ${LIB_FSFW_NAME}
    PRE_BUILD
    COMMAND ${CMAKE_COMMAND} -E touch ${FSFW_SOURCES_DIR}/FSFWVersion.h.in
    COMMENT "${MSG_PREFIX} Updating FSFWVersion.h"
)

appears to re-trigger the version handling every time the FSFW sources change. I'm not fully sure how this works, but I guess re-touching a configure file causes CMake to execute the CMake code again which can set values in the input files. I still need to check whether the FSFWConfig.h.in is reset every time, because that would be bad.

Alright after some more testing I think I managed to create a solution for some issues. - Update some cmake modules. The Git helpers from the `bilke` repo were obsolete, add the ones from the https://github.com/rpavlik/cmake-modules repository The following PRE_BUILD command ```cmake # This causes the version file to be updated each time the sources change add_custom_command( TARGET ${LIB_FSFW_NAME} PRE_BUILD COMMAND ${CMAKE_COMMAND} -E touch ${FSFW_SOURCES_DIR}/FSFWVersion.h.in COMMENT "${MSG_PREFIX} Updating FSFWVersion.h" ) ``` appears to re-trigger the version handling every time the FSFW sources change. I'm not fully sure how this works, but I guess re-touching a configure file causes CMake to execute the CMake code again which can set values in the input files. I still need to check whether the `FSFWConfig.h.in` is reset every time, because that would be bad.

muellerr commented 2022-05-10 12:36:24 +02:00

Author

Owner

Copy Link

Alright, I think the other input files are reset as well.. I think another solution is required here..

Alright, I think the other input files are reset as well.. I think another solution is required here..

muellerr commented 2022-05-10 12:46:18 +02:00

Author

Owner

Copy Link

This might be a better solution: https://www.mattkeeter.com/blog/2018-01-06-versioning/

This might be a better solution: https://www.mattkeeter.com/blog/2018-01-06-versioning/

muellerr commented 2022-05-10 16:03:14 +02:00

Author

Owner

Copy Link

I am not happy at all with that soluton.. It is hacky and it does not work properly right now. I have a feeling the configure_file way is the least ulgy solution.

I am not happy at all with that soluton.. It is hacky and it does not work properly right now. I have a feeling the configure_file way is the least ulgy solution.

muellerr commented 2022-05-10 16:44:07 +02:00

Author

Owner

Copy Link

Managed to make it work by using a target command instead. Performing some more tests.

Managed to make it work by using a target command instead. Performing some more tests.

muellerr commented 2022-05-10 16:49:58 +02:00

Author

Owner

Copy Link

I think this is a working and possible solution. This will re-generated the auto-gen file every time source code changes and the commit tag changed. I will also update the CHANGELOG

I think this is a working and possible solution. This will re-generated the auto-gen file every time source code changes and the commit tag changed. I will also update the CHANGELOG

muellerr force-pushed mueller/extend-version-class from da44045377 to 377c3325d2 2022-05-11 20:14:53 +02:00 Compare

muellerr commented 2022-05-11 20:20:25 +02:00

Author

Owner

Copy Link

I have decided to go back to the previous solution using a configure file. This requires re-building th CMake cache, but I was not happy with the other solution for the following reasons

• Extremely (!) hacky solution. Everybody trying to figure out what is happening in the CMakeLists.txt file is going to have a hard time
• It was not possible to only re-generate the file only when it changes. CMake behaves very oddly when you leave the well-trodden path
• This means the version file is always re-compiled, and all link steps are re-done for every compilation process
• Other projects could re-use the version handling. I'd prefer if they follow the old way instead of taking a hacky solution which is way off the well-trodden path

At the end of the day, a fallback solution is still required so someone still needs to manually update the version number in the CMakeLists.txt. Build servers always seem to have issues using git tags (did not work on Github, does not appear to work with Jenkins either), but I like the new feature that the VSC information is written into a header file , even if a cache reset is necessary to get update it.

The hacky solution can still be found in the mueller/extend-version-class-ugly-cmake-hack branch

I have decided to go back to the previous solution using a configure file. This requires re-building th CMake cache, but I was not happy with the other solution for the following reasons - Extremely (!) hacky solution. Everybody trying to figure out what is happening in the `CMakeLists.txt` file is going to have a hard time - It was not possible to only re-generate the file only when it changes. CMake behaves very oddly when you leave the well-trodden path - This means the version file is always re-compiled, and all link steps are re-done for every compilation process - Other projects could re-use the version handling. I'd prefer if they follow the old way instead of taking a hacky solution which is way off the well-trodden path At the end of the day, a fallback solution is still required so someone still needs to manually update the version number in the `CMakeLists.txt`. Build servers always seem to have issues using git tags (did not work on Github, does not appear to work with Jenkins either), but I like the new feature that the VSC information is written into a header file , even if a cache reset is necessary to get update it. The hacky solution can still be found in the `mueller/extend-version-class-ugly-cmake-hack` branch

muellerr added 1 commit 2022-05-12 17:12:40 +02:00

e77bde459b

Merge remote-tracking branch 'upstream/development' into mueller/extend-version-class

muellerr added 1 commit 2022-05-13 11:29:16 +02:00

271057ca6b

Merge remote-tracking branch 'upstream/development' into mueller/extend-version-class

muellerr added 1 commit 2022-05-13 13:21:09 +02:00

970f039e85

Merge remote-tracking branch 'origin/development' into mueller/extend-version-class

muellerr added 1 commit 2022-05-13 17:23:03 +02:00

5736023ffa Merge branch 'development' into mueller/extend-version-class

muellerr added 1 commit 2022-05-16 14:16:58 +02:00

4821706561 Merge branch 'development' into mueller/extend-version-class

muellerr added 1 commit 2022-05-16 14:42:28 +02:00

ef9d7aa7d3 Merge branch 'development' into mueller/extend-version-class

muellerr commented 2022-05-16 14:51:53 +02:00

Author

Owner

Copy Link

Approved by @mohr

Approved by @mohr

muellerr merged commit f0debecbbc into development 2022-05-16 14:52:15 +02:00

muellerr referenced this issue from a commit 2022-05-16 14:52:15 +02:00

Merge pull request 'Extend version handling and class' (#601) from eive/fsfw:mueller/extend-version-class into development

muellerr deleted branch mueller/extend-version-class 2022-05-16 14:52:17 +02:00

Sign in to join this conversation.

Reviewers

No reviewers

gaisser

mohr

Labels

Clear labels   

API Change

Changes the API, so Users need to adapted

  

Breaking API Change

Changes the API in a non backwards compatible way

  

bug

Something is not working

  

build

Issues related to the buildsystem, including CI

  

cosmetics

Small change most users won't notice

  

Documentation

Improvement of code documentation

  

duplicate

This issue or pull request already exists

  

feature

New feature

  

help wanted

Need some help

  

hotfix

Quick or important fix that is merged directly into master, causing a new revision

  

invalid

#worksforme

  

question

More information is needed

  

Refactor

Contains a refactoring of some code

  

Tests

Issues related to testing code

  

wontfix

This won't be fixed

No Label

API Change

Breaking API Change

bug

build

cosmetics

Documentation

duplicate

feature

help wanted

hotfix

invalid

question

Refactor

Tests

wontfix

Milestone

Clear milestone

No items

No Milestone

v5.0.0

Assignees

Clear assignees

No Assignees

2 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: fsfw/fsfw#601

No description provided.