Merge pull request #209 from cyphar/tartime-compare

compare: properly compare tar_time and time

compare.go

@@ -3,7 +3,7 @@ package mtree
 import (
 	"encoding/json"
 	"fmt"
-	"strconv"
+	"slices"
 )
 
 // XXX: Do we need a Difference interface to make it so people can do var x
@@ -252,23 +252,35 @@ func compareEntry(oldEntry, newEntry Entry) ([]KeyDelta, error) {
 
 		// Make a new tar_time.
 		if diffs["tar_time"].Old == nil {
-			time, err := strconv.ParseFloat(timeStateT.Old.Value(), 64)
+			var (
-			if err != nil {
+				timeSec, timeNsec int64
-				return nil, fmt.Errorf("failed to parse old time: %s", err)
+				// used to check for trailing characters
+				trailing rune
+			)
+			val := timeStateT.Old.Value()
+			n, _ := fmt.Sscanf(val, "%d.%d%c", &timeSec, &timeNsec, &trailing)
+			if n != 2 {
+				return nil, fmt.Errorf("failed to parse old time: invalid format %q", val)
 			}
 
 			newTime := new(KeyVal)
-			*newTime = KeyVal(fmt.Sprintf("tar_time=%d.000000000", int64(time)))
+			*newTime = KeyVal(fmt.Sprintf("tar_time=%d.%9.9d", timeSec, 0))
 
 			diffs["tar_time"].Old = newTime
 		} else if diffs["tar_time"].New == nil {
-			time, err := strconv.ParseFloat(timeStateT.New.Value(), 64)
+			var (
-			if err != nil {
+				timeSec, timeNsec int64
-				return nil, fmt.Errorf("failed to parse new time: %s", err)
+				// used to check for trailing characters
+				trailing rune
+			)
+			val := timeStateT.New.Value()
+			n, _ := fmt.Sscanf(val, "%d.%d%c", &timeSec, &timeNsec, &trailing)
+			if n != 2 {
+				return nil, fmt.Errorf("failed to parse new time: invalid format %q", val)
 			}
 
 			newTime := new(KeyVal)
-			*newTime = KeyVal(fmt.Sprintf("tar_time=%d.000000000", int64(time)))
+			*newTime = KeyVal(fmt.Sprintf("tar_time=%d.%9.9d", timeSec, 0))
 
 			diffs["tar_time"].New = newTime
 		} else {
@@ -405,15 +417,16 @@ func compare(oldDh, newDh *DirectoryHierarchy, keys []Keyword, same bool) ([]Ino
 				return nil, fmt.Errorf("comparison failed %s: %s", path, err)
 			}
 
-			// Now remove "changed" entries that don't match the keys.
+			// Ignore changes to keys not in the requested set.
 			if keys != nil {
-				var filterChanged []KeyDelta
+				changed = slices.DeleteFunc(changed, func(delta KeyDelta) bool {
-				for _, keyDiff := range changed {
+					name := delta.name.Prefix()
-					if InKeywordSlice(keyDiff.name.Prefix(), keys) {
+					return !InKeywordSlice(name, keys) &&
-						filterChanged = append(filterChanged, keyDiff)
+						// We remap time to tar_time in compareEntry, so we
-					}
+						// need to treat them equivalently here.
-				}
+						!(name == "time" && InKeywordSlice("tar_time", keys)) &&
-				changed = filterChanged
+						!(name == "tar_time" && InKeywordSlice("time", keys))
+				})
 			}
 
 			// Check if there were any actual changes.

test/cli/0012.sh

@@ -0,0 +1,64 @@
+#!/bin/bash
+set -ex
+
+name=$(basename $0)
+root="$(dirname $(dirname $(dirname $0)))"
+gomtree=$(go run ${root}/test/realpath/main.go ${root}/gomtree)
+t=$(mktemp -d /tmp/go-mtree.XXXXXX)
+
+echo "[${name}] Running in ${t}"
+
+## Make sure that comparing manifests with tar_time and time correctly detect
+## errors (while also *truncating* timestamps for tar_time comparisons).
+
+pushd ${root}
+mkdir -p ${t}/root
+
+date="2025-09-05T13:05:10.12345" # POSIX format for "touch -d".
+
+touch -d "$date" ${t}/root/foo
+
+keywords=type,uid,gid,nlink,link,mode,flags,xattr,size,sha256
+
+# Generate regular manifests with time and tar_time.
+${gomtree} -c -k ${keywords},time -p ${t}/root -f ${t}/time.mtree
+${gomtree} -c -k ${keywords},tar_time -p ${t}/root -f ${t}/tartime.mtree
+
+# Validation with both manifests should still succeed.
+${gomtree} validate -p ${t}/root -f ${t}/time.mtree
+${gomtree} validate -p ${t}/root -f ${t}/tartime.mtree
+# Manifest comparison should also succeed.
+${gomtree} validate -f ${t}/tartime.mtree -f ${t}/time.mtree
+${gomtree} validate -f ${t}/time.mtree -f ${t}/tartime.mtree
+# Forcefully requesting a different time type should also succeed.
+${gomtree} validate -k ${keywords},tar_time -p ${t}/root -f ${t}/time.mtree
+${gomtree} validate -k ${keywords},time -p ${t}/root -f ${t}/tartime.mtree
+
+# Change the timestamp.
+touch -d "1997-03-25T13:40:00.67890" ${t}/root/foo
+
+# Standard validations should fail.
+(! ${gomtree} validate -p ${t}/root -f ${t}/time.mtree)
+(! ${gomtree} validate -p ${t}/root -f ${t}/tartime.mtree)
+
+# Forcefully requesting a different time type should also fail.
+(! ${gomtree} validate -k ${keywords},tar_time -p ${t}/root -f ${t}/time.mtree)
+(! ${gomtree} validate -k ${keywords},time -p ${t}/root -f ${t}/tartime.mtree)
+
+# Ditto if we generate the manifests and compare them.
+${gomtree} -c -k ${keywords},time -p ${t}/root -f ${t}/time-change.mtree
+${gomtree} -c -k ${keywords},tar_time -p ${t}/root -f ${t}/tartime-change.mtree
+
+# Same time types.
+(! ${gomtree} validate -f ${t}/time.mtree -f ${t}/time-change.mtree)
+(! ${gomtree} validate -f ${t}/time-change.mtree -f ${t}/time.mtree)
+(! ${gomtree} validate -f ${t}/tartime.mtree -f ${t}/tartime-change.mtree)
+(! ${gomtree} validate -f ${t}/tartime-change.mtree -f ${t}/tartime.mtree)
+
+# Different time types:
+# (old) time <=> (new) tar_time
+(! ${gomtree} validate -f ${t}/time.mtree -f ${t}/tartime-change.mtree)
+(! ${gomtree} validate -f ${t}/tartime-change.mtree -f ${t}/time.mtree)
+# (old) tar_time <=> (new) time
+(! ${gomtree} validate -f ${t}/tartime.mtree -f ${t}/time-change.mtree)
+(! ${gomtree} validate -f ${t}/time-change.mtree -f ${t}/tartime.mtree)

test/cli/0012trunc.sh

@@ -0,0 +1,96 @@
+#!/bin/bash
+set -ex
+
+name=$(basename $0)
+root="$(dirname $(dirname $(dirname $0)))"
+gomtree=$(go run ${root}/test/realpath/main.go ${root}/gomtree)
+t=$(mktemp -d /tmp/go-mtree.XXXXXX)
+
+echo "[${name}] Running in ${t}"
+
+## Make sure that comparing manifests with tar_time and time are correctly
+## truncated.
+
+pushd ${root}
+mkdir -p ${t}/root
+
+date="2025-09-05T13:05:10" # POSIX format for "touch -d".
+
+echo "less than .5" >${t}/root/lowerhalf
+touch -d "$date.1000" ${t}/root/lowerhalf
+echo "more than .5" >${t}/root/upperhalf
+touch -d "$date.8000" ${t}/root/upperhalf
+echo "no subsecond" >${t}/root/tartime
+touch -d "$date.0000" ${t}/root/tartime
+
+keywords=type,uid,gid,nlink,link,mode,flags,xattr,size,sha256
+
+# Generate regular manifests with time and tar_time.
+${gomtree} -c -k ${keywords},time -p ${t}/root -f ${t}/time.mtree
+${gomtree} -c -k ${keywords},tar_time -p ${t}/root -f ${t}/tartime.mtree
+
+# Make sure that tar_time truncates the value.
+unix="$(date -d ${date} +%s)"
+grep -q "lowerhalf.*tar_time=$unix.000000000" ${t}/tartime.mtree
+grep -q "upperhalf.*tar_time=$unix.000000000" ${t}/tartime.mtree
+grep -q "tartime.*tar_time=$unix.000000000" ${t}/tartime.mtree
+
+# Validation with both manifests should still succeed.
+${gomtree} validate -p ${t}/root -f ${t}/time.mtree
+${gomtree} validate -p ${t}/root -f ${t}/tartime.mtree
+# Manifest comparison should also succeed.
+${gomtree} validate -f ${t}/tartime.mtree -f ${t}/time.mtree
+${gomtree} validate -f ${t}/time.mtree -f ${t}/tartime.mtree
+
+# Truncate the on-disk timestamps manually.
+touch -d "$date.0000" ${t}/root/lowerhalf
+touch -d "$date.0000" ${t}/root/upperhalf
+touch -d "$date.0000" ${t}/root/tartime
+
+# Only the tar_time manifest should succeed.
+(! ${gomtree} validate -p ${t}/root -f ${t}/time.mtree)
+${gomtree} validate -p ${t}/root -f ${t}/tartime.mtree
+${gomtree} validate -k ${keywords},time -p ${t}/root -f ${t}/tartime.mtree
+# ... unless you force the usage of tar_time.
+${gomtree} validate -k ${keywords},tar_time -p ${t}/root -f ${t}/time.mtree
+
+# The same goes for if you generate the manifests and compare them instead.
+${gomtree} -c -k ${keywords},time -p ${t}/root -f ${t}/time-trunc.mtree
+${gomtree} -c -k ${keywords},tar_time -p ${t}/root -f ${t}/tartime-trunc.mtree
+# Comparing time with time should fail ...
+(! ${gomtree} validate -f ${t}/time.mtree -f ${t}/time-trunc.mtree)
+(! ${gomtree} validate -f ${t}/time-trunc.mtree -f ${t}/time.mtree)
+# ... tar_time with tar_time should succeed ...
+${gomtree} validate -f ${t}/tartime.mtree -f ${t}/tartime-trunc.mtree
+${gomtree} validate -f ${t}/tartime-trunc.mtree -f ${t}/tartime.mtree
+# ... old tar_time with new time should succeed ...
+${gomtree} validate -f ${t}/tartime.mtree -f ${t}/time-trunc.mtree
+${gomtree} validate -f ${t}/time-trunc.mtree -f ${t}/tartime.mtree
+# ... and new tar_time against old time should succeed.
+${gomtree} validate -f ${t}/tartime-trunc.mtree -f ${t}/time.mtree
+${gomtree} validate -f ${t}/time.mtree -f ${t}/tartime-trunc.mtree
+
+# Change the timestamp entirely.
+touch -d "1997-03-25T13:40:00" ${t}/root/lowerhalf
+touch -d "1997-03-25T13:40:00" ${t}/root/upperhalf
+touch -d "1997-03-25T13:40:00" ${t}/root/tartime
+
+# Now all validations should fail.
+(! ${gomtree} validate -p ${t}/root -f ${t}/time.mtree)
+(! ${gomtree} validate -p ${t}/root -f ${t}/tartime.mtree)
+(! ${gomtree} validate -k ${keywords},tar_time -p ${t}/root -f ${t}/time.mtree)
+(! ${gomtree} validate -k ${keywords},time -p ${t}/root -f ${t}/tartime.mtree)
+
+# Ditto for generating the manifests and comparing them.
+${gomtree} -c -k ${keywords},time -p ${t}/root -f ${t}/time-change.mtree
+${gomtree} -c -k ${keywords},tar_time -p ${t}/root -f ${t}/tartime-change.mtree
+
+# Try all combinations.
+lefts=( ${t}/{tar,}time{,-trunc}.mtree )
+rights=( ${t}/{tar,}time-change.mtree )
+for left in "${lefts[@]}"; do
+	for right in "${rights[@]}"; do
+		(! ${gomtree} validate -f ${left} -f ${right})
+		(! ${gomtree} validate -f ${right} -f ${left})
+	done
+done