LittleFS library linked when not used

[TD-er]

I've been browsing through my builds using the very nice and new Inspect tool from the (not yet released) PIO 4.1.0rc1 which allows me to browse through the binary and see all libs included and its size. (big, big kudo's for that tool!)

I noticed the LittleFS lib is linked even though I am not using it in my build (as far as I know)
Is this needed to be linked in when only using SPIFFS?
It does occupy about 20k in the build (compiled with debug, so release size my be less)

Apparently it is included here:

Arduino/libraries/ESP8266HTTPUpdateServer/src/ESP8266HTTPUpdateServer-impl.h

Line 8 in 3733ece

#include <LittleFS.h>

The same applies to ESP8266SdFat and SDFS. (not 100% sure that's not included by accident)

[TD-er]

Is it needed to have LittleFS included like this?
It does seem to be used only here:

Arduino/libraries/ESP8266HTTPUpdateServer/src/ESP8266HTTPUpdateServer-impl.h

Lines 95 to 102 in 3733ece

	if (upload.name == "filesystem") {
	size_t fsSize = ((size_t) &_FS_end - (size_t) &_FS_start);
	SPIFFS.end();
	LittleFS.end();
	if (!Update.begin(fsSize, U_FS)){//start with max available size
	if (_serial_output) Update.printError(Serial);
	}
	} else {

Wouldn't it make more sense to have a define stating which filesystem has to be included?
Not including LittleFS would make the 1M builds a lot more useful, since they are now too big to be able to perform a 2-step OTA flash and it is really hard to get them small enough.

[earlephilhower]

You've a good point. If you were using SPIFFS, would not want to include LittleFS code. And if using LittleFS then SPIFFS code (almost as big) is wasted, too.

A #define NOSPIFFSUPDATE or #define NOLITTLEFSUPDATE before including the HTTPUpdateServer.h file and a #ifndef NOSPIFFSUPDATE / SPIFFS.end() / #endif would do it, if slightly hacky

Alternative, probably 3.0 since it changes API, is to require the app to stop the FS itself before beginning an update process. So you would call FS.end() manually before httpupdate.begin().

[dirkmueller]

I thought the documentation says that anyway already...

[TD-er]

I thought the documentation says that anyway already...

Do you have a link to that?

[dirkmueller]

https://arduino-esp8266.readthedocs.io/en/latest/filesystem.html#end

[TD-er]

It does indeed state that the HTTPUpdateServer should only be called after stopping the filesystem.

Right now that update server does call the end on both filesystem types, which causes both to be included.
I do agree with @earlephilhower that it should not be the responsibility of this update server to end the filesystem processes, but that does mean a change in the interface of this update server.

If it was always meant to be called only with a running filesystem, then it is just a matter of removing these lines from that HTTPUpdateServer code.

Edit:
Changed the first sentence of my reply after reading again the docs link you provided. :)

[d-a-v]

Does it make sense, instead of calling SPIFFS.end() and LittleFS.end(), to call two weak empty functions that do nothing.
These two functions would be redefined each in their respective file (spiffs, littlefs) and this time do the right thing (call .end()).
The same way we disable WPS in https://github.com/esp8266/Arduino/blob/master/cores/esp8266/core_esp8266_app_entry_noextra4k.cpp#L13

[TD-er]

I guess that does make sense, although I do not yet see the implications of these kind of implementations with regard to these extern declarations. (in a .cpp file)
I do not fully understand how that code does what it does suggest it is doing, no matter what order the files are being compiled.

[earlephilhower]

@TD-er , give #6697 a try. I've manually tested the WebUpdater example with no SPIFFS/LittleFS.begin() in it and seen no FS code linked, then added SPIFFS/LittleFS.begin, rebuilt, and seen full code linked in as expected.

[TD-er]

Will try it later this evening.
Not before 22h Dutch time. (GMT + 1)