Skip to content

Conversation

@fogti
Copy link
Contributor

@fogti fogti commented Oct 31, 2025

No description provided.

@fogti fogti force-pushed the image-reader branch 3 times, most recently from c95bd47 to d26ee72 Compare October 31, 2025 16:55
Comment on lines 30 to 54
[dependencies.byte-unit]
version = "5"
default-features = false
features = ["byte", "serde"]
optional = true

[dependencies.compression]
version = "0.1"
default-features = false
features = ["gzip"]
optional = true

[dependencies.goblin]
version = "0.10"
default-features = false
features = ["elf64"]
optional = true

[dependencies.num-traits]
version = "0.2"
default-features = false

[dependencies.serde]
version = "1"
default-features = false
features = ["alloc", "derive"]
optional = true

[dependencies.toml]
version = "0.9"
default-features = false
features = ["parse", "serde"]
optional = true

[dependencies.yoke]
version = "0.8"
default-features = false
features = ["derive"]
optional = true

[dev-dependencies.proptest]
version = "1.9"

Copy link
Member

Choose a reason for hiding this comment

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

Is there a specific reason you are using the explicit dependencies instead of the "one-liners"?

Copy link
Contributor Author

@fogti fogti Nov 6, 2025

Choose a reason for hiding this comment

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

It is a bit hard to read "one-liners" when they contain nested structures (features) and often wrap the line (or alternatively go out of field of vision when the window is small enough).

Copy link
Member

Choose a reason for hiding this comment

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

I guess it is an implicit kind of code-style to use the one-liners in the hermit projects. For consistency reasons, I'd stick to it. A mixture seems like a bad choice from my perspective.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does the Hermit kernel Cargo.toml count? (because it does mix those)

Copy link
Member

Choose a reason for hiding this comment

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

But only for the "very complex" smoltcp and the ones where we have platform specific dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we have some policy ala "dependencies should be in the multi-line format when the line length exceeds N"?

Copy link
Member

Choose a reason for hiding this comment

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

No. But I think here the one-liners are a better choice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the current version okay (I transformed every entry with at most one feature into a one-liner)?

Copy link
Member

Choose a reason for hiding this comment

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

Fine for me. Maybe @mkroening has some opinion on this.

@fogti
Copy link
Contributor Author

fogti commented Nov 6, 2025

For now, I fixed many of the complaints in the uhyve version of this PR, and will port them to this one later.

edit: okay, ported.

@fogti fogti force-pushed the image-reader branch 8 times, most recently from 60353e9 to 1450be7 Compare November 8, 2025 10:18
@fogti fogti marked this pull request as ready for review November 8, 2025 10:19
@fogti fogti force-pushed the image-reader branch 2 times, most recently from e2609aa to ee728b4 Compare November 10, 2025 11:32
@fogti
Copy link
Contributor Author

fogti commented Nov 10, 2025

I have now completely split this from #62, to make review + fixes easier.

@fogti fogti force-pushed the image-reader branch 3 times, most recently from 7d0e573 to 723759e Compare November 10, 2025 13:44
@fogti fogti force-pushed the image-reader branch 4 times, most recently from da5dd36 to c55f6bb Compare November 10, 2025 21:13
@fogti fogti force-pushed the image-reader branch 4 times, most recently from b720109 to f8c2e61 Compare November 17, 2025 13:44
@fogti
Copy link
Contributor Author

fogti commented Nov 17, 2025

hermit-os/hermit-rs#856 should help with further necessary development (integration into kernel)

Signed-off-by: Ellen Εμιλία Άννα Zscheile <[email protected]>
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