Skip to content

Conversation

@frezbo
Copy link
Contributor

@frezbo frezbo commented Dec 19, 2025

Support reproducible fat32 images as like mkfs.vfat --invariant.

This also supports creating files and directories with with the value of SOURCE_DATE_EPOCH when set as per https://reproducible-builds.org/ providing a way to create reproducible filesystems.

@frezbo frezbo force-pushed the feat/fat-invariant-support branch from b13804b to ea91d86 Compare December 19, 2025 08:03
@deitch
Copy link
Collaborator

deitch commented Dec 19, 2025

Thanks for this @frezbo .

I am quite in favour of this. I have used reproducible builds in other spaces, notably OS builds, and like the ideas behind it.

I have some thoughts on the implementation details.

First, it needs a rebase, since #316 went in, which fixes some issues with fat32 images

Second, I like how you added it to the FilesystemSpec. I am not sure Invariant is the right term here. I get that mkfs.vfat uses that term, but it is not universal, as far as I know, and can be confusing. Once this is in for fat32, I expect someone will step in and implement the same for iso9660, and ext4, and squashfs, etc.

Which leads to wondering if a boolean Invariant (or even a Reproducible or FixedTimestamp) is the right approach. Rather than passing a boolean and then letting each filesystem resolve what to use for the date, does this go at a higher level? Should the FilesystemSpec include an actual time.Time to use, pass that in to the various filesystem implementations, which would just use it blindly? That has the advantage of bypassing all of the logic inside fat32 (and everywhere else) and just using it.

A sight variant would be to have Invariant (or more likely, Reproducible) as a property of FilesystemSpec, but instead of calculating the time inside fat32 (and iso9660, etc.), have CreateFilesystem() see it, use the SOURCE_DATE_EPOCH env var to retrieve the date, and pass it on.

Either way, fat32.Create (and iso9660 etc.) would all have simpler logic.

Thoughts?

@deitch
Copy link
Collaborator

deitch commented Dec 19, 2025

Ah, you rebased I just saw.

@deitch
Copy link
Collaborator

deitch commented Dec 19, 2025

Also, lint failed.

@frezbo
Copy link
Contributor Author

frezbo commented Dec 19, 2025

Which leads to wondering if a boolean Invariant (or even a Reproducible or FixedTimestamp) is the right approach. Rather than passing a boolean and then letting each filesystem resolve what to use for the date, does this go at a higher level? Should the FilesystemSpec include an actual time.Time to use, pass that in to the various filesystem implementations, which would just use it blindly? That has the advantage of bypassing all of the logic inside fat32 (and everywhere else) and just using it.

A sight variant would be to have Invariant (or more likely, Reproducible) as a property of FilesystemSpec, but instead of calculating the time inside fat32 (and iso9660, etc.), have CreateFilesystem() see it, use the SOURCE_DATE_EPOCH env var to retrieve the date, and pass it on.

Either way, fat32.Create (and iso9660 etc.) would all have simpler logic.

Makes sense I can pass in a time.Time and if not set default to time.Now(), setting time inside CreateFilesystem() has an issue that if you want to populate an already existing using GetFileSystem we don't have the time info, otherwise it needs to be added to the disk struct too, thoughts?

@frezbo frezbo force-pushed the feat/fat-invariant-support branch from ea91d86 to 0f433c2 Compare December 19, 2025 08:53
@deitch
Copy link
Collaborator

deitch commented Dec 19, 2025

has an issue that if you want to populate an already existing using GetFileSystem we don't have the time info, otherwise it needs to be added to the disk struct too, thoughts?

I didn't quite understand that. What is the problem case?

@frezbo
Copy link
Contributor Author

frezbo commented Dec 19, 2025

has an issue that if you want to populate an already existing using GetFileSystem we don't have the time info, otherwise it needs to be added to the disk struct too, thoughts?

I didn't quite understand that. What is the problem case?

Setting time inside CreateFilesystem will not work as the files are created on FileSystem, otherwise we'll have to pass that that info into the Directory struct

