fluffi
fluffi copied to clipboard
Explicitly list dependencies in makefile or use auto dependency generation
The present core/makefile
depends on shell tricks to list dependencies. This is error-prone.
Example: A change in core/LocalManager/LMDatabaseManager.h
may not trigger a rebuild of the LocalManager
binary. Specifically,
https://github.com/siemens/fluffi/blob/6fd185743822c9e48973a426cecd0081ac3e3e86/core/makefile#L168
leads to the var definition in
https://github.com/siemens/fluffi/blob/6fd185743822c9e48973a426cecd0081ac3e3e86/core/makefile#L73-L74
So we know the binary depends on LMDatabaseManager.o
because LMDatabaseManager.cpp
exists. However, the rule in
https://github.com/siemens/fluffi/blob/6fd185743822c9e48973a426cecd0081ac3e3e86/core/makefile#L164-L166
won't cause a rebuild of LMDatabaseManager.o
after LMDatabaseManager.h
has been changed because LMDatabaseManager.cpp
has indeed not been changed.
For this particular example (and some other similar cases we've seen), there can be some small and more-or-less systematic fixes due to the current inclusion structure. However, a correct long-term solution may call for either of the two ideas in the issue title (while accounting for both Linux and Windows toolchains).
P.S. I am filing this as a way to track the issue; we do not need any systematic fix in the short term. This bug was initially observed by @vinamrabhatia.
I am aware the current makefile has several shortcomings, including the header dependency issues you describe. It has, as many things have, grown over the course of the project to be too complex and confusing to maintain properly. We should mid to long term replace it. I intend to move the whole build to cmake in the future, but my time is currently limited. Let's keep this open as a feature to be implemented down the line.
Yes, we also think moving to cmake in the long term is a great idea. Internally, we are slowly growing our CMakeLists.txt; that was forced because we use CLion, which defines a project using CMakeList.txt, and we want auto-completion and definition look-up to work inside the IDE.
I think over time this effort may evolve into something that we can base the fix to this PR on. This is especially after seeing the bug above, which has convinced me there is something to fix. Let's wait a bit and see.
I just added the help wanted label. Any help migrating our current make file to cmake is welcomed :)