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

Bump littlefs version #19

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Bump littlefs version #19

wants to merge 2 commits into from

Conversation

sosthene-nitrokey
Copy link
Contributor

This patch:

  • Bumps the littlefs version to the latest released (v2.9.3)
  • Adds a "multiversion" feature flag that exposes the littlefs feature of the same name
  • Patch lfs.h before building to avoid errors from clang failing to find the string.h header (this step is not required for the compilation because compilation is done through gcc by default)

Copy link
Member

@robin-nitrokey robin-nitrokey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some thoughts:

  • Please add a changelog entry.
  • I think the header patch only hides a missing dependency. If we want to be able to compile with clang, we should fix the underlying issue. On NixOS, clang_multi is the only package required for me and also finds string.h (even when setting CC=clang). Not sure what is needed on Arch.
  • We should also add support for the LFS_READONLY and LFS_THREADSAFE features added in v2.3.0.

@sosthene-nitrokey
Copy link
Contributor Author

Arch doesn't appear to have any such package, I already looked for it. I can also fix it by adding newlib to the include paths or using the --sysroot to point to it. I totally agree that this fix seems very hacky and not future-proof to me.

LFS_READONLY would need to be a made additive too, and it would not be possible to use it with the littlefs2 crate.
LFS_THREADSAFE would face the same issue, and exposing it in littlefs2 would probably be pretty hard as it would not be semver-compatible.

Overall I'm in favour of not adding support for things that we don't plan to support in the littlefs2 crate, it would just lead to untested features. If someone needs them, they can add their PR. Maybe we can force a compat-v1 feature to be enabled so that having LFS_READONLY be additive would be possible.

This patch:

- Bumps the littlefs version to the latest released (v2.9.3)
- Adds a "multiversion" feature flag that exposes the littlefs feature
of the same name
@robin-nitrokey
Copy link
Member

Okay, then let’s skip the other new features. I think it would make sense to release this as v0.3 anyway to avoid accidental upgrades to the new 2.1 format.

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

Successfully merging this pull request may close these issues.

Upgrade littlefs submodule
2 participants