Lists ROMFS files explicity instead of using GLOB_RECURSE

[ksschwabe]

Fixes #9093.

Previously, when ROMFS files that were not airframes were touched, the ROMFS
would not be rebuilt. The ROMFS files are now specified explicityl in a
CMakeLists.txt file that is located in the root ROMFS directory.

Now when one of the ROMFS files is touched the whole ROMFS is rebuilt.

When new files are added to the ROMFS, they need to be explicity added to
the CMakeLists in the ROMFS root directory.

[dagar]

What do you think about including the CMakeLists.txt in the deepest directory? At each higher level back to the root you can have a CMakeLists.txt that simply adds each directory (add_subdirectory(init.d)).

I say this now because I'd like to come back and sort the airframes and mixers by group later.

[ksschwabe]

@dagar: Do you not think that might get a bit too confusing? Too many layers and levels of CMakeLists.txt? Especially when each of these CMakeLists will only be setting/appending to the lists and not actually creating targets/outputs. It is only the ROMFS/CMakeLists.txt that is currently creating any output.

I guess, it depends on how often you think you might be modifying the ROMFS files.

[dagar]

Personally I find lists and directories with ~ 100 things overwhelming and hard to sift through.

The only way I know to get a handle on that complexity is to break it down and keep it modular. I started doing this last year, but never got it merged. #7628
In the process I came across a number of effectively duplicated and obsolete mixers and airframes.

Either way is fine for now, but this is what I'd like to consider doing later.

[ksschwabe]

@dagar: I can take a crack at adding in CMakeLists.txt files in each subdirectory.

How do you want me to add the files in the deeper directories to the "master" list in the upper directory, though? In each subdirectory, do a

APPEND(config_romfs_files_list ...<list of new files>)

set(config_romfs_files_list ${config_romfs_files_list} PARENT_SCOPE)

?
I hope that won't be too confusing. If you add a new directory and forget to add the parent scope, or you do a SET instead of an APPEND, then you'll mess up your ROMFS build. (But I guess that should be ok...)

[dagar]

In the top level ROMFS CMakeLists.txt you set(config_romfs_files_list).

Then in each subdirectory you append

list(APPEND config_romfs_files_list
    AAERTWF.main.mix
    AAVVTWFF.main.mix
    AERT.main.mix
    AETRFG.main.mix
    bebop.main.mix
    blade130.main.mix
    ...
)

The other thing that just occurred to me is the exclude mechanism. We need to make sure that still works (available flash is currently quite limited on the original 1MB pixhawk). https:/PX4/Firmware/blob/master/ROMFS/px4fmu_common/init.d/13004_quad%2B_tailsitter#L19

[ksschwabe]

This is becoming messy. Each level of CMakeLists adds a CMakeLists directory and .cmake file in the temp romfs directory. I I would have to prune these out, or we need to find a better solution to prevent CMake files from littering the temp ROMFS directory. - I think the px4fmu-v2 build overflowed because these additional cmake build files were copied into the ROMFS.

[dagar]

If they're ending up in the tmp ROMFS at all I think you should change the path to avoid the conflict entirely. The location for ROMFS in the build directory is arbitrary, and placed at the top level for minor convenience. Unless it's trivial to tell genromfs what to exclude (or only include) I'd simply change the path.

[ksschwabe]

@dagar: I can't quite work out how to get CMake not to generate the cmake_install.cmake file and CMakeFiles directory in the romfs. You'll see that I tried to use a separate temp romfs directory with #set(romfs_temp_dir ${PX4_BINARY_DIR}/ROMFS/romfs), but it still generates the cmake files in this new temp directory.

I'd rather like to not have to specifically prune those files or exclude them manually in the genromfs program if possible, because that will make the ROMFS build all the more obscure.

Any ideas? Sorry to be a pain.... :-(

[ksschwabe]

It looks like whenever I have an add_subdirectory call, a cmake_install.cmake file and a CMakeFiles directory are created in the "subdirectory location" in the binary directory.

[dagar]

This is all internal cmake stuff, so let's not try to prune. All we need to do is pick a folder in the build directory that doesn't overlap with the cmake tmp (corresponding to source tree) at all.

Something like ${PX4_BINARY_DIR}/genromfs would be fine.

[ksschwabe]

@dagar: Setting the romfs_temp_dir to romfs_temp_dir ${PX4_BINARY_DIR}/ROMFS/genromfs helped to avoid having the cmake_install files included in the genromfs directory. Thanks! 👍

I have had to add set(config_romfs_files_list ${config_romfs_files_list} PARENT_SCOPE) to the bottom of each subdirectory CMakeLists file to make sure that the global list is correctly updated.

You will also see that in each of the subdirectory CMakeLists, I have had to add the ${CMAKE_CURRENT_LIST_DIR}/ path to each filename, otherwise I am unable to correctly build the list of all files uponwhich the ROMFS depends. If you have a more elegant solution to the problem of generating this list, let me know.

[dagar]

How about something like this this? ksschwabe#1

[dagar]

Looking good. To be paranoid let's check the romfs.txt (search the build dir) is identical in this PR and master. I'd also check that booting a pixhawk actually works (including IO flash).

[ksschwabe]

I've fixed the path issue. The ROMFS files are now generated in the temp directory <build>/genromfs/${config_romfs_root}/. I have also added a brief explanatory comment to the px4_add_romfs_files function.

I see two "issues"/problems still to discuss.

1. The copying of files is still determined ONLY by the $config_romfs_root variable:
   COMMAND cmake -E copy_directory ${romfs_src_dir} ${romfs_temp_dir}
   The px4_add_romfs_files function only adds to a list that determines the dependencies for when this directory copy function is called. Are we happy with this? We aren't actually using the config_romfs_files_list variable to perform the copy. It seems a bit confusing.

2. Maybe we should put the px4_add_romfs_files function into cmake/common/px4_base.cmake so that it is in the same place as all the other cmake function definitions. Although if we do that, we sort of lose "transparency" on where the config_romfs_files_list comes from.

[dagar]

1. Yes, and unfortunately we don't have visibility into the actual output files. The python script that does this may exclude things based on board.

2. I'm not sure what proper cmake convention is for functions, but you could move it to another file if appropriate. If it makes sense I'd prefer to keep everything to do with ROMFS generation together. From a PX4 perspective this keeps it as modular and simple as possible.

[ksschwabe]

2. Let's keep the px4_add_romfs_files function in the ROMFS/CMakeLists, then. I think it will be clearer from a "readability" standpoint.

[dagar]

Wrong copyright year.

[ksschwabe]

I have updated the Copyright header with a 2018 header.

I also added a 2018 copyright header to the ROMFS/CMakeLists.txt file - it previously didn't have one at all. Should the one for the ROMFS/CMakeLists.txt be a purely 2018 copyright header, or should we make it a 20XX - 2018 header?

[dagar]

2018 only is fine for the new ones.

[dagar]

I verified the ROMFS contents are identical vs master and did some actual testing on a pixhawk.

[dagar]

Thank you!