jkent / frogfs

Fast Read-Only General-purpose File System (ESP-IDF and ESP8266_RTOS_SDK compatible)
https://frogfs.readthedocs.io
Mozilla Public License 2.0
25 stars 32 forks source link

Cache created by mkfrogfs.py breaks 'idf.py clean' #61

Closed arex-ebee closed 2 months ago

arex-ebee commented 2 months ago

When a file system binary is created during CMake call target_add_frogfs() the mkfrogfs.py script is run. Besides creating a Python venv in functions.cmake the script also creates a directory <name>-cache within the build directory. Calling idf.py clean later on after a successful build results in a Ninja error:

Cleaning... ninja: error: remove(<name>-cache): Directory not empty
ninja: error: remove(CMakeFiles/venv): Directory not empty

As CMake doesn't know anything about the files created within the paths it does not generate according instructions for deleting them, hence the "known" directories can't be deleted as they're not empty.

The issue can be reproduced by a simple demo project:

idf.py create-project frogfs_test
cd frogfs_test
idf.py add-dependency jkent/frogfs=5.2.0
mkdir frogfs && echo foo > frogfs/bar.txt
echo -e "collect:\n  - \$cwd/frogfs/*" > frogfs.yaml
echo "target_add_frogfs(frogfs_test.elf)" >> CMakeLists.txt
idf.py build
...
idf.py clean

Right now I have no idea how to fix this issue (to be honest, CMake and I are not the best friends).

jkent commented 2 months ago

Thanks for pointing this out. I didn't test idf.py clean because I use the Visual Studio Code extension, which just deletes the entire build directory when performing a clean. The virtualenv and node_modules are also an issue. I'll have this fixed shortly.

jkent commented 2 months ago

Okay, I just pushed this as 8a4fb2c. Can you verify that it works for you before I make a new release? I tested it under Linux as both standalone and under IDF and it works for me.

arex-ebee commented 2 months ago

Many thanks for the quick response. I tested your commit and actually got some odd results:

My host machine runs a "well aged" Ubuntu 20.04 where CMake 3.16 is the most recent version from official sources there. In that configuration CMake seems to turn into an endless configuration step loop when performing the idf.py clean call with your update (even on a second idf.py build call!). In contrast, the build environment actually used for my project is a Debian 12 docker container where CMake 3.25.1 is running. There it only performs a single configuration step run during clean. Updating CMake on the Ubuntu host machine through Kitware's apt repo calms the issue.

Nevertheless I'm surprised about the configuration step at all even though 'clean' now seems to finish without error. Could this be caused the by if(NOT TARGET ...) checks of your commit?

jkent commented 2 months ago

Interesting.

The if(NOT TARGET ...) is just to prevent the target from being defined more than one time. I could have placed those targets outside of the macro, but that would have been a bit messy and I'd have to put some of the variables outside of the macro.

I think the issue might be a problem with the GLOB_RECURSE feature. Its supposed to find the globs changed (globbing is the only sane way to find all the files) and re-run CMake once to update that information (CMake is invoked from either Ninja or make). This is expected behavior.

I'm setting up a VM with bare minimum requirements, I'll report back after some more testing.

jkent commented 2 months ago

Well, I can't reproduce this with the makefile generator, but I can reproduce it with the Ninja generator. Ahha! It is a problem with the generated CMakeFiles/VerifyGlobs.cmake file not having the same policy as the build system. (You may have noticed the policy CMP0009 warning). Switching the GLOB_RECURSE to FOLLOW_SYMLINKS makes their behavior match. This was fixed in CMake 3.17. I also fixed Python 3.8 support, since it didn't support the root_dir argument to glob() until 3.10.

Would you kindly test the main branch again?

arex-ebee commented 2 months ago

There's nothing I'd rather do ;).

So in result, awesome! You even addressed the Python 3.8 problem I intended to mention in a different issue :thumbsup:. Although I don't really understand why a slight reconfiguration is performed by CMake during the 'idf.py clean' call, it now finishes successfully on both of my CMake/Python environments.

Many thanks for your great work!

jkent commented 2 months ago

Schweet. I'll make that release!