Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Add user-provided input variable to override fair_get_git_version #26

Open
dennisklein opened this issue Mar 15, 2022 · 5 comments

Comments

@dennisklein
Copy link
Member

See alisw/alidist#3945 for the use case.

@ChristianTackeGSI
Copy link
Member

I haven't yet looked in detail at the use case!

So just some general thoughts.

From our docs:

The function checks git attributes of the current tree. If it can’t detect anything useful (becausee Git is not installed, or because there is no git repository), it will not touch any output variables at all.

I think, our idea back then was: The user (FairLogger in this case) can detect that <prefix>_GIT_VERSION has not been touched and act accordingly. You could even set those variables beforehead with some "fallback value"?

(While reading the first paragraphas of the use case) Building from a tarball seems an interesting use case (you know I like that). But that one does not have a "GIT_VERSION", so fair_get_git_version (at least from its name) shouldn't be able to detect anything.

Maybe we could create some new fair_get_version which could have various tooling to detect versions?
Sources might include Git (TRY_GIT option calling into fair_get_git_version), Codemeta (TRY_CODEMETA, reading json from codemeta.json), Fallback (TRY_EXTERNAL?), etc?

@dennisklein
Copy link
Member Author

I think this feature is basically already supported. In addition to the logic we already document, we could more strongly define the output variable to be a CMake cache variable, which then gives user-overridability naturally out-of-the-box.

@ChristianTackeGSI
Copy link
Member

ChristianTackeGSI commented Mar 15, 2022

Okay, having read the "use case".

-DPROJECT_GIT_VERSION=$(echo $PKGVERSION | sed -e 's/v//')

is already well supported usage of fair_get_git_version.

We could add a note to our docs like this?

NOTE/Corollary: A project using fair_get_git_version can document their used <prefix> and can allow users of that project to provide "fallback" values by simple means of

cmake … -DYOURPREFIX_GIT_VERSION=3.1.4 …

The fallback obviously only works if your project is not using the REQUIRED option.
If your project wants to make this setability more visible by providing a cache variable, you could even do something like

set(YOURPREFIX_GIT_VERSION "${YOURPREFIX_GIT_VERSION}" CACHE STRING "This project's version")

after calling fair_get_git_version.
Whether all of this usage and its documenting is reasonable to your project is fully up to you.

WARNING: I haven't tested the set(CACHE) thing at all. And I don't think, fair_get_git_version is prepared to FORCE a detected version, if the variable is a CACHE value…

@ChristianTackeGSI
Copy link
Member

About cache value interaction:

macro(show title)
  message(STATUS "${title}")
  message(STATUS "  val: [${FOO}]")
  message(STATUS "  cache: [$CACHE{FOO}]")
endmacro()

function(something)
  set(FOO "other val" PARENT_SCOPE)
endfunction()

show("At Start")
something()
show("After Function Call")
set(FOO "${FOO}" CACHE STRING "My FOO")
show("After Marking as Cache")
$ cmake -P foo.cmake 
-- At Start
--   val: []
--   cache: []
-- After Function Call
--   val: [other val]
--   cache: []
-- After Marking as Cache
--   val: [other val]
--   cache: [other val]

$ cmake -DFOO=bar -P foo.cmake 
-- At Start
--   val: [bar]
--   cache: [bar]
-- After Function Call
--   val: [other val]
--   cache: [bar]
-- After Marking as Cache
--   val: [bar]
--   cache: [bar]

So "the project" has full control over which variant they want.

Projects just wanting a "fallback" do not need to do anything special. Just use ${YOURPREFIX_GIT_VERSION}, add some docs and be done.

And then there are are various simple snippet ideas (UNTESTED!):

  • Fallback with "cache marking":
    fair_get_git_version(…)
    set(YOURPREFIX_GIT_VERSION "${YOURPREFIX_GIT_VERSION}" CACHE STRING "Fallback Project Version" FORCE)
  • Projects wanting reasonable overriding:
    if (YOURPREFIX_GIT_VERSION)
      message(WARNING "Overriding version: ${YOURPREFIX_GIT_VERSION}")
    else()
      fair_get_git_version(…)
    endif()
  • Projects wanting full persistant cache style handling can do something like this (UNTESTED!):
    fair_get_git_version(…)
    set(YOURPREFIX_GIT_VERSION "${YOURPREFIX_GIT_VERSION}" CACHE STRING "Persistant Project Version")
    I would not recommend this, because it will have surprising effects for casual users.

@ChristianTackeGSI
Copy link
Member

New idea for adding docs:

NOTE: This function uses standard set(… PARENT_SCOPE) to set the output variables. As already noted, it only touches them, if it was able to discover the git version information. All standard CMake variable scoping and evaluation order princtiples apply, and can be utilized by your project.

Corollary: A project using straight fair_get_git_version can decide to document the used <prefix> and allow users of that project to provide "fallback" values by simple means of

cmake … -DYOURPREFIX_GIT_VERSION=3.1.4 …

The fallback obviously only works if your project is not using the REQUIRED option.

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

No branches or pull requests

2 participants