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

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

Open
5 of 6 tasks
atesin opened this issue Nov 23, 2022 · 2 comments
Open
5 of 6 tasks

Comments

@atesin
Copy link

atesin commented Nov 23, 2022

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 :

// Now try and remove any empty subdirs this makes, silently
char *pathStr = strdup(path);
if (pathStr) {
char *ptr = strrchr(pathStr, '/');
while (ptr) {
*ptr = 0;
lfs_remove(&_lfs, pathStr); // Don't care if fails if there are files left
ptr = strrchr(pathStr, '/');
}
free(pathStr);
}

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

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.
  • If there is a stack dump, I have decoded it.
  • I have filled out all fields below.

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)

@atesin atesin changed the title [request] option to make "remove(file)" NOT removes empty remaining directories also [request] option for "remove(file)" to disable auto removing empty remaining directories Nov 23, 2022
@atesin atesin changed the title [request] option for "remove(file)" to disable auto removing empty remaining directories [request] option for "LittleFS.remove(file)" to disable auto removing empty remaining directories Nov 23, 2022
@earlephilhower
Copy link
Collaborator

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

class LittleFSConfig : public FSConfig
{
public:
static constexpr uint32_t FSId = 0x4c495454;
LittleFSConfig(bool autoFormat = true) : FSConfig(FSId, autoFormat) { }
};

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
Copy link
Author

atesin commented Nov 23, 2022

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 :)

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

2 participants