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

ROMFS not rebuilt correctly when ROMFS files are 'touch'ed #9093

Closed
ksschwabe opened this issue Mar 16, 2018 · 6 comments
Closed

ROMFS not rebuilt correctly when ROMFS files are 'touch'ed #9093

ksschwabe opened this issue Mar 16, 2018 · 6 comments

Comments

@ksschwabe
Copy link
Contributor

ksschwabe commented Mar 16, 2018

@dagar: related to discovery in PR: #9067.

If one of the files in ROMFS/px4_common/ (or whatever directory you specify as your ROMFS src directory) is touched, then the ROMFS is not rebuilt - it is rebuilt ONLY if an airframe is touched (or a file that starts with a number). This is because the add_custom_command in "ROMFS/CMakeLists.txt" does not depend on the contents of the ROMFS/px4_common/ directory. It only depends on the air frames (files that start with a number).

file(GLOB_RECURSE init_airframes ${PX4_SOURCE_DIR}/ROMFS/${config_romfs_root}/*/[1-9]*)
add_custom_command(OUTPUT ${romfs_temp_dir}/init.d/rcS ${romfs_temp_dir}/init.d/rc.autostart
COMMAND cmake -E copy_directory ${romfs_src_dir} ${romfs_temp_dir}
COMMAND ${PYTHON_EXECUTABLE} ${PX4_SOURCE_DIR}/Tools/px_process_airframes.py
-a ${romfs_temp_dir}/init.d
-s ${romfs_temp_dir}/init.d/rc.autostart
--board ${BOARD}
COMMAND ${PYTHON_EXECUTABLE} ${PX4_SOURCE_DIR}/Tools/px_romfs_pruner.py
--folder ${romfs_temp_dir} --board ${BOARD}
DEPENDS
${romfs_src_files}
${init_airframes}
${PX4_SOURCE_DIR}/ROMFS/${config_romfs_root}/init.d/rcS
${PX4_SOURCE_DIR}/Tools/px_process_airframes.py
)

Note: although, the add_custom_command() depends on ${romfs_src_files}, this variable is never set anywhere; it is an empty dependency. I suspect that ${romfs_src_files} was supposed be set with a file(GLOB_RECURSE ...), but this has just not been done.

Note: As it currently stands, if you make a change to any of the rc.* files other than rcS, then the ROMFS will not be rebuilt.

@ksschwabe
Copy link
Contributor Author

ksschwabe commented Mar 16, 2018

What we could do is remove the file(GLOB_RECURSE init_airframes ${PX4_SOURCE_DIR}/ROMFS/${config_romfs_root}/*/[1-9]*) line ROMFS/CMakeLists.txt and replace it with file(GLOB_RECURSE romfs_src_files ${romfs_src_dir} ${romfs_src_dir}/*).

We could then remove the dependence on ${init_airframes}, since it would be redundant. As far as I can see, its only purpose is to cause a rebuild of the ROMFS if an airframe file is touched.

Tell me if you think I should submit a PR for this.

@dagar
Copy link
Member

dagar commented Mar 16, 2018

I think we might need to explicitly list all the files. Globbing generally doesn't work very well in cmake. It will miss added files (you need to know how to manually kick off another cmake configure), and it's perhaps even more annoying when you remove a file that was previously globbed.

@dagar
Copy link
Member

dagar commented Mar 18, 2018

I'd be up for requiring all the ROMFS files to be explicitly listed in a CMakeLists.txt. With some simple directory hierarchy in place in won't be too overwhelming.

@bkueng
Copy link
Member

bkueng commented Mar 19, 2018

I'd be up for requiring all the ROMFS files to be explicitly listed in a CMakeLists.txt

Agreed. cmake recommends not to use GLOB and it will improve incremental builds.

@taileron
Copy link
Contributor

current master after flashing into fmu_V4 (vtol) all tuning values get lost or don´t load, probably the same source as mentioned above. Now I flashed back to master from 8 th of march that still was o.k.

@ksschwabe
Copy link
Contributor Author

Conversation continued here: #9107.

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

No branches or pull requests

4 participants