advancedtelematic / aktualizr

C++ SOTA Client
Mozilla Public License 2.0
144 stars 60 forks source link

Sql_schemas.cc being removed before it can be compared in embed_schemas.py #1815

Closed pongtato closed 2 years ago

pongtato commented 2 years ago

Hi!

With reference to the CMakeLists.txt that contains:

add_custom_command(OUTPUT sql_schemas.cc sql_schemas_target
    COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/embed_schemas.py ${PROJECT_SOURCE_DIR}/config/sql/ ${CMAKE_CURRENT_BINARY_DIR}/sql_schemas.cc libaktualizr
    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
)

I am on CMake version 3.16.3, and add_custom_command removes sql_schemas.cc, resulting in the following exception being thrown in embed_schemas.py:

[Errno 2] No such file or directory

The expected behavior for embed_schemas.py is understood to only edit the file if there was a content mismatch. However, in this case since the file does not exist, a new sql_schemas.cc would always be written to, triggering the building and linking process.

As a workaround, I made the following changes, but would like some feedback if this might lead to unexpected behaviors, or if there are just better ways to do this.

add_custom_target(sql_schemas_target ALL
    COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/embed_schemas.py ${PROJECT_SOURCE_DIR}/config/sql/ ${CMAKE_CURRENT_BINARY_DIR}/sql_schemas.cc libaktualizr
    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
)
...
add_library(storage OBJECT ${SOURCES} sql_schemas.cc)
add_dependencies(storage sql_schemas_target)

Thank you!

pattivacek commented 2 years ago

The expected behavior for embed_schemas.py is understood to only edit the file if there was a content mismatch.

Yes, the idea is that it shouldn't need to to be regenerated if the schemas haven't changed. However, I doubt anyone's looked closely at this in a while. I think @lbonn wrote that logic, perhaps he recalls something about this?

However, in this case since the file does not exist, a new sql_schemas.cc would always be written to, triggering the building and linking process.

Something seems wrong here. Why is the file being removed?

As a workaround, I made the following changes, but would like some feedback if this might lead to unexpected behaviors, or if there are just better ways to do this.

Looks okay at first glance, but I don't understand what makes it any different.

FYI community development of aktualizr has moved to the Uptane namespace: https://github.com/uptane/aktualizr/.

pongtato commented 2 years ago

Something seems wrong here. Why is the file being removed?

It might be something on CMake's end. I tested this behavior on a separate project, doing something as simple as:

add_custom_command(OUTPUT main.cpp
    COMMAND echo "Hello"
)
add_executable(Tutorial main.cpp)

causes my main.cpp to be removed. I just wanted to see if this was a known issue, or an issue only on my end.

Looks okay at first glance, but I don't understand what makes it any different.

I'm not entirely sure why this works, since I can't see much of a difference as well...

FYI community development of aktualizr has moved to the Uptane namespace: https://github.com/uptane/aktualizr/.

Oops, thanks for letting me know. I'll make future posts there if need be.

pattivacek commented 2 years ago

causes my main.cpp to be removed

I tried to reproduce this both with your example and sql_schemas.cc in libaktualizr. I don't see the same behavior. I have cmake version 3.22.1, but I know I used to have 3.16 at one time. The minimum version is still 3.5 (although I can't guarantee that's accurate) and when this project was started we were still using 2.x. I don't think it's related to your version.

pongtato commented 2 years ago

Hmmm. Thanks for testing! I'll make a clean installation of everything and see how it goes.

Edit* - I've tried a clean installation and even updated my version of CMake. Unfortunately, It only solved the behavior in my test project. I will just make do with this fix for now.