esp8266 / Arduino

ESP8266 core for Arduino
GNU Lesser General Public License v2.1
16.09k stars 13.33k forks source link

[request] option for "LittleFS.remove(file)" to disable auto removing empty remaining directories #8724

Open atesin opened 2 years ago

atesin commented 2 years ago

Problem Description (infos below)

in LittleFS, if i have a subdirectory with a single file and i remove it, the directory goes empty and it is ALSO removed, and ALL the empty parent directories back, RECURSIVELY... i know this is the standard behavior but is not always desirable, so searching a way to disable it by inspecting the source code (for some boolean argument, config object or something) i noticed there are none

i would like an official and everyone-available option (because i surely can modify my local source, but i don't feel is the shared spirit of opensource) to disable this auto-erase empty directories feature via a function argument, config or so... i think the fix will be easy and could have multiple options, currently i don't see any way to prevent executing this but hardcoding my source... the related source code is : https://github.com/esp8266/Arduino/blob/80bf71662551fb5e6579713dc15e541ac48ccf98/libraries/LittleFS/src/LittleFS.h#L151-L161

to ilustrate a situation when this could be annoying, imagine we have a sketch with a bash-like command line interface

/current/dir/>      # as FS doesn't have the "current dir" concept, we maintain it in a global char[] variable
/current/dir/> ls    # wrapper for LittleFS.openDir(currentDir) iteration
size file
0    myFile
- 1 object
/current/dir/> remove myFile   # wrapper for remove(myFile)... but it will also remove the empty parent dirs after! (getting slower)
/current/dir/> ls       # as the current dir was just deleted, a check with LittleFS.exists(currentDir) will generate an error!!
Error: path doesn't exist
/current/dir/> cd ..    # back to parent dir, just slicing currentDir global var
/current/> ls    # note subdir "dir" now doesn't exists as last seen, what is confusing.... 
size file
x    blah
- (any) object(s)
/current/>    # additionally, if this current dir was also emptied and deleted, "ls" would also generate another error!!

Basic Infos

Platform

Hardware: ESP-12E
Core Version: 3.0.2
Development Env: Arduino IDE 1.9
Operating System: Windows

Settings in IDE

Module: NodeMCU 1.0 amica v2 compatible
Flash Mode: qio, i guess (not relevant for this)
Flash Size: 4MB
lwip Variant: v2 Lower Memory
Reset Method: nodemcu
Flash Frequency: 40Mhz
CPU Frequency: 80Mhz
Upload Using: SERIAL
Upload Speed: 115200

MCVE Sketch

this is a general case about a lib function, this function will behave the same in any sketch

Debug Messages

i haven't enabled debugging (and i tried but i couldn't.. not relevant anyway)

earlephilhower commented 2 years ago

It was done this way to better match the way things worked with SPIFFS, but it is kind of odd when compared to normal POSIX-type behavior.

This would be a simple option to add to https://github.com/esp8266/Arduino/blob/80bf71662551fb5e6579713dc15e541ac48ccf98/libraries/LittleFS/src/LittleFS.h#L47-L52

Just add another call noDirectoryRemove or something, a class variable (make sure to init to false by default), and then check the configuration and skip 151-161 if it's true...

atesin commented 2 years ago

It was done this way to better match the way things worked with SPIFFS, but it is kind of odd when compared to normal POSIX-type behavior.

not only compared to posix-type behavior... we are not always aware of contents of our filesystems, nor keep a mental count of files in each folder to know which folder will be auto-deleted or which won't... so somebody could consider this "extra" action as unexpected and annoying.... i think the correct behavior of any app should be to avoid unaware surprises and do just what we tell to do (just do one thing, but do it well)

i think another simple way to doing this optional, while keeping backward compatibility, could be, for example, to modify the function signature by adding an optional boolean parameter (line 142)

    bool remove(const char* path, bool wipeEmptyParentDirs = true) override {

and then insert 2 lines to return the function earlier, skipping the delete-directories code (in line 151)

        if (!wipeEmptyParentDirs)
            return true;

..... or... maybe this doesn't work because the override keyword o_Ô ?? (i am not so skilled in C)

but as i don't need this functionality and even obstructs me, in my source i simply commented out all the wipe-directory code block :)