Skip to content

Conversation

@jeckersb
Copy link
Collaborator

This seems generally useful, mostly I could see always canonicalizing
the cmdline generated by bootc to remove a source of potential
drift/nondeterminism and better enable reproducible builds.

Signed-off-by: John Eckersberg [email protected]

@bootc-bot bootc-bot bot requested a review from jmarrero November 18, 2025 19:32
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces canonicalized() methods to Cmdline, Parameter, and ParameterKey for both byte-based and UTF-8 representations of kernel command lines. This is a useful addition for creating deterministic command line strings, for example to support reproducible builds. The implementation is functionally correct, but I've identified a couple of opportunities for performance improvements in crates/kernel_cmdline/src/bytes.rs by avoiding unnecessary memory allocations. My review comments include specific suggestions to address these.

This seems generally useful, mostly I could see always canonicalizing
the cmdline generated by bootc to remove a source of potential
drift/nondeterminism and better enable reproducible builds.

Signed-off-by: John Eckersberg <[email protected]>
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

I'd be more comfortable with just doing semantic equality where it matters, like our handling of /usr/lib/bootc/kargs.d i.e. it'd be unfortunate for someone to have to reboot because their build system started quoting a karg unnecessarily or so.

///
/// This:
///
/// 1. Sorts the parameter list
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm but I think in some corner cases this could actually change semantics. Some kargs probably have last-one-wins behavior e.g.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah that's a good point. I don't think it's at all consistent either, it varies arg to arg.

@jeckersb
Copy link
Collaborator Author

I'm going to just mark this as draft for now since I kinda hastily threw it together from branches/stash I had lying around. Might make sense to rip the Cmdline parts out but keep it for Parameter/ParameterKey ?

@jeckersb jeckersb marked this pull request as draft November 18, 2025 20:00
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