-
Notifications
You must be signed in to change notification settings - Fork 52
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
Version info #11
Comments
@koldunovn, thinking about it, I agree that doing it at runtime is also not the best idea. I also find that doing it at compile-time would be a nice idea (less things for the developers to remember when making a new release) If I understand correctly, you use CMake for building, and the actual CMake command is normally executed inside the besom repository. So, it could be possible to do something like this, assuming cmake can execute system commands: execute_process("echo 'SUBROUTINE Version'", OUTPUT_FILE /some/path.f90)
... repeat with other Fortran basics ...
execute_process("git describe") I'm not a Cmake expert, but something like that could work... To simply, it might even be easier to just have a dummy template file somewhere with a "UNKNOWN" tag, and then put in the git stuff with an sed command. In that way, you'd always get either a version number, or UNKOWN showing up in your logs... |
Hi guys, I think the build info should be compile time generated and displayed as a build in constant via fesom. Otherwise you could update the repo and have a newer commit sha being displayed without a rebuild. Also, some systems do not have a git installed on the compute nodes and this approach would fail. I can provide a compile time solution next week, if you like. |
What should I put in the info, the |
@hegish would be really nice if you could provide a solution! I think |
Hi all, I have pushed a branch which prints the compile time git SHA to the main repo. The SHA is determined on each rebuild, so it really reflects the git SHA from the build. I made sure everything still compiles if you do not use the cmake build for some reason, i.e. the fesom sources are still sufficient to build everything. |
@hegish What's the name of the branch? Can you link it here? |
Nevermind, find it in gitlab, will transpher it here. @hegish I will add you to GitHub, sorry for not doing it earlier. |
Ups, forgot the branch name: "version_info" |
FYI: I already opened a merge request in the main repo. |
Let's just try to do it here, if you don't mind :) |
No problem, should I push it here? |
I already did #13 |
@hegish Perfect, thanks a lot, Jan! Merged now. |
@hegish Ok, maybe I was too fast with merging it :) Now on juwels I have:
|
This error is probably related to a misconfigured linker or environment (i.e. it tries to link a library with the name FALSE). It has nothing to do with the changes of the branch. Are you sure the error did not occur in master before? |
Just tried it with |
@koldunovn, can you show an example of what is printed? We are adding a function in esm tools which will pull out version numbers from the stdout of the models if they exist, so I can add that of what we search for. |
@koldunovn, does the error also exist with c819797, i.e. master before the merge? |
@pgierz You can have a look here: https://github.com/FESOM/fesom2/runs/1116977046?check_suite_focus=true#step:6:10 |
Problem is magically solved by itself :) |
Discussion related to #10 PR
@pgierz thanks for such a fast reaction with a PR :)
I give it a deeper though and now think that retrieving the version number dynamically is not the best way to do it:
.git
folder is not present, there is no way we can get the info anyway.bin
folder in another fesom "installation". So version is something that should be inserted at the moment of compilation.So I see two possible options - somehow insert the git version at the process of compilation (in this case git should be available on any sane system), or just manually put the tag version. The second option has an advantage of being simpler to communicate and not having git as a dependency. But the first option is more precise and probably much more useful.
Would be nice to have @patrickscholz and @dsidoren opinions on this :)
The text was updated successfully, but these errors were encountered: