makefs: Honor -T timestamps when creating images from mtree manifests

[markj]

Summary

makefs backends rely on the fsnode structure to derive most
of the information about the underlying filesystem objects.
Depending on how the image is built, the fsnode structures
are initialized in the walk_dir or read_mtree functions.
However, read_mtree fails to take timestamps passed by -T into account,
leading to nonreproducible images in backends that do not check for -T.
Fix this and make -T backend-agnostic by adding an appropriate check
in read_mtree_keywords while making sure that mtree entries can
still override -T timestamps.

PR: 285630
Sponsored by: Klara, Inc.
Sponsored by: The FreeBSD Foundation

[markj]

This looks like a much nicer solution.

Does this mean we can remove references to stampst in the backends, e.g., ffs_build_dinode2()?

usr.sbin/makefs/mtree.c

726–728

[bnovkov]

Address @markj 's comments.

[bnovkov]

In D49531#1129628, @markj wrote:

Does this mean we can remove references to stampst in the backends, e.g., ffs_build_dinode2()?

Yes, I think ffs and msdos have stampst checks and I plan on removing them in a separate commit (along with adding appropriate tests for each backend).

[emaste]

We need to make sure that all four combinations work -- mtree mode / directory scan, -T / no -T

[emaste]

There are three possible timestamps of interest:

1. As stored in the filesystem
2. Specified in a mtree file
3. Overridden timestamp for reproducible builds (cmdline arg or SOURCE_DATE_EPOCH)

In this list higher numbered timestamps should take priority over lower ones.

[bnovkov]

Address @emaste 's comments - mtree timestamps will now no longer override the default timestamp passed by -T.

[kevans]

usr.sbin/makefs/makefs.8

262

I would've perhaps instead changed this to note that they're explicitly ignored if a default timestamp is provided.

[markj]

usr.sbin/makefs/makefs.8

262

Note that there are two distinct uses of mtree in makefs: the "normal" way, where the mtree provides the file metadata rather than the input file tree, and "specfile mode", where the mtree file acts as a filter on the input file tree. In specfile mode, input files that don't have a corresponding mtree entry are ignored, and mtree entries that don't have a corresponding input file (usually a directory) are synthesized from the mtree attributes. And, mtree attributes, such as timestamps, override those of the input filesystem.

For release building I believe we do not care about specfile mode. But the deleted comment is still correct, as apply_specdir() still ignores the -T timestamp (and this is probably the right behaviour). It would be good to further clarify how -T interacts with the two modes.

[bnovkov]

Clarify interaction between -F and -T.

usr.sbin/makefs/makefs.8

262

For release building I believe we do not care about specfile mode. But the deleted comment is still correct, as apply_specdir() still ignores the -T timestamp (and this is probably the right behaviour). It would be good to further clarify how -T interacts with the two modes.

Thanks for pointing this out, I've clarified the interaction between the two. I'll also add tests for testing this interaction.

[markj]

usr.sbin/makefs/makefs.8

264–268

This is still a bit confusing IMO: "spec file" already refers to the mtree passed with -F. Otherwise the makefs documentation refers to it as a "manifest." I'd suggest something like the following, to be more explicit: "Timestamps in a mtree(5) specfile (specified with -F) are used even if a default timestamp is specified. However, the timestamps in an mtree manifest are ignored if a default timestamp is specified."

[bnovkov]

Address @markj 's comments.

[markj]

usr.sbin/makefs/mtree.c

1059–1068

This timestamp also needs to come from stampst. I'll update the review with that change shortly.

[markj]

• Fix the man page.
• Update mtree_global_inode as well.
• Factor out some duplicated code.