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

Copy source

• 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

exntend version class to allow add info 8c6c8ad3c0

extend version handling with git describe 07155e2546

apply afmt 7e1aed6ad9

add fsfw specific cmake message prefix b951cb736a

include cmake-modules manually instead ... effecd4662

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

prefixed git info variable 513ae9dc10

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

git version handler more robust now 9c7eba4431

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

fix tests a569990ca2

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

update changelog 4079edc80e

minor form change 539e01deee

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

small addition and possible fix 750369b0a6

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

unique helper file name cccdced74d

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

maybe this fixed CI/CD issues 617d41c7d5

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

small remaining fix 85e849ca00

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

changelog typo 68231db9a1

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

add git CST and sha info to version ctor 18f9958332

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

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

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

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

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

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

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

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

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

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

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

added back fsfw namespace 80a5ed3c5b

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

Author

Owner

Copy link

Copy source

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

fixed tests 398d04dc50

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

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

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

order fix 736f8d0238

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

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

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

Owner

Copy link

Copy source

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

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

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

Author

Owner

Copy link

Copy source

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

experimenting with PRE_BUILD command dd986fefd3

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

added missing prefix efb3d982f3

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

update cmake-modules file 377c3325d2

muellerr added 1 commit 2022-05-10 12:26:31 +02:00

this appears to do the trick 88cc8c46a1

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

Author

Owner

Copy link

Copy source

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 added 1 commit 2022-05-10 12:28:48 +02:00

update changelog 58cb96ece0

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

Author

Owner

Copy link

Copy source

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

Copy source

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

Copy source

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 added 1 commit 2022-05-10 16:13:46 +02:00

remove custom command which reset .h.in file aa4e2d7209

muellerr added 1 commit 2022-05-10 16:43:23 +02:00

this compiles f57bd290d9

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

Author

Owner

Copy link

Copy source

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 added 1 commit 2022-05-10 16:46:13 +02:00

commit seems to change 5d946f5640

muellerr added 2 commits 2022-05-10 16:48:31 +02:00

removed faulty include f35c4049c0

update auto-gen file 1e569128c6

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

Author

Owner

Copy link

Copy source

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 added 2 commits 2022-05-10 16:53:43 +02:00

cmakelists update ac95e8f250

possible fix for cicd 1e5ba0212e

muellerr added 1 commit 2022-05-10 16:55:30 +02:00

update changelog 42ac1af4b2

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

some more fixes 7b767b238a

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

Copy source

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

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

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

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

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

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

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

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

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

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

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

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

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

Author

Owner

Copy link

Copy source

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

Documentation

Improvement of code documentation

Refactor

Contains a refactoring of some code

Tests

Issues related to testing code

bug

Something is not working

build

Issues related to the buildsystem, including CI

cosmetics

Small change most users won't notice

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

wontfix

This won't be fixed

No labels feature

Milestone

No items

No Milestone v5.0.0

Assignees

Clear assignees

ahinkel (Andy Hinkel) gaisser (Steffen Gaisser) jenkins_buildfix meierj (Jakob Meier) mohr (Ulrich Mohr) mspahr muellerr (Robin Müller) tbaumgartl (Tobias Baumgartl)

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fsfw/fsfw#601

Allow edits from maintainers