My example was creating files on an elready existing FAT32 file, created with mkfs.vfat so the code goes like this:

f := d.GetPartition(0)
f.OpenFile`

in this case having time inside `CreatePartition` will not work

@frezbo frezbo force-pushed the feat/fat-invariant-support branch from 0f433c2 to 5f2db5b Compare December 19, 2025 09:51
@deitch
Copy link
Collaborator

deitch commented Dec 19, 2025

Oh, I think I see. You are not worrying about the timestamp when you create the filesystem (although that matters, too), but (also) about what happens when I already have a filesystem (accessed via Create where you created a new one, or via Get where you access an already existing one), and you want new directory or file creation to control the timestamp, rather than using time.Now() to use the actual current clock time. Is that it?

@frezbo
Copy link
Contributor Author

frezbo commented Dec 19, 2025

Oh, I think I see. You are not worrying about the timestamp when you create the filesystem (although that matters, too), but (also) about what happens when I already have a filesystem (accessed via Create where you created a new one, or via Get where you access an already existing one), and you want new directory or file creation to control the timestamp, rather than using time.Now() to use the actual current clock time. Is that it?

Yes, correct, using Get on vfat created elsewhere, then populating it with source

@deitch
Copy link
Collaborator

deitch commented Dec 19, 2025

Yes, correct, using Get on vfat created elsewhere, then populating it with source

Ah, I get it. So it makes sense to have all of these resolved inside the specific use case (here, fat32). I rather like it.

In the case, I have only two suggestions:

  1. Move time.go into util/ (maybe util/timestamps/ dir, so it doesn't conflict with the main library time). That way the standard reproducible logic is outside of a particular filesystem, and we can reuse it easily in other filesystems wherever we use time.Now() (when relevant).
  2. Create a unit test for getTime() (or. when moved, GetTime()). I can think of two simple test cases: set SOURCE_DATE_EPOCH and see that it is correct; don't set it, and see that the result is within, say, 3s of right now. You never will get it exactly right because of the time it takes to run the test. You could stub out time.Now(), but really not worth the trouble for that simple test.

@frezbo frezbo force-pushed the feat/fat-invariant-support branch from 5f2db5b to 8adbe03 Compare December 19, 2025 14:09
@frezbo
Copy link
Contributor Author

frezbo commented Dec 19, 2025

Yes, correct, using Get on vfat created elsewhere, then populating it with source

Ah, I get it. So it makes sense to have all of these resolved inside the specific use case (here, fat32). I rather like it.

In the case, I have only two suggestions:

1. Move `time.go` into `util/` (maybe `util/timestamps/` dir, so it doesn't conflict with the main library `time`). That way the standard reproducible logic is outside of a particular filesystem, and we can reuse it easily in other filesystems wherever we use `time.Now()` (when relevant).

2. Create a unit test for `getTime()` (or. when moved, `GetTime()`). I can think of two simple test cases: set `SOURCE_DATE_EPOCH` and see that it is correct; don't set it, and see that the result is within, say, 3s of right now. You never will get it exactly right because of the time it takes to run the test. You could stub out `time.Now()`, but really not worth the trouble for that simple test.

done

Support reproducible fat32 images as like `mkfs.vfat --invariant`.

This also supports creating files and directories with with the value of `SOURCE_DATE_EPOCH` when set as per https://reproducible-builds.org/ providing a way to create reproducible filesystems.

Signed-off-by: Noel Georgi <[email protected]>
@frezbo frezbo force-pushed the feat/fat-invariant-support branch from 8adbe03 to d1eea98 Compare December 19, 2025 14:12
Copy link
Collaborator

@deitch deitch left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for this.


got := timestamp.GetTime()
expected := tt.expectedTimeFunc()
if !got.Truncate(time.Second).Equal(expected.Truncate(time.Second)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like how you did this.

@deitch deitch merged commit 874ff92 into diskfs:master Dec 20, 2025
20 checks passed
@frezbo frezbo deleted the feat/fat-invariant-support branch December 21, 2025 10:22
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