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

Expose more directory iteration methods #33

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

Conversation

sosthene-nitrokey
Copy link
Contributor

Fix #32

Tests fail because of littlefs-project/littlefs#785

@sosthene-nitrokey
Copy link
Contributor Author

The release 2.6 of littlefs should soon include the fix for the bug that makes this PR fail littlefs-project/littlefs#814

@sosthene-nitrokey
Copy link
Contributor Author

Rebased on top of #42 , so the tests pass now

@sosthene-nitrokey sosthene-nitrokey force-pushed the iter-optimization branch 2 times, most recently from 760bb7f to 6f0a3c9 Compare June 12, 2023 09:08
@sosthene-nitrokey sosthene-nitrokey marked this pull request as ready for review June 12, 2023 09:27
@robin-nitrokey
Copy link
Member

Can you please rebase again?

@sosthene-nitrokey
Copy link
Contributor Author

Thinking about how to make this available from the core crate.

@sosthene-nitrokey
Copy link
Contributor Author

Making this available from core would need breaking changes, and would introduce the DirIterationTell struct to be part of the DynFilesystem trait, which would be weird.

As part of the trussed ecosystem this is only needed in the core trussed backend which already depends on the full littlefs2 and not the core.

@robin-nitrokey
Copy link
Member

robin-nitrokey commented Mar 3, 2025

As part of the trussed ecosystem this is only needed in the core trussed backend which already depends on the full littlefs2 and not the core.

But since trussed-dev/trussed#144 it uses DynFilesystem for filesystem operations, and since trussed-dev/trussed#195 it no longer has access to the Filesystem at all.

Could we add a new function to DynFilesystem that uses a custom iterator trait that provides access to seek and tell?

@sosthene-nitrokey
Copy link
Contributor Author

Oh right.

Surprisingly cargo-semver-checks does not flag this as a breaking change in core.

I'm investigating further if this is not a false positive

@sosthene-nitrokey
Copy link
Contributor Author

I actually see no way of exposing this as part of littlefs2-core without making this a breaking change.

This is a breaking change because an existing implementation of DynFilesystem would not provide the callback with an implementation of the DirIterator. Adding a new method is hazardous because the new one would not be able to provide a default implementation of tell, seek (though one could do with a wrapper struct that uses indices) but especially rewind.

@sosthene-nitrokey
Copy link
Contributor Author

I could see adding a new dyn compatible trait (with better names):

pub trait DynFilesystemV2: DynFilesytem {
    fn read_dir_and_then_unitv2();

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.

Improve iteration capability
2 participants