Skip to content

Conversation

@e-nomem
Copy link

@e-nomem e-nomem commented Nov 21, 2025

This mtime value will be used instead of DETERMINISTIC_TIMESTAMP when provided. This is useful e.g. to pass in $SOURCE_DATE_EPOCH to support reproducible builds.

This is a breaking change. Given that HeaderMode is marked as non_exhaustive, it is possible to make this a non-breaking change by adding a new mode variant that uses the mtime override instead.

This `mtime` value will be used instead of `DETERMINISTIC_TIMESTAMP` when provided.
This is useful e.g. to pass in `$SOURCE_DATE_EPOCH` to support reproducible builds.
@e-nomem e-nomem force-pushed the deterministic-mtime branch from 6fa6cf8 to ea42424 Compare November 21, 2025 19:17
Copy link
Collaborator

@xzfc xzfc left a comment

Choose a reason for hiding this comment

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

This is a breaking change. Given that HeaderMode is marked as non_exhaustive, it is possible to make this a non-breaking change by adding a new mode variant that uses the mtime override instead.

Breaking is not desirable. A new variant would be fine.

This mtime value will be used instead of DETERMINISTIC_TIMESTAMP when provided. This is useful e.g. to pass in $SOURCE_DATE_EPOCH to support reproducible builds.

The overall idea is LGTM.

@xzfc
Copy link
Collaborator

xzfc commented Nov 23, 2025

There are multiple feature requests to extend HeaderMode:

Maybe these and similar cases could be covered by a new enum variant with a single non_exhaustive struct field.

@e-nomem e-nomem force-pushed the deterministic-mtime branch from 4f8a3af to 8b69171 Compare November 24, 2025 03:37
@e-nomem
Copy link
Author

e-nomem commented Nov 24, 2025

I added a new HeaderMode::Override variant that has non-exhaustive struct with individual field overrides. AFAICT the design addresses the use cases from all the linked issues.

It feels a touch verbose but one of the upsides is that both HeaderMode::Complete and HeaderMode::Deterministic can be expressed as overrides so it does unify the HeaderMode handling logic a bit.

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