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

Add a “version” module to remove git dependency #269

Merged
merged 3 commits into from
Apr 9, 2019

Conversation

rafmudaf
Copy link
Collaborator

@rafmudaf rafmudaf commented Apr 8, 2019

Complete this sentence
THIS PULL REQUEST IS READY TO MERGE

Feature or improvement description
This pull request changes the versioning system as it was initially implemented in CMake. This does not affect the build for Windows with Visual Studio.

The existing implementation inserts the git hash (AKA the version) into the source code through a compiler flag and preprocessor directive in NWTC_IO. Therefore, anytime the git status changes the compile command also changes so everything has to be recompiled.

This pull request improves the build system

  1. Moving the git status checking to a standalone module
  2. Moving the compiler flag from the main CMakeLists to the new module CMakeLists

These changes mean that the compile command for the other modules does not change with the git commit and only the lightweight Version module needs to be recompiled. The driver programs which use this module are relinked during compile time.

Related issue, if one exists
#267

Impacted areas of the software
The build system through CMake

Additional supporting information
This is important when dealing with multiple branches as the git status changes frequently. Also, it will enable improved performance in the automated testing.

Test results, if applicable
Only the existing TravisCI tests are necessary.

This avoids rebuilding every source file when the git commit changes
@rafmudaf rafmudaf self-assigned this Apr 8, 2019
@rafmudaf rafmudaf requested a review from ebranlard April 8, 2019 21:02
@rafmudaf
Copy link
Collaborator Author

rafmudaf commented Apr 8, 2019

@bjonkman thank you for creating issue #267. I think this pull request addresses it as you've described. Could you please take a look?

@bjonkman
Copy link
Contributor

bjonkman commented Apr 9, 2019

I just briefly glanced through the changed files. Are you sure the Windows Visual Studio solution files will actually successfully build with these changes? I would be very surprised if they work without adding the new VersionInfo.f90 file to their list of source files.

@bjonkman
Copy link
Contributor

bjonkman commented Apr 9, 2019

@rafmudaf I just made a pull request to your branch to address the changes that need to happen in the Visual Studio builds.

Updated Visual Studio builds for Windows
@rafmudaf
Copy link
Collaborator Author

rafmudaf commented Apr 9, 2019

Thank you very much @bjonkman. After that bug fix, @ebranlard do you have any comments?

@rafmudaf
Copy link
Collaborator Author

rafmudaf commented Apr 9, 2019

By the way, the failed TravisCI check is due to the expired intel license. In another pull request, I'll remove that test from the Travis setup until we get a better solution.

@ebranlard
Copy link
Contributor

Everything looks good to me!

@ebranlard ebranlard merged commit ab84035 into OpenFAST:dev Apr 9, 2019
@rafmudaf rafmudaf deleted the feature/version_dependency branch April 9, 2019 19:36
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

Successfully merging this pull request may close these issues.

3 participants