Generate ginkgo.hpp via CMake
#1782
Conversation
There was a problem hiding this comment.
I like you did it in cmake!
I think we should add ginkgo.hpp into .gitignore in our repo.
However, it raises two concerns which I need to confirm
- when user pulls the change, will their old ginkgo.hpp they be stayed?
- If the file stays, will our include_header logics use source one or build one first?
Also, add a fatal error to detect whether user has the file in source
| @@ -30,10 +30,3 @@ repos: | |||
| examples/external-lib-interfacing/external-lib-interfacing.cpp| | |||
| core/base/workspace_aliases.hpp | |||
| )$ | |||
| - id: update-ginkgo-header | |||
There was a problem hiding this comment.
if we only aim for avoid pre-commit issue, we can move this to early stage such that they are checked after generation
There was a problem hiding this comment.
That would mean that the file gets changed twice, forcing all examples and other files that include ginkgo.hpp to be rebuilt
There was a problem hiding this comment.
To extend: I think as a general rule, different pre-commit steps shouldn't undo each other's changes.
There was a problem hiding this comment.
no, it will not. unless pre-commit -> build -> pre-commit -> build.
It will not undo anything if ginkgo.hpp.in is updated there too.
There was a problem hiding this comment.
I just checked, it's even worse than that: Regardless of order, if one hook undoes the change by another hook, both of them are marked as failed and prevent the changes from being committed
update ginkgo header.....................................................Failed
- hook id: update-ginkgo-header
- files were modified by this hook
reuse-annotate...........................................................Failed
- hook id: reuse-annotate
- files were modified by this hook
Successfully changed header of include/ginkgo/ginkgo.hpp
Successfully changed header of include/ginkgo/core/test.hpp
|
If users pull this change, their ginkgo.hpp will be deleted, so there is no danger of conflicting files (unless they changed ginkgo.hpp, then it's a merge conflict at rebase). |
| @@ -30,10 +30,3 @@ repos: | |||
| examples/external-lib-interfacing/external-lib-interfacing.cpp| | |||
| core/base/workspace_aliases.hpp | |||
| )$ | |||
| - id: update-ginkgo-header | |||
There was a problem hiding this comment.
no, it will not. unless pre-commit -> build -> pre-commit -> build.
It will not undo anything if ginkgo.hpp.in is updated there too.
probably not necessary, just being dilligent
|
Error: PR already merged! |
|
Error: PR already merged! |
Otherwise we always need to commit the header when adding new files. Additionally, it was being picked up by pre-commit, and the output of the script isn't properly formatted, so everything broke when adding a new header in #802
Closes #1781