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

Remove the dat folders to save littlefs space #39

Merged
merged 16 commits into from
Apr 2, 2024

Conversation

sosthene-nitrokey
Copy link
Contributor

This PR removes the dat folder used by ClientFilestore that wastes a lot of space on the filesystem.

Depends on Nitrokey/trussed#34

This PR also adds a migrate function that allows moving from the previous layout to the new layout.

The migrate function cannot by itself know if the migration has been done. I would probably have it done by the Admin App, through a "privilegded" syscall, since the admin app is loaded first and can keep track of whether the migration has already been done.

@sosthene-nitrokey
Copy link
Contributor Author

I made the migration and the removal of the dat folder optional, so this can be merged without staying a pain having to constantly rebase.

@sosthene-nitrokey sosthene-nitrokey marked this pull request as ready for review March 27, 2024 10:11
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.

I feel like the use_raw configuration option is leaking implementation details. And without context, it is not clear which setting is the “correct”/new one.

What do you think about just using a versioning system? That would make it easy to identify the correct setting, and the implementation details would stay in this crate. Potentially it could also be used to select the correct migration steps.

pub enum FilesystemLayout {
    V0,
    V1,
}

impl FilesystemLayout {
    fn use_raw(&self) -> bool {
        self != FilesystemLayout::V0
    }
}

Cargo.toml Outdated
@@ -20,12 +20,20 @@ serde-byte-array = "0.1.2"
sha2 = { version = "0.10.6", default-features = false }
subtle = { version = "2.4.1", default-features = false }
trussed = { version = "0.1.0", features = ["serde-extensions"] }
littlefs2 = "0.4.0"
admin-app = "0.1.0"
Copy link
Member

Choose a reason for hiding this comment

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

unneeded dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right admin-app can dev-dependency only.

@sosthene-nitrokey
Copy link
Contributor Author

I like that. Will do this. For se050 backend too.

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.

Thank you!

@sosthene-nitrokey sosthene-nitrokey merged commit a725ae6 into trussed-dev:main Apr 2, 2024
1 check passed
robin-nitrokey added a commit to robin-nitrokey/trussed-auth that referenced this pull request Apr 2, 2024
The changelog entry for PR trussed-dev#39 ended up in the 0.3.0 section instead of
the section for unreleased changes.
robin-nitrokey added a commit to robin-nitrokey/trussed-auth that referenced this pull request Apr 3, 2024
The changelog entry for PR trussed-dev#39 ended up in the 0.3.0 section instead of
the section for unreleased changes.
robin-nitrokey added a commit to robin-nitrokey/trussed-auth that referenced this pull request Apr 3, 2024
The changelog entry for PR trussed-dev#39 ended up in the 0.3.0 section instead of
the section for unreleased changes.
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.

2 participants