Page MenuHomeFreeBSD

makefs: Add tests for the -T flag
ClosedPublic

Authored by markj on Mar 25 2025, 1:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Sep 16, 11:28 PM
Unknown Object (File)
Tue, Sep 9, 5:46 AM
Unknown Object (File)
Mon, Sep 8, 10:36 PM
Unknown Object (File)
Sun, Sep 7, 6:02 PM
Unknown Object (File)
Sat, Sep 6, 7:50 PM
Unknown Object (File)
Sat, Sep 6, 5:30 PM
Unknown Object (File)
Wed, Sep 3, 3:12 AM
Unknown Object (File)
Sun, Aug 31, 1:31 AM
Subscribers

Details

Summary

Add tests for the -T flag to each makefs backend. This includes tests
for both mtree and directory scan options.

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

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

usr.sbin/makefs/tests/makefs_cd9660_tests.sh
380

IMO the description should be the current / expected behaviour, not the now-fixed bug - i.e. cd9660 RockRidge extensions honor timestamp provided by -T

This revision is now accepted and ready to land.Mar 25 2025, 3:49 PM
bnovkov retitled this revision from makefs: Add test for RockRidge TF records and -T timestamps to makefs: Add tests for the -T flag.
bnovkov edited the summary of this revision. (Show Details)

Add -T tests for every makefs backend.

This revision now requires review to proceed.Apr 1 2025, 11:51 AM

I've reworked this diff and added tests for all backends, including msdos which didn't have any tests previously.
Please let me know how you'd like the copyright statement to look like for the new msdos test script.

There's one additional thing I'd like to point out here - the msdos backend does not pass these tests ATM, it's atime is completely different while mtime and ctime get set to timestamp - 1. I'll look into this separately.

Update copyright statement for makefs_msdos_tests.sh.

These are all good (at catching the current failures).
My only concern however, is that we should also check that makefs takes into consideration the timestamp in the mtree file, so the priorities (at this point of the revision stack) are:

  1. -T flag .
  2. Time in mtree file.

This guarantees (or not) documentation on the behavior regarding the priorities when the SOURCE_DATE_EPOCH environment variable is introduced later on (D49602).
For example (very crude) just for FFS, but should be the similar for all cases:

# This helper function can live in makefs_tests_common.sh
change_mtree_timestamp()
{
	filename="$1"
	timestamp="$2"

	sed -i "" "s/time=.*$/time=${timestamp}/g" "$filename"
}
...
atf_test_case T_flag_mtree cleanup
T_flag_mtree_body()
{
	timestamp_m=1742574909
	timestamp_T=1742574910
	create_test_dirs
	mkdir -p $TEST_INPUTS_DIR/dir1

	atf_check -e empty -o save:$TEST_SPEC_FILE -s exit:0 \
	    mtree -c -k "type,time" -p $TEST_INPUTS_DIR
	change_mtree_timestamp $TEST_SPEC_FILE $timestamp_m
	atf_check -e empty -o not-empty -s exit:0 \
	    $MAKEFS -M 1m $TEST_IMAGE $TEST_SPEC_FILE

	mount_image
	eval $(stat -s  $TEST_MOUNT_DIR/dir1)
	atf_check_equal $st_atime $timestamp_m
	atf_check_equal $st_mtime $timestamp_m
	atf_check_equal $st_ctime $timestamp_m

	common_cleanup

	atf_check -e empty -o not-empty -s exit:0 \
	    $MAKEFS -M 1m -T $timestamp_T $TEST_IMAGE $TEST_SPEC_FILE

	mount_image
	eval $(stat -s  $TEST_MOUNT_DIR/dir1)
	atf_check_equal $st_atime $timestamp_T
	atf_check_equal $st_mtime $timestamp_T
	atf_check_equal $st_ctime $timestamp_T
}
...

I'm just trying to be very cautious about documenting (testing) this functionality.
Thank you.

Address @jlduran 's comments - add testcases that test the interaction between -F and -T

These are all good (at catching the current failures).
My only concern however, is that we should also check that makefs takes into consideration the timestamp in the mtree file, so the priorities (at this point of the revision stack) are:

  1. -T flag .
  2. Time in mtree file.

This guarantees (or not) documentation on the behavior regarding the priorities when the SOURCE_DATE_EPOCH environment variable is introduced later on (D49602).

Thank you for pointing this out and for sketching out the testcase!
After a brief discussion in D49531, the consensus was that the timestamp sources should be prioritized as follows:

  1. -F mtree specfile time
  2. -T
  3. Timestamp in input mtree file

I've added testcases that test the interactions between -F and -T.

These are all good (at catching the current failures).
My only concern however, is that we should also check that makefs takes into consideration the timestamp in the mtree file, so the priorities (at this point of the revision stack) are:

  1. -T flag .
  2. Time in mtree file.

This guarantees (or not) documentation on the behavior regarding the priorities when the SOURCE_DATE_EPOCH environment variable is introduced later on (D49602).

Thank you for pointing this out and for sketching out the testcase!
After a brief discussion in D49531, the consensus was that the timestamp sources should be prioritized as follows:

  1. -F mtree specfile time
  2. -T
  3. Timestamp in input mtree file

I've added testcases that test the interactions between -F and -T.

Great! For a moment I thought this feature would be removed.

There is another thing, but can be tested later: I am using -F just like you use it here in the tests, however, the documentation (on FreeBSD, not on NetBSD) has a very intimidating message: "This is almost certainly not the option you are looking for", so yet another combination should be tested: makefs with and without -T, and an mtree file (with a time=) as the last argument (similar to the original example). As said, it can be added later.

Thank you!

There's one additional thing I'd like to point out here - the msdos backend does not pass these tests ATM, it's atime is completely different while mtime and ctime get set to timestamp - 1. I'll look into this separately.

It's great to get tests added for things that are currently failing (already broken), but please tag it with atf_expect_fail

markj edited reviewers, added: bnovkov; removed: markj.
  • Remove expected failure annotations (assuming that the makefs code changes will be committed first)
  • Fix the msdos test: FAT timestamp precision is 2 seconds, not 1, and access times are not stored at all (only access dates are)
This revision was not accepted when it landed; it landed in state Needs Review.May 20 2025, 9:51 AM
This revision was automatically updated to reflect the committed changes.